[HttpKernel] Allow ErrorHandler ^5.0 to be used in HttpKernel 4.4 #46327
Conversation
This was done in 7ec445b by @nicolas-grekas. Not sure if he still knows why he did that back then. |
|
Thank you Matthias. |
|
Your pace is awesome, thank you guys! |
|
Cannot tell right now if that’s an incompatibility between Symfony packages and/or the person did not declare their dependencies correctly. |
|
Well, they probably don't have a direct dependency on symfony/error-handler in their project, and upgrading to version 5 of the error handler removed the BC layer with symfony/debug (but for a project upgrading Symfony 4 from minor to minor, not having a requirement on symfony/error-handler is quite expected as earlier versions of symfony/http-kernel were using symfony/debug for that feature) |
|
I think the situation is as follows:
I might be wrong, but is this a missing depencendy declaration (either requires or maybe conflicts) in TwigBundle 4.4 against either ErrorHandler or the Debug component? Reasons against this: TwigBundle does not require either one, and also has no problem when ErrorHandler 5.x is installed... it just can't handle exceptions generated in ErrorHandler 5.x. Another suggestion: I could change the ExceptionController in TwigBundle 4.4 to be kind of forwards-compatible and accept either Exceptions from Debug or ErrorHandler. But that change would probably be breaking BC for users inheriting from that Controller and overwriting the method, right? |
|
Would it be OK if ErrorHandler's |
|
The point of doing a major version bump is to get rid of those hacks. I think we should revert this PR. That's quite unfortunate but we cannot afford breaking existing stacks I fear. |
|
I agree about reverting that PR. |
|
Also agree! |
|
Should we learn from this that some kind of test is missing… if so, what might that be? |
|
The missing test would be a functional test triggering the rendering of an error page. |
|
Reverted in #46365 |
… be used" (nicolas-grekas) This PR was merged into the 4.4 branch. Discussion ---------- [HttpKernel] Revert "bug #46327 Allow ErrorHandler ^5.0 to be used" | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - This reverts commit 1e317cc, reversing changes made to 129a271. As discussed in #46327 Commits ------- c206591 Revert "bug #46327 [HttpKernel] Allow ErrorHandler ^5.0 to be used in HttpKernel 4.4 (mpdude)"
* 4.4: [Mime] Add null check for EmailHeaderSame Add approriate description to CollectionToArrayTransformer::reverseTransform docblock [PropertyInfo] CS fixes [VarDumper] fix test on PHP 8.2 [Config] Fix looking for single files in phars with GlobResource Revert "bug #46327 [HttpKernel] Allow ErrorHandler ^5.0 to be used in HttpKernel 4.4 (mpdude)"
* 5.4: [VarDumper] fix tests on PHP 8.2 [Mime] Add null check for EmailHeaderSame Add approriate description to CollectionToArrayTransformer::reverseTransform docblock [PropertyInfo] CS fixes [VarDumper] fix test on PHP 8.2 [Config] Fix looking for single files in phars with GlobResource Revert "bug #46327 [HttpKernel] Allow ErrorHandler ^5.0 to be used in HttpKernel 4.4 (mpdude)"
* 6.0: [VarDumper] fix tests on PHP 8.2 [Mime] Add null check for EmailHeaderSame Add approriate description to CollectionToArrayTransformer::reverseTransform docblock [PropertyInfo] CS fixes [VarDumper] fix test on PHP 8.2 [Config] Fix looking for single files in phars with GlobResource Revert "bug #46327 [HttpKernel] Allow ErrorHandler ^5.0 to be used in HttpKernel 4.4 (mpdude)"
I wonder what prevents us from allowing HttpKernel ^4.4 to be used with ErrorHandler ^5.0?
ErrorHandler 5.4 (IIRC) adds the incredibly helpful feature ❤️🔥 to emit deprecation notices for missing return type hints. This is valuable when trying to migrate larger codebases to make use of type hints.
Symfony 4.4 is the current LTS, and this PR would allow people to switch to ErrorHandler 5.4 while the rest of the code can still use Symfony 4.4 components.