Skip to content

Commit 4fc192f

Browse files
feature #49275 [FrameworkBundle][HttpKernel] Configure ErrorHandler on boot (HypeMC)
This PR was merged into the 6.3 branch. Discussion ---------- [FrameworkBundle][HttpKernel] Configure `ErrorHandler` on boot | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Currently the `ErrorHandler` is somewhat configured in `FrameworkBundle::boot()` and then later fully in `DebugHandlersListener::configure()`. https://github.com/symfony/symfony/blob/3e79a276ecdad009bd36b85c7c8745f5bc2b525a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php#L92-L96 https://github.com/symfony/symfony/blob/3e79a276ecdad009bd36b85c7c8745f5bc2b525a/src/Symfony/Component/HttpKernel/EventListener/DebugHandlersListener.php#L70-L144 However, I've noticed that there are some edge case when the handler doesn't get configured in time/at all. One such example is when using the Sentry bundle. The bundle has its own error handler which gets registered when the [Sentry client is instantiated](https://github.com/getsentry/sentry-php/blob/6402930e7fde5198a07fb126786d1ed6c3fbe281/src/Client.php#L105). If you're using [Sentry with Monolog](https://docs.sentry.io/platforms/php/guides/symfony/#monolog-integration) the client gets instantiated when a Monolog logger is used. Since the `http_kernel` service requires a logger, the client gets instantiated before the `kernel.request` event and, as a result, when the `DebugHandlersListener` is trigger both `$handler` and `$this->earlyHandler` are `null` and the error handler is never configured properly. Another example when the handler is not configured in time is when an error occurs in a `kernel.request` listener that has a higher priority then the `DebugHandlersListener`. In such cases the error handler's loggers are not yet configured and the errors are not logged properly. These cases were tested with `APP_DEBUG=0` since the error handler is registered [earlier when debug is enabled](https://github.com/symfony/symfony/blob/3e79a276ecdad009bd36b85c7c8745f5bc2b525a/src/Symfony/Component/Runtime/GenericRuntime.php#L71-L81). The idea of this PR is to configure the error handler as early as possible to prevent such edge cases from ever being able to happen. The error handler is now fully configured in `FrameworkBundle::boot()`. Commits ------- 26e6a56694 [FrameworkBundle][HttpKernel] Configure ErrorHandler on boot
2 parents 4f64a5b + 900dd00 commit 4fc192f

File tree

6 files changed

+24
-19
lines changed

6 files changed

+24
-19
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ CHANGELOG
1313
* Deprecate the `notifier.logger_notification_listener` service, use the `notifier.notification_logger_listener` service instead
1414
* Allow setting private services with the test container
1515
* Register alias for argument for workflow services with workflow name only
16+
* Configure the `ErrorHandler` on `FrameworkBundle::boot()`
1617

1718
6.2
1819
---

DependencyInjection/FrameworkExtension.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,12 +1095,12 @@ private function registerDebugConfiguration(array $config, ContainerBuilder $con
10951095
$loader->load('debug.php');
10961096
}
10971097

1098-
$definition = $container->findDefinition('debug.debug_handlers_listener');
1098+
$definition = $container->findDefinition('debug.error_handler_configurator');
10991099

11001100
if (false === $config['log']) {
1101-
$definition->replaceArgument(1, null);
1101+
$definition->replaceArgument(0, null);
11021102
} elseif (true !== $config['log']) {
1103-
$definition->replaceArgument(2, $config['log']);
1103+
$definition->replaceArgument(1, $config['log']);
11041104
}
11051105

11061106
if (!$config['throw']) {

FrameworkBundle.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ class FrameworkBundle extends Bundle
9494
*/
9595
public function boot()
9696
{
97-
ErrorHandler::register(null, false)->throwAt($this->container->getParameter('debug.error_handler.throw_at'), true);
97+
$handler = ErrorHandler::register(null, false);
98+
$this->container->get('debug.error_handler_configurator')->configure($handler);
9899

99100
if ($this->container->getParameter('kernel.http_method_override')) {
100101
Request::enableHttpMethodParameterOverride();

Resources/config/debug_prod.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,26 +11,29 @@
1111

1212
namespace Symfony\Component\DependencyInjection\Loader\Configurator;
1313

14+
use Symfony\Component\HttpKernel\Debug\ErrorHandlerConfigurator;
1415
use Symfony\Component\HttpKernel\Debug\FileLinkFormatter;
1516
use Symfony\Component\HttpKernel\EventListener\DebugHandlersListener;
1617

1718
return static function (ContainerConfigurator $container) {
1819
$container->parameters()->set('debug.error_handler.throw_at', -1);
1920

2021
$container->services()
21-
->set('debug.debug_handlers_listener', DebugHandlersListener::class)
22+
->set('debug.error_handler_configurator', ErrorHandlerConfigurator::class)
23+
->public()
2224
->args([
23-
null, // Exception handler
2425
service('monolog.logger.php')->nullOnInvalid(),
2526
null, // Log levels map for enabled error levels
2627
param('debug.error_handler.throw_at'),
2728
param('kernel.debug'),
2829
param('kernel.debug'),
2930
service('monolog.logger.deprecation')->nullOnInvalid(),
3031
])
31-
->tag('kernel.event_subscriber')
3232
->tag('monolog.logger', ['channel' => 'php'])
3333

34+
->set('debug.debug_handlers_listener', DebugHandlersListener::class)
35+
->tag('kernel.event_subscriber')
36+
3437
->set('debug.file_link_formatter', FileLinkFormatter::class)
3538
->args([param('debug.file_link_format')])
3639

Tests/DependencyInjection/FrameworkExtensionTestCase.php

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -541,41 +541,41 @@ public function testEnabledPhpErrorsConfig()
541541
{
542542
$container = $this->createContainerFromFile('php_errors_enabled');
543543

544-
$definition = $container->getDefinition('debug.debug_handlers_listener');
545-
$this->assertEquals(new Reference('monolog.logger.php', ContainerInterface::NULL_ON_INVALID_REFERENCE), $definition->getArgument(1));
546-
$this->assertNull($definition->getArgument(2));
544+
$definition = $container->getDefinition('debug.error_handler_configurator');
545+
$this->assertEquals(new Reference('monolog.logger.php', ContainerInterface::NULL_ON_INVALID_REFERENCE), $definition->getArgument(0));
546+
$this->assertNull($definition->getArgument(1));
547547
$this->assertSame(-1, $container->getParameter('debug.error_handler.throw_at'));
548548
}
549549

550550
public function testDisabledPhpErrorsConfig()
551551
{
552552
$container = $this->createContainerFromFile('php_errors_disabled');
553553

554-
$definition = $container->getDefinition('debug.debug_handlers_listener');
554+
$definition = $container->getDefinition('debug.error_handler_configurator');
555+
$this->assertNull($definition->getArgument(0));
555556
$this->assertNull($definition->getArgument(1));
556-
$this->assertNull($definition->getArgument(2));
557557
$this->assertSame(0, $container->getParameter('debug.error_handler.throw_at'));
558558
}
559559

560560
public function testPhpErrorsWithLogLevel()
561561
{
562562
$container = $this->createContainerFromFile('php_errors_log_level');
563563

564-
$definition = $container->getDefinition('debug.debug_handlers_listener');
565-
$this->assertEquals(new Reference('monolog.logger.php', ContainerInterface::NULL_ON_INVALID_REFERENCE), $definition->getArgument(1));
566-
$this->assertSame(8, $definition->getArgument(2));
564+
$definition = $container->getDefinition('debug.error_handler_configurator');
565+
$this->assertEquals(new Reference('monolog.logger.php', ContainerInterface::NULL_ON_INVALID_REFERENCE), $definition->getArgument(0));
566+
$this->assertSame(8, $definition->getArgument(1));
567567
}
568568

569569
public function testPhpErrorsWithLogLevels()
570570
{
571571
$container = $this->createContainerFromFile('php_errors_log_levels');
572572

573-
$definition = $container->getDefinition('debug.debug_handlers_listener');
574-
$this->assertEquals(new Reference('monolog.logger.php', ContainerInterface::NULL_ON_INVALID_REFERENCE), $definition->getArgument(1));
573+
$definition = $container->getDefinition('debug.error_handler_configurator');
574+
$this->assertEquals(new Reference('monolog.logger.php', ContainerInterface::NULL_ON_INVALID_REFERENCE), $definition->getArgument(0));
575575
$this->assertSame([
576576
\E_NOTICE => \Psr\Log\LogLevel::ERROR,
577577
\E_WARNING => \Psr\Log\LogLevel::ERROR,
578-
], $definition->getArgument(2));
578+
], $definition->getArgument(1));
579579
}
580580

581581
public function testExceptionsConfig()

composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
"symfony/error-handler": "^6.1",
2727
"symfony/event-dispatcher": "^5.4|^6.0",
2828
"symfony/http-foundation": "^6.2",
29-
"symfony/http-kernel": "^6.2.1",
29+
"symfony/http-kernel": "^6.3",
3030
"symfony/polyfill-mbstring": "~1.0",
3131
"symfony/filesystem": "^5.4|^6.0",
3232
"symfony/finder": "^5.4|^6.0",

0 commit comments

Comments
 (0)