Skip to content

Commit 7f7312f

Browse files
ycerutonicolas-grekas
yceruto
authored and
nicolas-grekas
committed
[ErrorHandler] help finish the PR
1 parent 6c9157b commit 7f7312f

File tree

17 files changed

+108
-111
lines changed

17 files changed

+108
-111
lines changed

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,13 @@
1616

1717
<service id="error_handler.error_renderer.serializer" class="Symfony\Component\ErrorHandler\ErrorRenderer\SerializerErrorRenderer">
1818
<argument type="service" id="serializer" />
19-
<argument type="service" id="request_stack" />
19+
<argument type="service">
20+
<service>
21+
<factory class="Symfony\Component\ErrorHandler\ErrorRenderer\SerializerErrorRenderer" method="getPreferredFormat" />
22+
<argument type="service" id="request_stack" />
23+
</service>
24+
</argument>
2025
<argument type="service" id="error_renderer.html" />
21-
<argument>%kernel.debug%</argument>
2226
</service>
2327

2428
<service id="error_renderer.html" alias="error_handler.error_renderer.html" />

‎src/Symfony/Bundle/TwigBundle/ErrorRenderer/TwigHtmlErrorRenderer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public function render(\Throwable $exception): FlattenException
4444
{
4545
$exception = $this->htmlErrorRenderer->render($exception);
4646

47-
if ($this->debug || !$template = $this->findTemplate($exception->getStatusCode());
47+
if ($this->debug || !$template = $this->findTemplate($exception->getStatusCode())) {
4848
return $exception;
4949
}
5050

‎src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/TwigExtensionTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;
2424
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
2525
use Symfony\Component\DependencyInjection\Reference;
26+
use Symfony\Component\ErrorHandler\ErrorRenderer\HtmlErrorRenderer;
2627

2728
class TwigExtensionTest extends TestCase
2829
{
@@ -302,6 +303,7 @@ public function testRuntimeLoader()
302303
$container->register('templating.locator', 'FooClass');
303304
$container->register('templating.name_parser', 'FooClass');
304305
$container->register('foo', '%foo%')->addTag('twig.runtime');
306+
$container->register('error_renderer.html', HtmlErrorRenderer::class);
305307
$container->addCompilerPass(new RuntimeLoaderPass(), PassConfig::TYPE_BEFORE_REMOVING);
306308
$container->getCompilerPassConfig()->setRemovingPasses([]);
307309
$container->getCompilerPassConfig()->setAfterRemovingPasses([]);

‎src/Symfony/Bundle/TwigBundle/Tests/ErrorRenderer/TwigHtmlErrorRendererTest.php‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Bundle\TwigBundle\ErrorRenderer\TwigHtmlErrorRenderer;
1616
use Symfony\Component\ErrorHandler\ErrorRenderer\HtmlErrorRenderer;
17+
use Symfony\Component\ErrorHandler\Exception\FlattenException;
1718
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
1819
use Twig\Environment;
1920
use Twig\Loader\ArrayLoader;
@@ -46,27 +47,19 @@ public function testFallbackToNativeRendererIfCustomTemplateNotFound()
4647
->expects($this->once())
4748
->method('render')
4849
->with($exception)
50+
->willReturn(FlattenException::createFromThrowable($exception))
4951
;
5052

5153
(new TwigHtmlErrorRenderer($twig, $nativeRenderer, false))->render($exception);
5254
}
5355

5456
public function testRenderCustomErrorTemplate()
5557
{
56-
$exception = new NotFoundHttpException();
57-
5858
$twig = new Environment(new ArrayLoader([
5959
'@Twig/Exception/error404.html.twig' => '<h1>Page Not Found</h1>',
6060
]));
61+
$exception = (new TwigHtmlErrorRenderer($twig, new HtmlErrorRenderer()))->render(new NotFoundHttpException());
6162

62-
$nativeRenderer = $this->createMock(HtmlErrorRenderer::class);
63-
$nativeRenderer
64-
->expects($this->never())
65-
->method('render')
66-
;
67-
68-
$content = (new TwigHtmlErrorRenderer($twig, $nativeRenderer, false))->render($exception);
69-
70-
$this->assertSame('<h1>Page Not Found</h1>', $content->getAsString());
63+
$this->assertSame('<h1>Page Not Found</h1>', $exception->getAsString());
7164
}
7265
}

‎src/Symfony/Bundle/WebProfilerBundle/Tests/DependencyInjection/WebProfilerExtensionTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ protected function setUp(): void
5454
$this->kernel = $this->getMockBuilder('Symfony\\Component\\HttpKernel\\KernelInterface')->getMock();
5555

5656
$this->container = new ContainerBuilder();
57-
$this->container->register('error_renderer.html', HtmlErrorRenderer::class)->setPublic(true);
57+
$this->container->register('error_handler.error_renderer.html', HtmlErrorRenderer::class)->setPublic(true);
5858
$this->container->register('event_dispatcher', EventDispatcher::class)->setPublic(true);
5959
$this->container->register('router', $this->getMockClass('Symfony\\Component\\Routing\\RouterInterface'))->setPublic(true);
6060
$this->container->register('twig', 'Twig\Environment')->setPublic(true);

‎src/Symfony/Component/ErrorHandler/ErrorHandler.php‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
use Symfony\Component\ErrorHandler\ErrorEnhancer\UndefinedMethodErrorEnhancer;
2222
use Symfony\Component\ErrorHandler\ErrorRenderer\CliErrorRenderer;
2323
use Symfony\Component\ErrorHandler\ErrorRenderer\HtmlErrorRenderer;
24-
use Symfony\Component\ErrorHandler\Exception\FlattenException;
2524
use Symfony\Component\ErrorHandler\Exception\SilencedErrorContext;
2625

2726
/**
@@ -580,7 +579,12 @@ public function handleException(\Throwable $exception)
580579
}
581580

582581
$exceptionHandler = $this->exceptionHandler;
583-
$this->exceptionHandler = null;
582+
$this->exceptionHandler = [$this, 'renderException'];
583+
584+
if (null === $exceptionHandler || $exceptionHandler === $this->exceptionHandler){
585+
$this->exceptionHandler = null;
586+
}
587+
584588
try {
585589
if (null !== $exceptionHandler) {
586590
return $exceptionHandler($exception);
@@ -593,7 +597,14 @@ public function handleException(\Throwable $exception)
593597
throw $exception; // Give back $exception to the native handler
594598
}
595599

596-
$this->handleException($handlerException);
600+
$loggedErrors = $this->loggedErrors;
601+
$this->loggedErrors = $exception === $handlerException ? 0 : $this->loggedErrors;
602+
603+
try {
604+
$this->handleException($handlerException);
605+
} finally {
606+
$this->loggedErrors = $loggedErrors;
607+
}
597608
}
598609

599610
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public function render(\Throwable $exception): FlattenException
3030
protected function supportsColors(): bool
3131
{
3232
$outputStream = $this->outputStream;
33-
$this->outputStream = STDOUT;
33+
$this->outputStream = fopen('php://stdout', 'w');
3434

3535
try {
3636
return parent::supportsColors();

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,13 +102,13 @@ private function renderException(FlattenException $exception, string $debugTempl
102102
'statusText' => $statusText,
103103
'statusCode' => $statusCode,
104104
'logger' => $this->logger instanceof DebugLoggerInterface ? $this->logger : null,
105-
'currentContent' => $request ? $this->getAndCleanOutputBuffering($request->headers->get('X-Php-Ob-Level')) : '',
105+
'currentContent' => $request ? $this->getAndCleanOutputBuffering($request->headers->get('X-Php-Ob-Level', -1)) : '',
106106
]);
107107
}
108108

109-
private function getAndCleanOutputBuffering(?int $startObLevel): string
109+
private function getAndCleanOutputBuffering(int $startObLevel): string
110110
{
111-
if (null === $startObLevel || ob_get_level() <= $startObLevel) {
111+
if (ob_get_level() <= $startObLevel) {
112112
return '';
113113
}
114114

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

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111

1212
namespace Symfony\Component\ErrorHandler\ErrorRenderer;
1313

14-
use Symfony\Component\ErrorRenderer\Exception\FlattenException;
14+
use Symfony\Component\ErrorHandler\Exception\FlattenException;
15+
use Symfony\Component\HttpFoundation\RequestStack;
1516
use Symfony\Component\Serializer\Exception\NotEncodableValueException;
1617
use Symfony\Component\Serializer\SerializerInterface;
1718

@@ -20,31 +21,50 @@
2021
*
2122
* @author Nicolas Grekas <p@tchwork.com>
2223
*/
23-
class SerializerErrorRenderer
24+
class SerializerErrorRenderer implements ErrorRendererInterface
2425
{
2526
private $serializer;
26-
private $requestStack;
27-
private $debug;
27+
private $format;
28+
private $fallbackErrorRenderer;
2829

29-
public function __construct(SerializerInterface $serializer, RequestStack $requestStack, bool $debug = true)
30+
/**
31+
* @param string|callable $format The format as a string or a callable that should return it
32+
*/
33+
public function __construct(SerializerInterface $serializer, $format, ErrorRendererInterface $fallbackErrorRenderer = null)
3034
{
35+
if (!\is_string($format) && !\is_callable($format)) {
36+
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)));
37+
}
38+
3139
$this->serializer = $serializer;
32-
$this->requestStack = $requestStack;
33-
$this->debug = $debug;
40+
$this->format = $format;
41+
$this->fallbackErrorRenderer = $fallbackErrorRenderer ?? new HtmlErrorRenderer();
3442
}
3543

3644
/**
3745
* {@inheritdoc}
3846
*/
3947
public function render(\Throwable $exception): FlattenException
4048
{
41-
$format = $this->requestStack->getCurrentRequest()->getPreferredFormat();
4249
$flattenException = FlattenException::createFromThrowable($exception);
4350

4451
try {
52+
$format = \is_string($this->format) ? $this->format : ($this->format)();
53+
4554
return $flattenException->setAsString($this->serializer->serialize($flattenException, $format, ['exception' => $exception]));
46-
} catch (NotEncodableValueException $_) {
47-
return (new HtmlErrorHandler($this->debug))->render($exception);
55+
} catch (NotEncodableValueException $e) {
56+
return $this->fallbackErrorRenderer->render($exception);
4857
}
4958
}
59+
60+
public static function getPreferredFormat(RequestStack $requestStack): \Closure
61+
{
62+
return static function () use ($requestStack) {
63+
if (!$request = $requestStack->getCurrentRequest()) {
64+
throw new NotEncodableValueException();
65+
}
66+
67+
return $request->getPreferredFormat();
68+
};
69+
}
5070
}

‎src/Symfony/Component/ErrorHandler/Tests/ErrorHandlerTest.php‎

Lines changed: 11 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -50,23 +50,13 @@ public function testRegister()
5050
$h = set_error_handler('var_dump');
5151
restore_error_handler();
5252
$this->assertSame([$newHandler, 'handleError'], $h);
53-
} catch (\Exception $e) {
53+
} finally {
54+
restore_error_handler();
55+
restore_exception_handler();
5456
}
55-
57+
} finally {
5658
restore_error_handler();
5759
restore_exception_handler();
58-
59-
if (isset($e)) {
60-
throw $e;
61-
}
62-
} catch (\Exception $e) {
63-
}
64-
65-
restore_error_handler();
66-
restore_exception_handler();
67-
68-
if (isset($e)) {
69-
throw $e;
7060
}
7161
}
7262

@@ -86,11 +76,9 @@ public function testErrorGetLast()
8676
'line' => __LINE__ - 5,
8777
];
8878
$this->assertSame($expected, error_get_last());
89-
} catch (\Exception $e) {
79+
} finally {
9080
restore_error_handler();
9181
restore_exception_handler();
92-
93-
throw $e;
9482
}
9583
}
9684

@@ -323,14 +311,9 @@ public function testHandleError()
323311
unset($undefVar);
324312
$line = __LINE__ + 1;
325313
@$undefVar++;
326-
327-
restore_error_handler();
328-
restore_exception_handler();
329-
} catch (\Exception $e) {
314+
} finally {
330315
restore_error_handler();
331316
restore_exception_handler();
332-
333-
throw $e;
334317
}
335318
}
336319

@@ -406,6 +389,7 @@ public function testHandleException(string $expectedMessage, \Throwable $excepti
406389
;
407390

408391
$handler->setDefaultLogger($logger, E_ERROR);
392+
$handler->setExceptionHandler(null);
409393

410394
try {
411395
$handler->handleException($exception);
@@ -530,16 +514,12 @@ public function testHandleFatalError()
530514
;
531515

532516
$handler->setDefaultLogger($logger, E_PARSE);
517+
$handler->setExceptionHandler(null);
533518

534519
$handler->handleFatalError($error);
535-
536-
restore_error_handler();
537-
restore_exception_handler();
538-
} catch (\Exception $e) {
520+
} finally {
539521
restore_error_handler();
540522
restore_exception_handler();
541-
542-
throw $e;
543523
}
544524
}
545525

@@ -563,6 +543,7 @@ public function testCustomExceptionHandler()
563543
$this->expectException('Exception');
564544
$handler = new ErrorHandler();
565545
$handler->setExceptionHandler(function ($e) use ($handler) {
546+
$handler->setExceptionHandler(null);
566547
$handler->handleException($e);
567548
});
568549

@@ -572,7 +553,7 @@ public function testCustomExceptionHandler()
572553
public function testSendPhpResponse()
573554
{
574555
$handler = new ErrorHandler();
575-
$handler->setExceptionHandler([$handler, 'sendPhpResponse']);
556+
$handler->setExceptionHandler([$handler, 'renderException']);
576557

577558
ob_start();
578559
$handler->handleException(new \RuntimeException('Class Foo not found'));

0 commit comments

Comments
 (0)