Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Throwable to be extended outside of Exception #17233

Open
robincafolla opened this issue Dec 21, 2024 · 3 comments
Open

Allow Throwable to be extended outside of Exception #17233

robincafolla opened this issue Dec 21, 2024 · 3 comments

Comments

@robincafolla
Copy link

Description

At present \Throwable cannot be directly extended. If you try to it throws a Fatal error:

PHP Fatal error: Class FakeException cannot implement interface Throwable, extend Exception or Error instead in /censored/path/to/FakeException.php on line 7

This makes sense for the vast majority of use cases; it prevents people from re-inventing the wheel and trying to implement Throwable when really they should just extend Exception.

However, there are cases where extending Throwable would make sense, particularly in light of the fact that all methods on Exception are final.

For example, tests for exception handling and logging code can't mock Exceptions fully or sensibly. Testing that your log messages are being correctly written to log storage is difficult to make repeatable.

There are 3 solutions to this problem:

  1. Allow \Throwable to be extended
  2. Make final methods in \Exception non-final to allow stubs to over-ride existing behaviour
  3. Add a new class specifically for tests which implements Throwable and can be extended to stub behaviour

I think that option 1 is the simplest. We should rely on the docs to tell people to extend Exception, but allow Throwable to be implemented.

@bwoebi
Copy link
Member

bwoebi commented Dec 22, 2024

While the methods are final, the properties can be trivially set via reflection. (And mostly, the methods just return those.)

vishwamartur added a commit to vishwamartur/php-src that referenced this issue Jan 6, 2025
Related to php#17233

Allow `\Throwable` to be extended directly without causing a Fatal error.

* **Zend/zend_exceptions.c**
  - Remove the restriction in `zend_implement_throwable` function that checks if a class implementing `\Throwable` extends `Exception` or `Error`.
  - Update the `zend_register_default_exception` function to register the `zend_ce_throwable` interface without the restriction.

* **Zend/tests/bug_test.phpt**
  - Add a new test file to ensure that extending `\Throwable` directly works as expected.
  - Add a test case to check if a class can extend `\Throwable` directly without causing a Fatal error.
  - Add a test case to check if a class extending `\Throwable` directly can implement the required methods.
@iluuu1994
Copy link
Member

From the Throwable RFC:

https://wiki.php.net/rfc/throwable-interface

The proposed patch does not allow userland classes to implement the Throwable interface. Instead all classes declared in userland must extend one of the existing exception classes.

Exception and Error are distinct classes of exceptions. Exception is used for anticipated errors, i.e. things the user should guard for. Error is used for programming mistakes that should generally never be caught. It is currently safe to assume that handling both of them handles all exceptions.

Changing how this works should be discussed in an RFC.

@mvorisek
Copy link
Contributor

mvorisek commented Jan 6, 2025

I do not think implementing Throwable directly should be allowed neither. I miss to find the strong reason why and also it will imply huge BC break as a lot of code assumes the Error/Exception only.

What I however support is to remove the final from the methods. My usecase is to store some backtrace references using WeakReference and return the backtrace without WeakReference (with real objects if they are still alive).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants