Skip to content

[ErrorHandler] Decouple from ErrorRenderer component #32637

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

Merged
merged 1 commit into from
Jul 25, 2019

Conversation

yceruto
Copy link
Member

@yceruto yceruto commented Jul 20, 2019

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

built on top of #32636 (See 2nd commit only)

@yceruto yceruto added this to the next milestone Jul 20, 2019
@yceruto yceruto requested a review from nicolas-grekas July 20, 2019 12:55
@yceruto yceruto changed the title [ErrorHandler] Decouple error renderer [ErrorHandler] Decouple from ErrorRenderer component Jul 20, 2019
@yceruto yceruto force-pushed the decouple_error_renderer branch 2 times, most recently from 764c38a to 1aa863e Compare July 23, 2019 12:37
@fabpot fabpot force-pushed the decouple_error_renderer branch from 1aa863e to 8f13fc0 Compare July 25, 2019 09:38
@fabpot
Copy link
Member

fabpot commented Jul 25, 2019

Thank you @yceruto.

@fabpot fabpot merged commit 8f13fc0 into symfony:4.4 Jul 25, 2019
fabpot added a commit that referenced this pull request Jul 25, 2019
…yceruto)

This PR was squashed before being merged into the 4.4 branch (closes #32637).

Discussion
----------

[ErrorHandler] Decouple from ErrorRenderer component

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

built on top of #32636 (See 2nd commit only)

Commits
-------

8f13fc0 [ErrorHandler] Decouple from ErrorRenderer component
@yceruto yceruto deleted the decouple_error_renderer branch July 25, 2019 12:09
@yceruto
Copy link
Member Author

yceruto commented Jul 25, 2019

This PR was containing some changes belong to #32636, were aware of it?

Just asking if should we revert that changes related to #32636 or should we close #32636 and iterate over Nicolas' comments in a separate PR?

fabpot added a commit that referenced this pull request Aug 20, 2019
…le early failures (yceruto)

This PR was merged into the 4.4 branch.

Discussion
----------

[ErrorHandler] Registering basic exception handler to handle early failures

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

This behavior was previously handled by the removed `ExceptionHandler` class in #32637.

As this method is mainly called during Kernel boot, where nothing is yet available, the Response content is always HTML. Otherwise, If all goes well on booting, this exception handler will be replaced in `DebugHandlersListener` class:
https://github.com/symfony/symfony/blob/8073b8abfb82d59c6acabce7cec7bfb3af24738a/src/Symfony/Component/HttpKernel/EventListener/DebugHandlersListener.php#L139

where the advanced exception handler mechanism is activated.

Commits
-------

a2077a2 Registers basic exception handler to handle early failures
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
This was referenced Nov 12, 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.

5 participants