[HttpKernel] Add request attribute _handle_all_throwables to allow HttpKernel to handle thrown Error in addition to Exception

nicolas-grekas · nicolas-grekas · commit bfbc8f7c0efb · 2022-10-18T15:29:34.000+02:00
diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
@@ -6,7 +6,6 @@ CHANGELOG
 
  * Add `resolve-env` option to `debug:config` command to display actual values of environment variables in dumped configuration
  * Add `NotificationAssertionsTrait`
- * Add option `framework.catch_all_throwables` to allow `Symfony\Component\HttpKernel\HttpKernel` to catch all kinds of `Throwable`
  * Make `AbstractController::render()` able to deal with forms and deprecate `renderForm()`
  * Deprecate the `Symfony\Component\Serializer\Normalizer\ObjectNormalizer` and
    `Symfony\Component\Serializer\Normalizer\PropertyNormalizer` autowiring aliases, type-hint against
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
@@ -133,7 +133,6 @@ public function getConfigTreeBuilder(): TreeBuilder
                 ->scalarNode('error_controller')
                     ->defaultValue('error_controller')
                 ->end()
-                ->booleanNode('catch_all_throwables')->defaultFalse()->info('HttpKernel will catch all kinds of \Throwable')->end()
             ->end()
         ;
 
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -330,7 +330,6 @@ public function load(array $configs, ContainerBuilder $container)
 
         $container->getDefinition('locale_listener')->replaceArgument(3, $config['set_locale_from_accept_language']);
         $container->getDefinition('response_listener')->replaceArgument(1, $config['set_content_language_from_locale']);
-        $container->getDefinition('http_kernel')->replaceArgument(4, $config['catch_all_throwables']);
 
         // If the slugger is used but the String component is not available, we should throw an error
         if (!ContainerBuilder::willBeAvailable('symfony/string', SluggerInterface::class, ['symfony/framework-bundle'])) {
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/services.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/services.php
@@ -89,7 +89,6 @@ class_exists(WorkflowEvents::class) ? WorkflowEvents::ALIASES : []
                 service('controller_resolver'),
                 service('request_stack'),
                 service('argument_resolver'),
-                false,
             ])
             ->tag('container.hot_path')
             ->tag('container.preload', ['class' => HttpKernelRunner::class])
diff --git a/src/Symfony/Bundle/FrameworkBundle/Test/ExceptionSubscriber.php b/src/Symfony/Bundle/FrameworkBundle/Test/ExceptionSubscriber.php
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php
@@ -657,7 +657,6 @@ class_exists(SemaphoreStore::class) && SemaphoreStore::isSupported() ? 'semaphor
                 'sanitizers' => [],
             ],
             'exceptions' => [],
-            'catch_all_throwables' => false,
         ];
     }
 }
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/UidTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/UidTest.php
@@ -11,7 +11,6 @@
 
 namespace Symfony\Bundle\FrameworkBundle\Tests\Functional;
 
-use Symfony\Bundle\FrameworkBundle\Test\ExceptionSubscriber;
 use Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\UidController;
 use Symfony\Component\Uid\Ulid;
 use Symfony\Component\Uid\UuidV1;
@@ -30,15 +29,12 @@ protected function setUp(): void
     public function testArgumentValueResolverDisabled()
     {
         $client = $this->createClient(['test_case' => 'Uid', 'root_config' => 'config_disabled.yml']);
+        $client->catchExceptions(false);
 
-        $client->request('GET', '/1/uuid-v1/'.new UuidV1());
-        $this->assertSame(500, $client->getResponse()->getStatusCode());
-        $exceptions = ExceptionSubscriber::shiftAll();
-        $this->assertCount(1, $exceptions);
-        $exception = reset($exceptions);
+        $this->expectException(\TypeError::class);
+        $this->expectExceptionMessage('Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\UidController::anyFormat(): Argument #1 ($userId) must be of type Symfony\Component\Uid\UuidV1, string given');
 
-        $this->assertInstanceOf(\TypeError::class, $exception);
-        $this->assertStringContainsString(UidController::class.'::anyFormat(): Argument #1 ($userId) must be of type Symfony\Component\Uid\UuidV1, string given', $exception->getMessage());
+        $client->request('GET', '/1/uuid-v1/'.new UuidV1());
     }
 
     public function testArgumentValueResolverEnabled()
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Uid/config_disabled.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Uid/config_disabled.yml
@@ -5,8 +5,3 @@ framework:
     http_method_override: false
     uid:
         enabled: false
-
-services:
-    Symfony\Bundle\FrameworkBundle\Test\ExceptionSubscriber:
-        tags:
-            - { name: kernel.event_subscriber }
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/config/framework.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/config/framework.yml
@@ -11,7 +11,6 @@ framework:
     enabled_locales: ['en', 'fr']
     session:
         storage_factory_id: session.storage.factory.mock_file
-    catch_all_throwables: true
 
 services:
     logger: { class: Psr\Log\NullLogger }
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Kernel/ConcreteMicroKernel.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Kernel/ConcreteMicroKernel.php
@@ -86,7 +86,6 @@ protected function configureContainer(ContainerBuilder $c, LoaderInterface $load
             'http_method_override' => false,
             'secret' => '$ecret',
             'router' => ['utf8' => true],
-            'catch_all_throwables' => true,
         ]);
 
         $c->setParameter('halloween', 'Have a great day!');
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Kernel/MicroKernelTraitTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Kernel/MicroKernelTraitTest.php
@@ -122,7 +122,6 @@ protected function configureContainer(ContainerConfigurator $c): void
                 $c->extension('framework', [
                     'http_method_override' => false,
                     'router' => ['utf8' => true],
-                    'catch_all_throwables' => true,
                 ]);
                 $c->services()->set('logger', NullLogger::class);
             }
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Kernel/flex-style/src/FlexStyleMicroKernel.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Kernel/flex-style/src/FlexStyleMicroKernel.php
@@ -103,7 +103,6 @@ protected function configureContainer(ContainerConfigurator $c): void
         $c->extension('framework', [
             'http_method_override' => false,
             'router' => ['utf8' => true],
-            'catch_all_throwables' => true,
         ]);
     }
 }
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/Authenticator/config.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/Authenticator/config.yml
@@ -7,7 +7,6 @@ framework:
     profiler: false
     session:
         storage_factory_id: session.storage.factory.mock_file
-    catch_all_throwables: true
 
 services:
     logger: { class: Psr\Log\NullLogger }
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/FirewallEntryPoint/config.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/FirewallEntryPoint/config.yml
@@ -11,7 +11,6 @@ framework:
     session:
         storage_factory_id: session.storage.factory.mock_file
     profiler: { only_exceptions: false }
-    catch_all_throwables: true
 
 services:
     logger: { class: Psr\Log\NullLogger }
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/config/framework.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/config/framework.yml
@@ -12,7 +12,6 @@ framework:
     session:
         storage_factory_id: session.storage.factory.mock_file
     profiler: { only_exceptions: false }
-    catch_all_throwables: true
 
 services:
     logger: { class: Psr\Log\NullLogger }
diff --git a/src/Symfony/Bundle/SecurityBundle/composer.json b/src/Symfony/Bundle/SecurityBundle/composer.json
@@ -38,7 +38,7 @@
         "symfony/dom-crawler": "^5.4|^6.0",
         "symfony/expression-language": "^5.4|^6.0",
         "symfony/form": "^5.4|^6.0",
-        "symfony/framework-bundle": "^6.2",
+        "symfony/framework-bundle": "^5.4|^6.0",
         "symfony/ldap": "^5.4|^6.0",
         "symfony/process": "^5.4|^6.0",
         "symfony/rate-limiter": "^5.4|^6.0",
@@ -53,7 +53,7 @@
     "conflict": {
         "symfony/browser-kit": "<5.4",
         "symfony/console": "<5.4",
-        "symfony/framework-bundle": "<6.2",
+        "symfony/framework-bundle": "<5.4",
         "symfony/ldap": "<5.4",
         "symfony/twig-bundle": "<5.4"
     },
diff --git a/src/Symfony/Bundle/WebProfilerBundle/Tests/Functional/WebProfilerBundleKernel.php b/src/Symfony/Bundle/WebProfilerBundle/Tests/Functional/WebProfilerBundleKernel.php
@@ -14,7 +14,6 @@
 use Psr\Log\NullLogger;
 use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
 use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait;
-use Symfony\Bundle\FrameworkBundle\Test\ExceptionSubscriber;
 use Symfony\Bundle\TwigBundle\TwigBundle;
 use Symfony\Bundle\WebProfilerBundle\WebProfilerBundle;
 use Symfony\Component\Config\Loader\LoaderInterface;
@@ -59,11 +58,6 @@ protected function configureContainer(ContainerBuilder $containerBuilder, Loader
             'router' => ['utf8' => true],
         ];
 
