[HttpKernel] Inherit Throwable in HttpExceptionInterface #33809
Conversation
b2d201b to
2ac3fbf
Compare
Did it, and removed the upgrade line. However, I do not understand why this is considered a new feature instead of a BC break. 🤔 |
|
If it's considered as a BC break, then merging on master is equally wrong: we don't do hard BC breaks that don't trigger a prior notice in 4.4. Now we can discuss the BC break nature of this. This is a BC break only if some class implements |
I see your point, I don't think anyone did this too. |
|
Thank you @nesk. |
… (nesk)
This PR was merged into the 4.4 branch.
Discussion
----------
[HttpKernel] Inherit Throwable in HttpExceptionInterface
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| **BC BREAKS?**| **YES**
| Tickets | n/a
| License | MIT
| Doc PR | n/a
When using a static analysis tools, it is not possible to do this:
```php
if ($exception instanceof HttpExceptionInterface) {
$exception->getStatusCode();
$exception->getHeaders();
$exception->getMessage(); // ❌ Will fail here
}
```
This is due to `getMessage()` not being a method declared in `HttpExceptionInterface`. Since Symfony now requires PHP 7.1+ to run, it is safe to inherit from the `Throwable` interface (added in PHP 7.0).
### About backward compatibility
Adding new methods to `HttpExceptionInterface` [breaks BC](https://symfony.com/doc/current/contributing/code/bc.html#changing-interfaces), however this interface shouldn't be used on a class other than an exception, so this shouldn't affect much code.
### About tests
I'm not sure this really needs tests, but maybe I'm wrong? Tell me what to test if you think this is required.
Commits
-------
2ac3fbf Inherit Throwable in HttpExceptionInterface
When using a static analysis tools, it is not possible to do this:
This is due to
getMessage()not being a method declared inHttpExceptionInterface. Since Symfony now requires PHP 7.1+ to run, it is safe to inherit from theThrowableinterface (added in PHP 7.0).About backward compatibility
Adding new methods to
HttpExceptionInterfacebreaks BC, however this interface shouldn't be used on a class other than an exception, so this shouldn't affect much code.About tests
I'm not sure this really needs tests, but maybe I'm wrong? Tell me what to test if you think this is required.