Skip to content

Fix BC Compatibility to use old FlattenException in ExceptionController #33918

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

Closed
wants to merge 2 commits into from

Conversation

alexander-schranz
Copy link
Contributor

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #...
License MIT
Doc PR

If you create a CustomExceptionController and typehinted the showAction with the old FlattenException this will break.

Maybe a better solution would to change that the new extend from the old in https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/ErrorRenderer/Exception/FlattenException.php#L381 instead of the old extending the new.

Copy link
Member

@yceruto yceruto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same opinion as #33916 (review) (:-1:)

The solution will be require symfony/debug as hard dependecy to avoid BC break.

#32873 (comment)

We provide no guarantee at the dependency level: if ppl use Debug but have it transitively, it's accepted policy to require them to make it an explicit dep as of the next version (minor included)
Said another way: the BC policy covers the API, not the deps.

@alexander-schranz
Copy link
Contributor Author

@yceruto Sorry this does not work in this case. I change to have symfony/debug as requirement in my library and it is installed but as you see here:

'exception' => FlattenException::createFromThrowable($exception),

Symfony ExceptionListener create a new FlattenException and as that does not extend from the old one. You will get an error that a false FlattenException is given.

@alexander-schranz
Copy link
Contributor Author

alexander-schranz commented Oct 9, 2019

I think its a similiar issue we had with the KernelBrowser and the Client here:

#31762 which was then later fixed in: #31881

The change in this PR is maybe not correct. Think the only correct fix would be to keep backward compatibility is that a new added class extends the old one and not the other way around.

@alexander-schranz
Copy link
Contributor Author

Want to move this discussion to an issue: #33929

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants