Skip to content

Commit

Permalink
feature #34312 [ErrorHandler] merge and remove the ErrorRenderer comp…
Browse files Browse the repository at this point in the history
…onent (nicolas-grekas, yceruto)

This PR was merged into the 4.4 branch.

Discussion
----------

[ErrorHandler] merge and remove the ErrorRenderer component

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | yes
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

This PR supersedes #34288.

Here is what it does:
- Merge the `ErrorRenderer` component into `ErrorHandler`
- Add `ErrorRendererInterface::render(\Throwable $e): FlattenException` and refactor error renderers around it.
- Add `FlattenException::setAsString()` to make the previous possible.
- Add `CliErrorRenderer` to render error on the CLI too. This means `VarDumper` is now a required dependency of `ErrorHandler`. This paves the way to use it also for rendering HTML - the logic there is much more advanced than what `HtmlErrorRenderer` provides and ever should provide.
- Make `BufferingLogger` map its collected logs to `error_log()` if they are not emptied before.
- Remove some classes that are not needed anymore (`ErrorRenderer`, `ErrorRendererPass`, `HtmlErrorRendererInterface`)
- Simplified the logic in `Debug::enable()` - nobody uses its arguments
- Fix a few issues found meanwhile.

With these changes, the component can be used standalone. One is now able to require only it, register it either with either `ErrorHandler::register()` or `Debug::enable()` and profit.

Commits
-------

d1bf1ca [ErrorHandler] help finish the PR
6c9157b [ErrorHandler] merge and remove the ErrorRenderer component
  • Loading branch information
fabpot committed Nov 12, 2019
2 parents 25c166f + d1bf1ca commit 0b867be
Show file tree
Hide file tree
Showing 105 changed files with 650 additions and 1,709 deletions.
42 changes: 15 additions & 27 deletions UPGRADE-4.4.md
Expand Up @@ -18,8 +18,6 @@ Console
Debug Debug
----- -----


* Deprecated the `Debug` class, use the one from the `ErrorRenderer` component instead
* Deprecated the `FlattenException` class, use the one from the `ErrorRenderer` component instead
* Deprecated the component in favor of the `ErrorHandler` component * Deprecated the component in favor of the `ErrorHandler` component


Config Config
Expand Down Expand Up @@ -306,48 +304,38 @@ TwigBundle
``` ```


* Deprecated the `ExceptionController` and `PreviewErrorController` controllers, use `ErrorController` from the HttpKernel component instead * Deprecated the `ExceptionController` and `PreviewErrorController` controllers, use `ErrorController` from the HttpKernel component instead
* Deprecated all built-in error templates, use the error renderer mechanism of the `ErrorRenderer` component * Deprecated all built-in error templates, use the error renderer mechanism of the `ErrorHandler` component
* Deprecated loading custom error templates in non-html formats. Custom HTML error pages based on Twig keep working as before: * Deprecated loading custom error templates in non-html formats. Custom HTML error pages based on Twig keep working as before:


Before (`templates/bundles/TwigBundle/Exception/error.jsonld.twig`): Before (`templates/bundles/TwigBundle/Exception/error.json.twig`):
```twig ```twig
{ {
"@id": "https://example.com", "type": "https://example.com/error",
"@type": "error", "title": "{{ status_text }}",
"@context": { "status": {{ status_code }}
"title": "{{ status_text }}",
"code": {{ status_code }},
"message": "{{ exception.message }}"
}
} }
``` ```


After (`App\ErrorRenderer\JsonLdErrorRenderer`): After (`App\Serializer\ProblemJsonNormalizer`):
```php ```php
class JsonLdErrorRenderer implements ErrorRendererInterface class ProblemJsonNormalizer implements NormalizerInterface
{ {
public static function getFormat(): string public function normalize($exception, $format = null, array $context = [])
{ {
return 'jsonld'; return [
'type' => 'https://example.com/error',
'title' => $exception->getStatusText(),
'status' => $exception->getStatusCode(),
];
} }


public function render(FlattenException $exception): string public function supportsNormalization($data, $format = null)
{ {
return json_encode([ return 'json' === $format && $data instanceof FlattenException;
'@id' => 'https://example.com',
'@type' => 'error',
'@context' => [
'title' => $exception->getTitle(),
'code' => $exception->getStatusCode(),
'message' => $exception->getMessage(),
],
]);
} }
} }
``` ```


Configure your rendering service tagging it with `error_renderer.renderer`.

Validator Validator
--------- ---------


Expand Down
2 changes: 0 additions & 2 deletions UPGRADE-5.0.md
Expand Up @@ -57,8 +57,6 @@ Console
Debug Debug
----- -----


* Removed the `Debug` class, use the one from the `ErrorRenderer` component instead
* Removed the `FlattenException` class, use the one from the `ErrorRenderer` component instead
* Removed the component in favor of the `ErrorHandler` component * Removed the component in favor of the `ErrorHandler` component


DependencyInjection DependencyInjection
Expand Down
1 change: 0 additions & 1 deletion composer.json
Expand Up @@ -48,7 +48,6 @@
"symfony/dom-crawler": "self.version", "symfony/dom-crawler": "self.version",
"symfony/dotenv": "self.version", "symfony/dotenv": "self.version",
"symfony/error-handler": "self.version", "symfony/error-handler": "self.version",
"symfony/error-renderer": "self.version",
"symfony/event-dispatcher": "self.version", "symfony/event-dispatcher": "self.version",
"symfony/expression-language": "self.version", "symfony/expression-language": "self.version",
"symfony/filesystem": "self.version", "symfony/filesystem": "self.version",
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bridge/Twig/CHANGELOG.md
Expand Up @@ -4,6 +4,7 @@ CHANGELOG
4.4.0 4.4.0
----- -----


* added a new `TwigErrorRenderer` for `html` format, integrated with the `ErrorHandler` component
* marked all classes extending twig as `@final` * marked all classes extending twig as `@final`
* deprecated to pass `$rootDir` and `$fileLinkFormatter` as 5th and 6th argument respectively to the * deprecated to pass `$rootDir` and `$fileLinkFormatter` as 5th and 6th argument respectively to the
`DebugCommand::__construct()` method, swap the variables position. `DebugCommand::__construct()` method, swap the variables position.
Expand Down
Expand Up @@ -9,11 +9,11 @@
* file that was distributed with this source code. * file that was distributed with this source code.
*/ */


namespace Symfony\Bundle\TwigBundle\ErrorRenderer; namespace Symfony\Bridge\Twig\ErrorRenderer;


use Symfony\Component\ErrorRenderer\ErrorRenderer\ErrorRendererInterface; use Symfony\Component\ErrorHandler\ErrorRenderer\ErrorRendererInterface;
use Symfony\Component\ErrorRenderer\ErrorRenderer\HtmlErrorRenderer; use Symfony\Component\ErrorHandler\ErrorRenderer\HtmlErrorRenderer;
use Symfony\Component\ErrorRenderer\Exception\FlattenException; use Symfony\Component\ErrorHandler\Exception\FlattenException;
use Twig\Environment; use Twig\Environment;
use Twig\Error\LoaderError; use Twig\Error\LoaderError;
use Twig\Loader\ExistsLoaderInterface; use Twig\Loader\ExistsLoaderInterface;
Expand All @@ -24,50 +24,36 @@
* *
* @author Yonel Ceruto <yonelceruto@gmail.com> * @author Yonel Ceruto <yonelceruto@gmail.com>
*/ */
class TwigHtmlErrorRenderer implements ErrorRendererInterface class TwigErrorRenderer implements ErrorRendererInterface
{ {
private $twig; private $twig;
private $htmlErrorRenderer; private $fallbackErrorRenderer;
private $debug; private $debug;


public function __construct(Environment $twig, HtmlErrorRenderer $htmlErrorRenderer, bool $debug = false) public function __construct(Environment $twig, HtmlErrorRenderer $fallbackErrorRenderer = null, bool $debug = false)
{ {
$this->twig = $twig; $this->twig = $twig;
$this->htmlErrorRenderer = $htmlErrorRenderer; $this->fallbackErrorRenderer = $fallbackErrorRenderer ?? new HtmlErrorRenderer();
$this->debug = $debug; $this->debug = $debug;
} }


/** /**
* {@inheritdoc} * {@inheritdoc}
*/ */
public static function getFormat(): string public function render(\Throwable $exception): FlattenException
{ {
return 'html'; $exception = $this->fallbackErrorRenderer->render($exception);
}

/**
* {@inheritdoc}
*/
public function render(FlattenException $exception): string
{
$debug = $this->debug && ($exception->getHeaders()['X-Debug'] ?? true);

if ($debug) {
return $this->htmlErrorRenderer->render($exception);
}

$template = $this->findTemplate($exception->getStatusCode());


if (null === $template) { if ($this->debug || !$template = $this->findTemplate($exception->getStatusCode())) {
return $this->htmlErrorRenderer->render($exception); return $exception;
} }


return $this->twig->render($template, [ return $exception->setAsString($this->twig->render($template, [
'legacy' => false, // to be removed in 5.0 'legacy' => false, // to be removed in 5.0
'exception' => $exception, 'exception' => $exception,
'status_code' => $exception->getStatusCode(), 'status_code' => $exception->getStatusCode(),
'status_text' => $exception->getTitle(), 'status_text' => $exception->getStatusText(),
]); ]));
} }


private function findTemplate(int $statusCode): ?string private function findTemplate(int $statusCode): ?string
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/Twig/Mime/NotificationEmail.php
Expand Up @@ -11,7 +11,7 @@


namespace Symfony\Bridge\Twig\Mime; namespace Symfony\Bridge\Twig\Mime;


use Symfony\Component\ErrorRenderer\Exception\FlattenException; use Symfony\Component\ErrorHandler\Exception\FlattenException;
use Symfony\Component\Mime\Header\Headers; use Symfony\Component\Mime\Header\Headers;
use Symfony\Component\Mime\Part\AbstractPart; use Symfony\Component\Mime\Part\AbstractPart;
use Twig\Extra\CssInliner\CssInlinerExtension; use Twig\Extra\CssInliner\CssInlinerExtension;
Expand Down
Expand Up @@ -9,21 +9,21 @@
* file that was distributed with this source code. * file that was distributed with this source code.
*/ */


namespace Symfony\Bundle\TwigBundle\Tests\ErrorRenderer; namespace Symfony\Bridge\Twig\Tests\ErrorRenderer;


use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Symfony\Bundle\TwigBundle\ErrorRenderer\TwigHtmlErrorRenderer; use Symfony\Bridge\Twig\ErrorRenderer\TwigErrorRenderer;
use Symfony\Component\ErrorRenderer\ErrorRenderer\HtmlErrorRenderer; use Symfony\Component\ErrorHandler\ErrorRenderer\HtmlErrorRenderer;
use Symfony\Component\ErrorRenderer\Exception\FlattenException; use Symfony\Component\ErrorHandler\Exception\FlattenException;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Twig\Environment; use Twig\Environment;
use Twig\Loader\ArrayLoader; use Twig\Loader\ArrayLoader;


class TwigHtmlErrorRendererTest extends TestCase class TwigErrorRendererTest extends TestCase
{ {
public function testFallbackToNativeRendererIfDebugOn() public function testFallbackToNativeRendererIfDebugOn()
{ {
$exception = FlattenException::createFromThrowable(new \Exception()); $exception = new \Exception();


$twig = $this->createMock(Environment::class); $twig = $this->createMock(Environment::class);
$nativeRenderer = $this->createMock(HtmlErrorRenderer::class); $nativeRenderer = $this->createMock(HtmlErrorRenderer::class);
Expand All @@ -33,12 +33,12 @@ public function testFallbackToNativeRendererIfDebugOn()
->with($exception) ->with($exception)
; ;


(new TwigHtmlErrorRenderer($twig, $nativeRenderer, true))->render($exception); (new TwigErrorRenderer($twig, $nativeRenderer, true))->render(new \Exception());
} }


public function testFallbackToNativeRendererIfCustomTemplateNotFound() public function testFallbackToNativeRendererIfCustomTemplateNotFound()
{ {
$exception = FlattenException::createFromThrowable(new NotFoundHttpException()); $exception = new NotFoundHttpException();


$twig = new Environment(new ArrayLoader([])); $twig = new Environment(new ArrayLoader([]));


Expand All @@ -47,27 +47,19 @@ public function testFallbackToNativeRendererIfCustomTemplateNotFound()
->expects($this->once()) ->expects($this->once())
->method('render') ->method('render')
->with($exception) ->with($exception)
->willReturn(FlattenException::createFromThrowable($exception))
; ;


(new TwigHtmlErrorRenderer($twig, $nativeRenderer, false))->render($exception); (new TwigErrorRenderer($twig, $nativeRenderer, false))->render($exception);
} }


public function testRenderCustomErrorTemplate() public function testRenderCustomErrorTemplate()
{ {
$exception = FlattenException::createFromThrowable(new NotFoundHttpException());

$twig = new Environment(new ArrayLoader([ $twig = new Environment(new ArrayLoader([
'@Twig/Exception/error404.html.twig' => '<h1>Page Not Found</h1>', '@Twig/Exception/error404.html.twig' => '<h1>Page Not Found</h1>',
])); ]));
$exception = (new TwigErrorRenderer($twig))->render(new NotFoundHttpException());


$nativeRenderer = $this->createMock(HtmlErrorRenderer::class); $this->assertSame('<h1>Page Not Found</h1>', $exception->getAsString());
$nativeRenderer
->expects($this->never())
->method('render')
;

$content = (new TwigHtmlErrorRenderer($twig, $nativeRenderer, false))->render($exception);

$this->assertSame('<h1>Page Not Found</h1>', $content);
} }
} }
3 changes: 2 additions & 1 deletion src/Symfony/Bridge/Twig/composer.json
Expand Up @@ -24,10 +24,11 @@
"egulias/email-validator": "^2.1.10", "egulias/email-validator": "^2.1.10",
"symfony/asset": "^3.4|^4.0|^5.0", "symfony/asset": "^3.4|^4.0|^5.0",
"symfony/dependency-injection": "^3.4|^4.0|^5.0", "symfony/dependency-injection": "^3.4|^4.0|^5.0",
"symfony/error-handler": "^4.4|^5.0",
"symfony/finder": "^3.4|^4.0|^5.0", "symfony/finder": "^3.4|^4.0|^5.0",
"symfony/form": "^4.4|^5.0", "symfony/form": "^4.4|^5.0",
"symfony/http-foundation": "^4.3|^5.0", "symfony/http-foundation": "^4.3|^5.0",
"symfony/http-kernel": "^3.4|^4.0", "symfony/http-kernel": "^4.4",
"symfony/mime": "^4.3|^5.0", "symfony/mime": "^4.3|^5.0",
"symfony/polyfill-intl-icu": "~1.0", "symfony/polyfill-intl-icu": "~1.0",
"symfony/routing": "^3.4|^4.0|^5.0", "symfony/routing": "^3.4|^4.0|^5.0",
Expand Down
Expand Up @@ -32,7 +32,6 @@ class UnusedTagsPass implements CompilerPassInterface
'controller.service_arguments', 'controller.service_arguments',
'config_cache.resource_checker', 'config_cache.resource_checker',
'data_collector', 'data_collector',
'error_renderer.renderer',
'form.type', 'form.type',
'form.type_extension', 'form.type_extension',
'form.type_guesser', 'form.type_guesser',
Expand Down
2 changes: 0 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php
Expand Up @@ -33,7 +33,6 @@
use Symfony\Component\DependencyInjection\Compiler\RegisterReverseContainerPass; use Symfony\Component\DependencyInjection\Compiler\RegisterReverseContainerPass;
use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\ErrorHandler\ErrorHandler; use Symfony\Component\ErrorHandler\ErrorHandler;
use Symfony\Component\ErrorRenderer\DependencyInjection\ErrorRendererPass;
use Symfony\Component\EventDispatcher\DependencyInjection\RegisterListenersPass; use Symfony\Component\EventDispatcher\DependencyInjection\RegisterListenersPass;
use Symfony\Component\Form\DependencyInjection\FormPass; use Symfony\Component\Form\DependencyInjection\FormPass;
use Symfony\Component\HttpClient\DependencyInjection\HttpClientPass; use Symfony\Component\HttpClient\DependencyInjection\HttpClientPass;
Expand Down Expand Up @@ -92,7 +91,6 @@ public function build(ContainerBuilder $container)
KernelEvents::FINISH_REQUEST, KernelEvents::FINISH_REQUEST,
]; ];


$container->addCompilerPass(new ErrorRendererPass());
$container->addCompilerPass(new LoggerPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, -32); $container->addCompilerPass(new LoggerPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, -32);
$container->addCompilerPass(new RegisterControllerArgumentLocatorsPass()); $container->addCompilerPass(new RegisterControllerArgumentLocatorsPass());
$container->addCompilerPass(new RemoveEmptyControllerArgumentLocatorsPass(), PassConfig::TYPE_BEFORE_REMOVING); $container->addCompilerPass(new RemoveEmptyControllerArgumentLocatorsPass(), PassConfig::TYPE_BEFORE_REMOVING);
Expand Down
Expand Up @@ -194,12 +194,6 @@
<tag name="console.command" command="debug:form" /> <tag name="console.command" command="debug:form" />
</service> </service>


<service id="console.command.error_renderer_debug" class="Symfony\Component\ErrorRenderer\Command\DebugCommand">
<argument type="collection" /> <!-- All error renderers are injected here by ErrorRendererPass -->
<argument type="service" id="debug.file_link_formatter" on-invalid="null" />
<tag name="console.command" command="debug:error-renderer" />
</service>

<service id="console.command.secrets_set" class="Symfony\Bundle\FrameworkBundle\Command\SecretsSetCommand"> <service id="console.command.secrets_set" class="Symfony\Bundle\FrameworkBundle\Command\SecretsSetCommand">
<argument type="service" id="secrets.vault" /> <argument type="service" id="secrets.vault" />
<argument type="service" id="secrets.local_vault" on-invalid="ignore" /> <argument type="service" id="secrets.local_vault" on-invalid="ignore" />
Expand Down
Expand Up @@ -5,12 +5,7 @@
xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd"> xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">


<services> <services>
<service id="error_renderer" class="Symfony\Component\ErrorRenderer\DependencyInjection\LazyLoadingErrorRenderer"> <service id="error_handler.error_renderer.html" class="Symfony\Component\ErrorHandler\ErrorRenderer\HtmlErrorRenderer">
<argument /> <!-- error renderer locator -->
</service>

<service id="error_renderer.renderer.html" class="Symfony\Component\ErrorRenderer\ErrorRenderer\HtmlErrorRenderer">
<tag name="error_renderer.renderer" />
<argument>%kernel.debug%</argument> <argument>%kernel.debug%</argument>
<argument>%kernel.charset%</argument> <argument>%kernel.charset%</argument>
<argument type="service" id="debug.file_link_formatter" on-invalid="null" /> <argument type="service" id="debug.file_link_formatter" on-invalid="null" />
Expand All @@ -19,21 +14,19 @@
<argument type="service" id="logger" on-invalid="null" /> <argument type="service" id="logger" on-invalid="null" />
</service> </service>


<service id="error_renderer.renderer.json" class="Symfony\Component\ErrorRenderer\ErrorRenderer\JsonErrorRenderer"> <service id="error_handler.error_renderer.serializer" class="Symfony\Component\ErrorHandler\ErrorRenderer\SerializerErrorRenderer">
<tag name="error_renderer.renderer" /> <argument type="service" id="serializer" />
<argument>%kernel.debug%</argument> <argument type="service">
<service>
<factory class="Symfony\Component\ErrorHandler\ErrorRenderer\SerializerErrorRenderer" method="getPreferredFormat" />
<argument type="service" id="request_stack" />
</service>
</argument>
<argument type="service" id="error_renderer.html" />
</service> </service>


