Skip to content

[ErrorRenderer] Add alias to FlattenException to avoid BC break #32873

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 5, 2019
Merged

[ErrorRenderer] Add alias to FlattenException to avoid BC break #32873

merged 1 commit into from
Aug 5, 2019

Conversation

yceruto
Copy link
Member

@yceruto yceruto commented Aug 1, 2019 •
edited
Loading

Q A
Branch? 4.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #32473
License MIT
Doc PR -

@yceruto yceruto added this to the next milestone Aug 1, 2019
@yceruto
Copy link
Member Author

yceruto commented Aug 1, 2019 •
edited
Loading

I wonder if should we create an alias for each exception class in the ErrorHandler component?

/cc @Tobion @nicolas-grekas

@nicolas-grekas
Copy link
Member

this won't solve the linked issue, as the alias will never be loaded...
the only way is to create the alias when loading the new class
so we could add the file you have here at the end of the real one

@yceruto yceruto force-pushed the flatten_exception_alias branch from 948f32e to 4f071df Compare August 1, 2019 20:16
@yceruto
Copy link
Member Author

yceruto commented Aug 1, 2019 •
edited
Loading

this won't solve the linked issue, as the alias will never be loaded...
the only way is to create the alias when loading the new class
so we could add the file you have here at the end of the real one

That's the case when we (the HttpKernel or ErrorRenderer component) throws the new exception, but shouldn't we also cover the case when someone is throwning the old one? The lastest changes cover both cases.

nicolas-grekas
nicolas-grekas reviewed Aug 1, 2019
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

(we should do this only for a very limited subset of the classes in Debug - this one only maybe?)

@yceruto yceruto force-pushed the flatten_exception_alias branch from 4f071df to 7b11821 Compare August 1, 2019 20:40
@yceruto yceruto force-pushed the flatten_exception_alias branch from 5445a1d to 44b0e7d Compare August 1, 2019 21:59
@yceruto
Copy link
Member Author

yceruto commented Aug 1, 2019

Done!
Thank you Nicolas :)

seriquynh
seriquynh reviewed Aug 5, 2019
View reviewed changes
src/Symfony/Component/ErrorRenderer/Exception/FlattenException.php
class_alias(\Symfony\Component\ErrorRenderer\Exception\FlattenException::class, FlattenException::class);
}

if (false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of "if (false)" this case?

@fabpot
Copy link
Member

fabpot commented Aug 5, 2019

Thank you @yceruto.

@fabpot fabpot merged commit 44b0e7d into symfony:4.4 Aug 5, 2019
fabpot added a commit that referenced this pull request Aug 5, 2019
@fabpot
bug #32873 [ErrorRenderer] Add alias to FlattenException to avoid BC …
748dd8e 
…break (yceruto)

This PR was merged into the 4.4 branch.

Discussion
----------

[ErrorRenderer] Add alias to FlattenException to avoid BC break

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32473
| License       | MIT
| Doc PR        | -

Commits
-------

44b0e7d Created alias to FlattenException to avoid BC break
@yceruto yceruto deleted the flatten_exception_alias branch August 5, 2019 11:52
@chalasr chalasr mentioned this pull request Aug 8, 2019
Merged
@skalpa skalpa mentioned this pull request Sep 18, 2019
Closed
@yceruto yceruto mentioned this pull request Oct 9, 2019
Closed
@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
Reviewers

@seriquynh seriquynh seriquynh left review comments

@fabpot fabpot fabpot approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

Assignees
No one assigned
Labels
Bug Status: Reviewed
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.
5 participants
@yceruto @nicolas-grekas @fabpot @seriquynh @carsonbot