-        // If Symfony >= 6.2
-        if (class_exists(ExceptionSubscriber::class)) {
-            $config['catch_all_throwables'] = true;
-        }
-
         $containerBuilder->loadFromExtension('framework', $config);
 
         $containerBuilder->loadFromExtension('web_profiler', [
diff --git a/src/Symfony/Component/HttpKernel/CHANGELOG.md b/src/Symfony/Component/HttpKernel/CHANGELOG.md
@@ -4,7 +4,7 @@ CHANGELOG
 6.2
 ---
 
- * Add constructor argument `bool $catchThrowable` to `HttpKernel`
+ * Add request attribute `_handle_all_throwables` to allow `HttpKernel` to handle thrown `Error` in addition to `Exception`
  * Add `ControllerEvent::getAttributes()` to handle attributes on controllers
  * Add `#[Cache]` to describe the default HTTP cache headers on controllers
  * Add `absolute_uri` option to surrogate fragment renderers
diff --git a/src/Symfony/Component/HttpKernel/HttpKernel.php b/src/Symfony/Component/HttpKernel/HttpKernel.php
@@ -54,29 +54,26 @@ class HttpKernel implements HttpKernelInterface, TerminableInterface
     protected $resolver;
     protected $requestStack;
     private ArgumentResolverInterface $argumentResolver;
-    private bool $catchThrowable;
 
-    public function __construct(EventDispatcherInterface $dispatcher, ControllerResolverInterface $resolver, RequestStack $requestStack = null, ArgumentResolverInterface $argumentResolver = null, bool $catchThrowable = false)
+    public function __construct(EventDispatcherInterface $dispatcher, ControllerResolverInterface $resolver, RequestStack $requestStack = null, ArgumentResolverInterface $argumentResolver = null)
     {
         $this->dispatcher = $dispatcher;
         $this->resolver = $resolver;
         $this->requestStack = $requestStack ?? new RequestStack();
         $this->argumentResolver = $argumentResolver ?? new ArgumentResolver();
-        $this->catchThrowable = $catchThrowable;
-        if (!$catchThrowable) {
-            trigger_deprecation('symfony/http-kernel', '6.2', 'Starting from 7.0, "%s::handle()" will catch \Throwable exceptions and convert them to HttpFoundation responses. Pass $catchThrowable=true to adapt to this behavior now.', self::class);
-        }
     }
 
     public function handle(Request $request, int $type = HttpKernelInterface::MAIN_REQUEST, bool $catch = true): Response
     {
         $request->headers->set('X-Php-Ob-Level', (string) ob_get_level());
 
+        $handleAllThrowables = $type & $request->attributes->getInt('_handle_all_throwables');
+
         $this->requestStack->push($request);
         try {
             return $this->handleRaw($request, $type);
         } catch (\Throwable $e) {
-            if (!$this->catchThrowable && !$e instanceof \Exception) {
+            if ($e instanceof \Error && !$handleAllThrowables) {
                 throw $e;
             }
 
@@ -89,7 +86,7 @@ public function handle(Request $request, int $type = HttpKernelInterface::MAIN_R
                 throw $e;
             }
 
-            return $this->handleThrowable($e, $request, $type);
+            return $this->handleThrowable($e, $request, $type, $handleAllThrowables);
         } finally {
             $this->requestStack->pop();
         }
@@ -114,7 +111,7 @@ public function terminateWithException(\Throwable $exception, Request $request =
         }
 
         try {
-            $response = $this->handleThrowable($exception, $request, self::MAIN_REQUEST);
+            $response = $this->handleThrowable($exception, $request, self::MAIN_REQUEST, self::MAIN_REQUEST & $request->attributes->getInt('_handle_all_throwables'));
         } finally {
             if ($pop) {
                 $this->requestStack->pop();
@@ -217,10 +214,8 @@ private function finishRequest(Request $request, int $type)
 
     /**
      * Handles a throwable by trying to convert it to a Response.
-     *
-     * @throws \Throwable
      */
-    private function handleThrowable(\Throwable $e, Request $request, int $type): Response
+    private function handleThrowable(\Throwable $e, Request $request, int $type, bool $handleAllThrowables): Response
     {
         $event = new ExceptionEvent($this, $request, $type, $e);
         $this->dispatcher->dispatch($event, KernelEvents::EXCEPTION);
@@ -251,7 +246,7 @@ private function handleThrowable(\Throwable $e, Request $request, int $type): Re
         try {
             return $this->filterResponse($response, $request, $type);
         } catch (\Throwable $e) {
-            if (!$this->catchThrowable && !$e instanceof \Exception) {
+            if ($e instanceof \Error && !$handleAllThrowables) {
                 throw $e;
             }
 
diff --git a/src/Symfony/Component/HttpKernel/Tests/DataCollector/RequestDataCollectorTest.php b/src/Symfony/Component/HttpKernel/Tests/DataCollector/RequestDataCollectorTest.php
@@ -396,7 +396,7 @@ protected function createResponse()
     protected function injectController($collector, $controller, $request)
     {
         $resolver = $this->createMock(ControllerResolverInterface::class);
-        $httpKernel = new HttpKernel(new EventDispatcher(), $resolver, null, $this->createMock(ArgumentResolverInterface::class), true);
+        $httpKernel = new HttpKernel(new EventDispatcher(), $resolver, null, $this->createMock(ArgumentResolverInterface::class));
         $event = new ControllerEvent($httpKernel, $controller, $request, HttpKernelInterface::MAIN_REQUEST);
         $collector->onKernelController($event);
     }
diff --git a/src/Symfony/Component/HttpKernel/Tests/Debug/TraceableEventDispatcherTest.php b/src/Symfony/Component/HttpKernel/Tests/Debug/TraceableEventDispatcherTest.php
@@ -119,6 +119,6 @@ protected function getHttpKernel($dispatcher)
         $argumentResolver = $this->createMock(ArgumentResolverInterface::class);
         $argumentResolver->expects($this->once())->method('getArguments')->willReturn([]);
 
-        return new HttpKernel($dispatcher, $controllerResolver, new RequestStack(), $argumentResolver, true);
+        return new HttpKernel($dispatcher, $controllerResolver, new RequestStack(), $argumentResolver);
     }
 }
diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/RouterListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/RouterListenerTest.php
@@ -173,7 +173,7 @@ public function testWithBadRequest()
             return new Response('Exception handled', 400);
         }));
 
-        $kernel = new HttpKernel($dispatcher, new ControllerResolver(), $requestStack, new ArgumentResolver(), true);
+        $kernel = new HttpKernel($dispatcher, new ControllerResolver(), $requestStack, new ArgumentResolver());
 
         $request = Request::create('http://localhost/');
         $request->headers->set('host', '###');
@@ -195,7 +195,7 @@ public function testNoRoutingConfigurationResponse()
         $dispatcher = new EventDispatcher();
         $dispatcher->addSubscriber(new RouterListener($requestMatcher, $requestStack, new RequestContext()));
 
-        $kernel = new HttpKernel($dispatcher, new ControllerResolver(), $requestStack, new ArgumentResolver(), true);
+        $kernel = new HttpKernel($dispatcher, new ControllerResolver(), $requestStack, new ArgumentResolver());
 
         $request = Request::create('http://localhost/');
         $response = $kernel->handle($request);
diff --git a/src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php b/src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php
@@ -142,7 +142,7 @@ public function testExceptionInSubRequestsDoesNotMangleOutputBuffers()
             ->willReturn([])
         ;
 
-        $kernel = new HttpKernel(new EventDispatcher(), $controllerResolver, new RequestStack(), $argumentResolver, true);
+        $kernel = new HttpKernel(new EventDispatcher(), $controllerResolver, new RequestStack(), $argumentResolver);
         $renderer = new InlineFragmentRenderer($kernel);
 
         // simulate a main request with output buffering
diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestHttpKernel.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestHttpKernel.php
@@ -36,7 +36,7 @@ public function __construct($body, $status, $headers, \Closure $customizer = nul
         $this->headers = $headers;
         $this->customizer = $customizer;
 
-        parent::__construct($eventDispatcher ?? new EventDispatcher(), $this, null, $this, true);
+        parent::__construct($eventDispatcher ?? new EventDispatcher(), $this, null, $this);
     }
 
     public function assert(\Closure $callback)
diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestMultipleHttpKernel.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/TestMultipleHttpKernel.php
@@ -35,7 +35,7 @@ public function __construct($responses)
             $this->headers[] = $response['headers'];
         }
 
-        parent::__construct(new EventDispatcher(), $this, null, $this, true);
+        parent::__construct(new EventDispatcher(), $this, null, $this);
     }
 
     public function getBackendRequest()
diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpKernelTest.php b/src/Symfony/Component/HttpKernel/Tests/HttpKernelTest.php
diff --git a/src/Symfony/Component/HttpKernel/Tests/TestHttpKernel.php b/src/Symfony/Component/HttpKernel/Tests/TestHttpKernel.php
diff --git a/src/Symfony/Component/Runtime/Runner/Symfony/HttpKernelRunner.php b/src/Symfony/Component/Runtime/Runner/Symfony/HttpKernelRunner.php

Original file line number	Diff line number	Diff line change
`@@ -133,7 +133,6 @@ public function getConfigTreeBuilder(): TreeBuilder`
`133`	`133`	`->scalarNode('error_controller')`
`134`	`134`	`->defaultValue('error_controller')`
`135`	`135`	`->end()`
`136`		`- ->booleanNode('catch_all_throwables')->defaultFalse()->info('HttpKernel will catch all kinds of \Throwable')->end()`
`137`	`136`	`->end()`
`138`	`137`	`;`
`139`	`138`
Original file line number	Diff line number	Diff line change
`@@ -657,7 +657,6 @@ class_exists(SemaphoreStore::class) && SemaphoreStore::isSupported() ? 'semaphor`
`657`	`657`	`'sanitizers' => [],`
`658`	`658`	`],`
`659`	`659`	`'exceptions' => [],`
`660`		`- 'catch_all_throwables' => false,`
`661`	`660`	`];`
`662`	`661`	`}`
`663`	`662`	`}`