[ErrorHandler] help finish the PR

symfony · fabpot · Nov 12, 2019 · Nov 10, 2019 · Nov 10, 2019 · Nov 12, 2019
commit d1bf1cada41e7e381237982fc282fc1e3df34c7b
diff --git a/UPGRADE-4.4.md b/UPGRADE-4.4.md
@@ -18,7 +18,6 @@ Console
 Debug
 -----
 
- * Deprecated the `FlattenException` class, use the one from the `ErrorRenderer` component instead
  * Deprecated the component in favor of the `ErrorHandler` component
 
 Config
@@ -305,7 +304,7 @@ TwigBundle
    ```
 
  * 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:
 
    Before (`templates/bundles/TwigBundle/Exception/error.json.twig`):

diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md
@@ -57,7 +57,6 @@ Console
 Debug
 -----
 
- * Removed the `FlattenException` class, use the one from the `ErrorRenderer` component instead
  * Removed the component in favor of the `ErrorHandler` component
 
 DependencyInjection

diff --git a/src/Symfony/Bridge/Twig/CHANGELOG.md b/src/Symfony/Bridge/Twig/CHANGELOG.md
@@ -4,6 +4,7 @@ CHANGELOG
 4.4.0
 -----
 
+ * added a new `TwigErrorRenderer` for `html` format, integrated with the `ErrorHandler` component
  * marked all classes extending twig as `@final`
  * deprecated to pass `$rootDir` and `$fileLinkFormatter` as 5th and 6th argument respectively to the 
    `DebugCommand::__construct()` method, swap the variables position.

diff --git a/...e/ErrorRenderer/TwigHtmlErrorRenderer.php → .../Twig/ErrorRenderer/TwigErrorRenderer.php b/...e/ErrorRenderer/TwigHtmlErrorRenderer.php → .../Twig/ErrorRenderer/TwigErrorRenderer.php
@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Symfony\Bundle\TwigBundle\ErrorRenderer;
+namespace Symfony\Bridge\Twig\ErrorRenderer;
 
 use Symfony\Component\ErrorHandler\ErrorRenderer\ErrorRendererInterface;
 use Symfony\Component\ErrorHandler\ErrorRenderer\HtmlErrorRenderer;
@@ -24,16 +24,16 @@
  *
  * @author Yonel Ceruto <yonelceruto@gmail.com>
  */
-class TwigHtmlErrorRenderer implements ErrorRendererInterface
+class TwigErrorRenderer implements ErrorRendererInterface
 {
     private $twig;
-    private $htmlErrorRenderer;
+    private $fallbackErrorRenderer;
     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->htmlErrorRenderer = $htmlErrorRenderer;
+        $this->fallbackErrorRenderer = $fallbackErrorRenderer ?? new HtmlErrorRenderer();
         $this->debug = $debug;
     }
 
@@ -42,9 +42,9 @@ public function __construct(Environment $twig, HtmlErrorRenderer $htmlErrorRende
      */
     public function render(\Throwable $exception): FlattenException
     {
-        $exception = $this->htmlErrorRenderer->render($exception);
+        $exception = $this->fallbackErrorRenderer->render($exception);
 
-        if ($this->debug || !$template = $this->findTemplate($exception->getStatusCode());
+        if ($this->debug || !$template = $this->findTemplate($exception->getStatusCode())) {
             return $exception;
         }
 

diff --git a/...rorRenderer/TwigHtmlErrorRendererTest.php → ...s/ErrorRenderer/TwigErrorRendererTest.php b/...rorRenderer/TwigHtmlErrorRendererTest.php → ...s/ErrorRenderer/TwigErrorRendererTest.php
@@ -9,16 +9,17 @@
  * 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 Symfony\Bundle\TwigBundle\ErrorRenderer\TwigHtmlErrorRenderer;
+use Symfony\Bridge\Twig\ErrorRenderer\TwigErrorRenderer;
 use Symfony\Component\ErrorHandler\ErrorRenderer\HtmlErrorRenderer;
+use Symfony\Component\ErrorHandler\Exception\FlattenException;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Twig\Environment;
 use Twig\Loader\ArrayLoader;
 
-class TwigHtmlErrorRendererTest extends TestCase
+class TwigErrorRendererTest extends TestCase
 {
     public function testFallbackToNativeRendererIfDebugOn()
     {
@@ -32,7 +33,7 @@ public function testFallbackToNativeRendererIfDebugOn()
             ->with($exception)
         ;
 
-        (new TwigHtmlErrorRenderer($twig, $nativeRenderer, true))->render(new \Exception());
+        (new TwigErrorRenderer($twig, $nativeRenderer, true))->render(new \Exception());
     }
 
     public function testFallbackToNativeRendererIfCustomTemplateNotFound()
@@ -46,27 +47,19 @@ public function testFallbackToNativeRendererIfCustomTemplateNotFound()
             ->expects($this->once())
             ->method('render')
             ->with($exception)
+            ->willReturn(FlattenException::createFromThrowable($exception))
         ;
 
-        (new TwigHtmlErrorRenderer($twig, $nativeRenderer, false))->render($exception);
+        (new TwigErrorRenderer($twig, $nativeRenderer, false))->render($exception);
     }
 
     public function testRenderCustomErrorTemplate()
     {
-        $exception = new NotFoundHttpException();
-
         $twig = new Environment(new ArrayLoader([
             '@Twig/Exception/error404.html.twig' => '<h1>Page Not Found</h1>',
         ]));
+        $exception = (new TwigErrorRenderer($twig))->render(new NotFoundHttpException());
 
-        $nativeRenderer = $this->createMock(HtmlErrorRenderer::class);
-        $nativeRenderer
-            ->expects($this->never())
-            ->method('render')
-        ;
-
-        $content = (new TwigHtmlErrorRenderer($twig, $nativeRenderer, false))->render($exception);
-
-        $this->assertSame('<h1>Page Not Found</h1>', $content->getAsString());
+        $this->assertSame('<h1>Page Not Found</h1>', $exception->getAsString());
     }
 }
diff --git a/src/Symfony/Bridge/Twig/composer.json b/src/Symfony/Bridge/Twig/composer.json
@@ -24,10 +24,11 @@
         "egulias/email-validator": "^2.1.10",
         "symfony/asset": "^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/form": "^4.4|^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/polyfill-intl-icu": "~1.0",
         "symfony/routing": "^3.4|^4.0|^5.0",

diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/error_renderer.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/error_renderer.xml
@@ -16,9 +16,13 @@
 
         <service id="error_handler.error_renderer.serializer" class="Symfony\Component\ErrorHandler\ErrorRenderer\SerializerErrorRenderer">
             <argument type="service" id="serializer" />
-            <argument type="service" id="request_stack" />
+            <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" />
-            <argument>%kernel.debug%</argument>
         </service>
 
         <service id="error_renderer.html" alias="error_handler.error_renderer.html" />

@@ -6,7 +6,6 @@ CHANGELOG
 
  * marked the `TemplateIterator` as `internal`
  * added HTML comment to beginning and end of `exception_full.html.twig`
- * added a new `TwigHtmlErrorRenderer` for `html` format, integrated with the `ErrorHandler` component
  * deprecated `ExceptionController` and `PreviewErrorController` controllers, use `ErrorController` from the `HttpKernel` component instead
  * deprecated all built-in error templates in favor of the new error renderer mechanism
  * deprecated `twig.exception_controller` configuration option, set it to "null" and use `framework.error_controller` configuration instead

@@ -162,7 +162,7 @@
             <argument /> <!-- runtime locator -->
         </service>
 
-        <service id="twig.error_renderer.html" class="Symfony\Bundle\TwigBundle\ErrorRenderer\TwigHtmlErrorRenderer" decorates="error_renderer.html">
+        <service id="twig.error_renderer.html" class="Symfony\Bridge\Twig\ErrorRenderer\TwigErrorRenderer" decorates="error_renderer.html">
             <argument type="service" id="twig" />
             <argument type="service" id="twig.error_renderer.html.inner" />
             <argument>%kernel.debug%</argument>

@@ -23,6 +23,7 @@
 use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;
 use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
 use Symfony\Component\DependencyInjection\Reference;
+use Symfony\Component\ErrorHandler\ErrorRenderer\HtmlErrorRenderer;
 
 class TwigExtensionTest extends TestCase
 {
@@ -302,6 +303,7 @@ public function testRuntimeLoader()
         $container->register('templating.locator', 'FooClass');
         $container->register('templating.name_parser', 'FooClass');
         $container->register('foo', '%foo%')->addTag('twig.runtime');
+        $container->register('error_renderer.html', HtmlErrorRenderer::class);
         $container->addCompilerPass(new RuntimeLoaderPass(), PassConfig::TYPE_BEFORE_REMOVING);
         $container->getCompilerPassConfig()->setRemovingPasses([]);
         $container->getCompilerPassConfig()->setAfterRemovingPasses([]);

diff --git a/src/Symfony/Bundle/WebProfilerBundle/Tests/DependencyInjection/WebProfilerExtensionTest.php b/src/Symfony/Bundle/WebProfilerBundle/Tests/DependencyInjection/WebProfilerExtensionTest.php
@@ -54,7 +54,7 @@ protected function setUp(): void
         $this->kernel = $this->getMockBuilder('Symfony\\Component\\HttpKernel\\KernelInterface')->getMock();
 
         $this->container = new ContainerBuilder();
-        $this->container->register('error_renderer.html', HtmlErrorRenderer::class)->setPublic(true);
+        $this->container->register('error_handler.error_renderer.html', HtmlErrorRenderer::class)->setPublic(true);
         $this->container->register('event_dispatcher', EventDispatcher::class)->setPublic(true);
         $this->container->register('router', $this->getMockClass('Symfony\\Component\\Routing\\RouterInterface'))->setPublic(true);
         $this->container->register('twig', 'Twig\Environment')->setPublic(true);

@@ -21,7 +21,6 @@
 use Symfony\Component\ErrorHandler\ErrorEnhancer\UndefinedMethodErrorEnhancer;
 use Symfony\Component\ErrorHandler\ErrorRenderer\CliErrorRenderer;
 use Symfony\Component\ErrorHandler\ErrorRenderer\HtmlErrorRenderer;
-use Symfony\Component\ErrorHandler\Exception\FlattenException;
 use Symfony\Component\ErrorHandler\Exception\SilencedErrorContext;
 
 /**
@@ -580,7 +579,12 @@ public function handleException(\Throwable $exception)
         }
 
         $exceptionHandler = $this->exceptionHandler;
-        $this->exceptionHandler = null;
+        $this->exceptionHandler = [$this, 'renderException'];
+
+        if (null === $exceptionHandler || $exceptionHandler === $this->exceptionHandler) {
+            $this->exceptionHandler = null;
+        }
+
         try {
             if (null !== $exceptionHandler) {
                 return $exceptionHandler($exception);
@@ -593,7 +597,14 @@ public function handleException(\Throwable $exception)
             throw $exception; // Give back $exception to the native handler
         }
 
-        $this->handleException($handlerException);
+        $loggedErrors = $this->loggedErrors;
+        $this->loggedErrors = $exception === $handlerException ? 0 : $this->loggedErrors;
+
+        try {
+            $this->handleException($handlerException);
+        } finally {
+            $this->loggedErrors = $loggedErrors;
+        }
     }
 
     /**

@@ -30,7 +30,7 @@ public function render(\Throwable $exception): FlattenException
             protected function supportsColors(): bool
             {
                 $outputStream = $this->outputStream;
-                $this->outputStream = STDOUT;
+                $this->outputStream = fopen('php://stdout', 'w');
 
                 try {
                     return parent::supportsColors();

@@ -102,13 +102,13 @@ private function renderException(FlattenException $exception, string $debugTempl
             'statusText' => $statusText,
             'statusCode' => $statusCode,
             'logger' => $this->logger instanceof DebugLoggerInterface ? $this->logger : null,
-            'currentContent' => $request ? $this->getAndCleanOutputBuffering($request->headers->get('X-Php-Ob-Level')) : '',
+            'currentContent' => $request ? $this->getAndCleanOutputBuffering($request->headers->get('X-Php-Ob-Level', -1)) : '',
         ]);
     }
 
-    private function getAndCleanOutputBuffering(?int $startObLevel): string
+    private function getAndCleanOutputBuffering(int $startObLevel): string
     {
-        if (null === $startObLevel || ob_get_level() <= $startObLevel) {
+        if (ob_get_level() <= $startObLevel) {
             return '';
         }
 

@@ -11,7 +11,8 @@
 
 namespace Symfony\Component\ErrorHandler\ErrorRenderer;
 
-use Symfony\Component\ErrorRenderer\Exception\FlattenException;
+use Symfony\Component\ErrorHandler\Exception\FlattenException;
+use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\Serializer\Exception\NotEncodableValueException;
 use Symfony\Component\Serializer\SerializerInterface;
 
@@ -20,31 +21,50 @@
  *
  * @author Nicolas Grekas <p@tchwork.com>
  */
-class SerializerErrorRenderer
+class SerializerErrorRenderer implements ErrorRendererInterface
 {
     private $serializer;
-    private $requestStack;
-    private $debug;
+    private $format;
+    private $fallbackErrorRenderer;
 
-    public function __construct(SerializerInterface $serializer, RequestStack $requestStack, bool $debug = true)
+    /**
+     * @param string|callable(FlattenException) $format The format as a string or a callable that should return it
+     */
+    public function __construct(SerializerInterface $serializer, $format, ErrorRendererInterface $fallbackErrorRenderer = null)
     {
+        if (!\is_string($format) && !\is_callable($format)) {
+            throw new \TypeError(sprintf('Argument 2 passed to %s() must be a string or a callable, %s given.', __METHOD__, \is_object($format) ? \get_class($format) : \gettype($format)));
+        }
+
         $this->serializer = $serializer;
-        $this->requestStack = $requestStack;
-        $this->debug = $debug;
+        $this->format = $format;
+        $this->fallbackErrorRenderer = $fallbackErrorRenderer ?? new HtmlErrorRenderer();
     }
 
     /**
      * {@inheritdoc}
      */
     public function render(\Throwable $exception): FlattenException
     {
-        $format = $this->requestStack->getCurrentRequest()->getPreferredFormat();
         $flattenException = FlattenException::createFromThrowable($exception);
 
         try {
+            $format = \is_string($this->format) ? $this->format : ($this->format)($flattenException);
+
             return $flattenException->setAsString($this->serializer->serialize($flattenException, $format, ['exception' => $exception]));
-        } catch (NotEncodableValueException $_) {
-            return (new HtmlErrorHandler($this->debug))->render($exception);
+        } catch (NotEncodableValueException $e) {
+            return $this->fallbackErrorRenderer->render($exception);
         }
     }
+
+    public static function getPreferredFormat(RequestStack $requestStack): \Closure
+    {
+        return static function () use ($requestStack) {
+            if (!$request = $requestStack->getCurrentRequest()) {
+                throw new NotEncodableValueException();
+            }
+
+            return $request->getPreferredFormat();
+        };
+    }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -57,7 +57,6 @@ Console @@
     Debug
     -----
-     * Removed the `FlattenException` class, use the one from the `ErrorRenderer` component instead
      * Removed the component in favor of the `ErrorHandler` component
     DependencyInjection
@@ Expand Down @@