<service id="error_renderer.renderer.xml" class="Symfony\Component\ErrorRenderer\ErrorRenderer\XmlErrorRenderer"> <service id="error_renderer.html" alias="error_handler.error_renderer.html" />
<tag name="error_renderer.renderer" format="atom" /> <service id="error_renderer.serializer" alias="error_handler.error_renderer.serializer" />
<tag name="error_renderer.renderer" /> <service id="error_renderer" alias="error_renderer.html" />
<argument>%kernel.debug%</argument>
<argument>%kernel.charset%</argument>
</service>

<service id="error_renderer.renderer.txt" class="Symfony\Component\ErrorRenderer\ErrorRenderer\TxtErrorRenderer">
<tag name="error_renderer.renderer" />
<argument>%kernel.debug%</argument>
</service>
</services> </services>
</container> </container>
Expand Up @@ -12,6 +12,8 @@
<services> <services>
<defaults public="false" /> <defaults public="false" />


<service id="error_renderer" alias="error_renderer.serializer" />

<service id="serializer" class="Symfony\Component\Serializer\Serializer" public="true"> <service id="serializer" class="Symfony\Component\Serializer\Serializer" public="true">
<argument type="collection" /> <argument type="collection" />
<argument type="collection" /> <argument type="collection" />
Expand Down Expand Up @@ -59,6 +61,12 @@
<tag name="serializer.normalizer" priority="-900" /> <tag name="serializer.normalizer" priority="-900" />
</service> </service>


<service id="serializer.normalizer.problem" class="Symfony\Component\Serializer\Normalizer\ProblemNormalizer">
<argument>%kernel.debug%</argument>
<!-- Run before serializer.normalizer.object -->
<tag name="serializer.normalizer" priority="-890" />
</service>

<service id="serializer.normalizer.object" class="Symfony\Component\Serializer\Normalizer\ObjectNormalizer"> <service id="serializer.normalizer.object" class="Symfony\Component\Serializer\Normalizer\ObjectNormalizer">
<argument type="service" id="serializer.mapping.class_metadata_factory" /> <argument type="service" id="serializer.mapping.class_metadata_factory" />
<argument type="service" id="serializer.name_converter.metadata_aware" /> <argument type="service" id="serializer.name_converter.metadata_aware" />
Expand Down
3 changes: 1 addition & 2 deletions src/Symfony/Bundle/FrameworkBundle/composer.json
Expand Up @@ -21,7 +21,6 @@
"symfony/cache": "^4.4|^5.0", "symfony/cache": "^4.4|^5.0",
"symfony/config": "^4.3.4|^5.0", "symfony/config": "^4.3.4|^5.0",
"symfony/dependency-injection": "^4.4|^5.0", "symfony/dependency-injection": "^4.4|^5.0",
"symfony/error-renderer": "^4.4|^5.0",
"symfony/http-foundation": "^4.4|^5.0", "symfony/http-foundation": "^4.4|^5.0",
"symfony/http-kernel": "^4.4", "symfony/http-kernel": "^4.4",
"symfony/polyfill-mbstring": "~1.0", "symfony/polyfill-mbstring": "~1.0",
Expand Down Expand Up @@ -50,7 +49,7 @@
"symfony/process": "^3.4|^4.0|^5.0", "symfony/process": "^3.4|^4.0|^5.0",
"symfony/security-csrf": "^3.4|^4.0|^5.0", "symfony/security-csrf": "^3.4|^4.0|^5.0",
"symfony/security-http": "^3.4|^4.0|^5.0", "symfony/security-http": "^3.4|^4.0|^5.0",
"symfony/serializer": "^4.3|^5.0", "symfony/serializer": "^4.4|^5.0",
"symfony/stopwatch": "^3.4|^4.0|^5.0", "symfony/stopwatch": "^3.4|^4.0|^5.0",
"symfony/translation": "^4.4|^5.0", "symfony/translation": "^4.4|^5.0",
"symfony/templating": "^3.4|^4.0|^5.0", "symfony/templating": "^3.4|^4.0|^5.0",
Expand Down

0 comments on commit 0b867be

Please sign in to comment.