Add ErrorHandler component #31065

yceruto · 2019-04-10T16:26:31Z

Q	A
Branch?	4.4
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	yes
Tests pass?	no
Fixed tickets	#25905, #26448
License	MIT
Doc PR	TODO

Mainly for API-based apps that don't require TwigBundle to get the correct exception response according to the request format (aka _format attribute).

✔️ RFC7807 compliant for JSON and XML formats.

This introduce a new ErrorRenderer service that render a FlattenException into a given format:

use Symfony\Component\ErrorHandler\ErrorRenderer\ErrorRenderer;
use Symfony\Component\ErrorHandler\ErrorRenderer\HtmlErrorRenderer;
use Symfony\Component\ErrorHandler\ErrorRenderer\JsonErrorRenderer;

$renderers = [
    new HtmlErrorRenderer(),
    new JsonErrorRenderer(),
    // ...
];
$errorRenderer = new ErrorRenderer($renderers);

return new Response(
    $errorRenderer->render($exception, $request->getRequestFormat()), 
    $exception->getStatusCode(), 
    $exception->getHeaders()
);

The built-in error renderers are:

Format	Class
html	HtmlErrorRenderer
json	JsonErrorRenderer
xml, atom	XmlErrorRenderer
txt	TxtErrorRenderer

And you can add your own error renderer by implementing the ErrorRendererInterface and tagging it with error_handler.renderer in your service definition.

Creating your own error renderer for a built-in format will end up replacing the related built-in error renderer.

Demo: https://github.com/yceruto/error-handler-app (add custom error renderer)

Simperfit · 2019-04-11T08:12:56Z

This is gonna be really nice !

nicolas-grekas · 2019-04-12T14:40:49Z

Would it make sense no name the component ErrorHandler?

yceruto · 2019-04-12T15:28:28Z

Would it make sense no name the component ErrorHandler?

Does that make me think that we should move ErrorHandler and ExceptionHandler classes from the Debug component to here, also? Currently, this component only solve the convertion of the exception to a formatted output (html, json, etc) but there is no register()/handle() method involved at this moment.

stof · 2019-04-12T15:31:09Z

The discussion we had with @fabpot and @nicolas-grekas in Brussels concluded that we should indeed create a new component for error handling. Having the production error handling being a responsibility of a component named debug looks weird, as prod is not in debug mode.

stof · 2019-04-12T15:36:06Z

I don't think having an ErrorException class being responsible both for representing the Exception and for rendering it in multiple formats is the right architecture for the component though. This is not extendable at all, and cannot even be configurable (as you cannot instantiate the rendering layer in a place configuring it).

To me, we need 3 separate things in the component:

the ExceptionHandler/ErrorHandler (moved from Debug)
the value object representing the actual exception
a renderer layer knowing how to render the exception in different format (HTML, JSON corresponding to some standard format, XML, etc...)

And then, the Twig-based HTML error page should ideally become a hook into this renderer layer rather than being a totally separate rendering system.

yceruto · 2019-04-12T15:41:31Z

I agree, we should also discuss about BC then, there is a lot of exception classes in Debug component used by these two handlers and we wouldn't want this new component require to symfony/debug :)

yceruto · 2019-04-12T19:59:12Z

Update

Component renamed to "ErrorHandler" as suggested by Nicolas (it's a better name IMO too).
Almost everything was moved from the Debug component because interdependence between handlers -> classes/interface, except Debug class.

Still working on the renderer layer...

Taluu

How about being able to support more formats ? Wouldn't it be better to have some sort of factory so that we can use whatever format we want ?

src/Symfony/Component/ErrorHandler/ErrorException.php

yceruto · 2019-04-12T20:11:55Z

How about being able to support more formats ? Wouldn't it be better to have some sort of factory so that we can use whatever format we want ?

Thanks @Taluu for your input, yes I'm working on it.

yceruto · 2019-04-14T00:08:28Z

Update

Added error renderer layer with interface + DI tag error_handler.renderer to add custom format/renderer:

interface ErrorRendererInterface
{
    /**
     * Gets the format of the content.
     *
     * @return string The content format
     */
    public static function getFormat(): string;

    /**
     * Renders an Exception and returns the Response content.
     *
     * @return string The Response content as a string
     */
    public function render(FlattenException $exception): string;
}

Added FrameworkBundle integration.

yceruto · 2019-04-14T00:08:33Z

And then, the Twig-based HTML error page should ideally become a hook into this renderer layer rather than being a totally separate rendering system.

There are many changes (a big diff), most are movements + deprecations, so I prefer to leave this last issue for another PR

yceruto · 2019-04-14T02:47:35Z

The PR app demo is ready: https://github.com/yceruto/error-handler-app (add custom error renderer)

fabpot · 2019-06-27T12:40:54Z

Thank you @yceruto.

This PR was merged into the 4.4 branch. Discussion ---------- Add ErrorHandler component | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | no | Fixed tickets | #25905, #26448 | License | MIT | Doc PR | TODO Mainly for API-based apps that don't require TwigBundle to get the correct exception response according to the request format (aka `_format` attribute). ![exception_response](https://user-images.githubusercontent.com/2028198/55509651-713dc700-562a-11e9-8b98-bef3b0229397.gif) :heavy_check_mark: [RFC7807](https://tools.ietf.org/html/rfc7807) compliant for JSON and XML formats. --- This introduce a new `ErrorRenderer` service that render a `FlattenException` into a given format: ```php use Symfony\Component\ErrorHandler\ErrorRenderer\ErrorRenderer; use Symfony\Component\ErrorHandler\ErrorRenderer\HtmlErrorRenderer; use Symfony\Component\ErrorHandler\ErrorRenderer\JsonErrorRenderer; $renderers = [ new HtmlErrorRenderer(), new JsonErrorRenderer(), // ... ]; $errorRenderer = new ErrorRenderer($renderers); return new Response( $errorRenderer->render($exception, $request->getRequestFormat()), $exception->getStatusCode(), $exception->getHeaders() ); ``` The built-in error renderers are: | Format | Class | | --- | --- | | html | HtmlErrorRenderer | | json | JsonErrorRenderer | | xml, atom | XmlErrorRenderer | | txt | TxtErrorRenderer | And you can add your own error renderer by implementing the `ErrorRendererInterface` and tagging it with `error_handler.renderer` in your service definition. Creating your own error renderer for a built-in format will end up replacing the related built-in error renderer. Demo: https://github.com/yceruto/error-handler-app ([add custom error renderer](yceruto/error-handler-app@06fc647)) Commits ------- 7057244 Added ErrorHandler component

src/Symfony/Bundle/FrameworkBundle/Resources/config/error_renderer.xml

src/Symfony/Component/ErrorHandler/ErrorRenderer/ErrorRenderer.php

src/Symfony/Bundle/FrameworkBundle/Resources/config/error_renderer.xml

driesvints · 2019-07-04T13:44:02Z

@fabpot @yceruto the removed public methods like getContent and getStylesheet on the Symfony\Component\Debug\ExceptionHandler class are breaking changes according to SemVer. Can you please revert these removals or introduce this new functionality on 5.0 instead?

These changes will, for example break Lumen: laravel/lumen-framework#935

Tobion · 2019-07-04T15:14:40Z

please open a new issue

driesvints · 2019-07-04T15:26:31Z

@Tobion done: #32371

@nicolas-grekas

…(1st step) (yceruto) This PR was squashed before being merged into the 4.4 branch (closes #32377). Discussion ---------- [Debug] Restoring back the state of the Debug component (1st step) | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #32371 | License | MIT After a good discussion with @nicolas-grekas, we made the decision to split the current `ErrorCatcher` component into several: * `ErrorHandler` it would be the Debug component before these changes #31065, with everything related to ErrorHandler, Debug, DebugClassLoader classes and change its name. * `ErrorDumper` it would be the current ErrorCatcher but with FlattenException + the new error renderer system only. This is the first step, then we can deprecate everything for the Debug component in favor of the ErrorHandler and ErrorDumper components, **BUT without moving any code !!**, that would give us more freedom to do it correctly in the new components. NOTE: For this PR I've copy the `Debug` component directory from the revision prior to merged commit #31065 in 4.4 branch. Commits ------- eda49e2 [Debug] Restoring back the state of the Debug component (1st step)

@nicolas-grekas

…(1st step) (yceruto) This PR was squashed before being merged into the 4.4 branch (closes #32377). Discussion ---------- [Debug] Restoring back the state of the Debug component (1st step) | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | symfony/symfony#32371 | License | MIT After a good discussion with @nicolas-grekas, we made the decision to split the current `ErrorCatcher` component into several: * `ErrorHandler` it would be the Debug component before these changes symfony/symfony#31065, with everything related to ErrorHandler, Debug, DebugClassLoader classes and change its name. * `ErrorDumper` it would be the current ErrorCatcher but with FlattenException + the new error renderer system only. This is the first step, then we can deprecate everything for the Debug component in favor of the ErrorHandler and ErrorDumper components, **BUT without moving any code !!**, that would give us more freedom to do it correctly in the new components. NOTE: For this PR I've copy the `Debug` component directory from the revision prior to merged commit symfony/symfony#31065 in 4.4 branch. Commits ------- eda49e295e [Debug] Restoring back the state of the Debug component (1st step)

src/Symfony/Component/ErrorHandler/ErrorRenderer/TxtErrorRenderer.php

…formats and using ErrorRenderer as fallback (yceruto) This PR was merged into the 4.4 branch. Discussion ---------- [TwigBundle] Deprecating error templates for non-html formats and using ErrorRenderer as fallback | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - In the previous [PR](#31065) we created a new mechanism to render any PHP error/exception in a formatted string, which if the FB is enabled, would return an HTTP Response according to the preferred Request format (html, json, xml, txt, etc.), but when installing the TwigBundle this rendering mechanism is replaced by the current ExceptionController. This ExceptionController allows us to render custom error pages based on Twig in many formats, just what is already supported with the new ErrorRenderer component, so let's deprecate this in favor of the native. Commits ------- bf0c24a Deprecating error templates for non-html formats and using ErrorRenderer

ghost · 2019-12-07T20:32:36Z

ErrorHandler respecting Content-Type is great in API projects! Thanks!

carsonbot added Status: Needs Review Feature Deprecation labels Apr 10, 2019

yceruto changed the title ~~[ErrorException] Add ErrorException component~~ Add ErrorException component Apr 10, 2019

yceruto mentioned this pull request Apr 10, 2019

[HttpKernel][Debug] Get exception content according to request format #30851

Closed

2 tasks

yceruto force-pushed the error_exception_component branch from 42d5a64 to 3f9b448 Compare April 10, 2019 16:28

yceruto mentioned this pull request Apr 10, 2019

Debug\ExceptionHandler should have a JSON output option #26448

Closed

nicolas-grekas added this to the next milestone Apr 11, 2019

yceruto force-pushed the error_exception_component branch from 3f9b448 to 342ee6e Compare April 11, 2019 19:31

yceruto force-pushed the error_exception_component branch 3 times, most recently from cb2ac97 to 50ec30a Compare April 12, 2019 19:34

yceruto changed the title ~~Add ErrorException component~~ Add ErrorHandler component Apr 12, 2019

yceruto force-pushed the error_exception_component branch from 50ec30a to cc01b78 Compare April 12, 2019 19:56

Taluu reviewed Apr 12, 2019

View reviewed changes

src/Symfony/Component/ErrorHandler/ErrorException.php Outdated Show resolved Hide resolved

yceruto force-pushed the error_exception_component branch from cc01b78 to 8d641e3 Compare April 12, 2019 20:10

yceruto force-pushed the error_exception_component branch from 8d641e3 to a82888e Compare April 13, 2019 14:43

yceruto force-pushed the error_exception_component branch from ed3480a to 3c05010 Compare April 14, 2019 01:25

yceruto marked this pull request as ready for review April 14, 2019 02:47

carsonbot added Status: Reviewed and removed Status: Needs Review labels Jun 27, 2019

fabpot mentioned this pull request Jun 27, 2019

Add docs for ErrorCatcher component symfony/symfony-docs#11837

Closed

fabpot merged commit 7057244 into symfony:4.4 Jun 27, 2019

yceruto deleted the error_exception_component branch June 27, 2019 12:46