Skip to content

Commit 919ec3b

Browse files
nicolas-grekasnicolas-grekas
committed
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 eaf1f26 + 26e06da commit 919ec3b

File tree

4 files changed

+216
-172
lines changed

4 files changed

+216
-172
lines changed

‎Debug/ErrorHandlerConfigurator.php

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpKernel\Debug;
13+
14+
use Psr\Log\LoggerInterface;
15+
use Symfony\Component\ErrorHandler\ErrorHandler;
16+
17+
/**
18+
* Configures the error handler.
19+
*
20+
* @final
21+
*
22+
* @internal
23+
*/
24+
class ErrorHandlerConfigurator
25+
{
26+
private ?LoggerInterface $logger;
27+
private ?LoggerInterface $deprecationLogger;
28+
private array|int|null $levels;
29+
private ?int $throwAt;
30+
private bool $scream;
31+
private bool $scope;
32+
33+
/**
34+
* @param array|int|null $levels An array map of E_* to LogLevel::* or an integer bit field of E_* constants
35+
* @param int|null $throwAt Thrown errors in a bit field of E_* constants, or null to keep the current value
36+
* @param bool $scream Enables/disables screaming mode, where even silenced errors are logged
37+
* @param bool $scope Enables/disables scoping mode
38+
*/
39+
public function __construct(LoggerInterface $logger = null, array|int|null $levels = \E_ALL, ?int $throwAt = \E_ALL, bool $scream = true, bool $scope = true, LoggerInterface $deprecationLogger = null)
40+
{
41+
$this->logger = $logger;
42+
$this->levels = $levels ?? \E_ALL;
43+
$this->throwAt = \is_int($throwAt) ? $throwAt : (null === $throwAt ? null : ($throwAt ? \E_ALL : null));
44+
$this->scream = $scream;
45+
$this->scope = $scope;
46+
$this->deprecationLogger = $deprecationLogger;
47+
}
48+
49+
/**
50+
* Configures the error handler.
51+
*/
52+
public function configure(ErrorHandler $handler): void
53+
{
54+
if ($this->logger || $this->deprecationLogger) {
55+
$this->setDefaultLoggers($handler);
56+
if (\is_array($this->levels)) {
57+
$levels = 0;
58+
foreach ($this->levels as $type => $log) {
59+
$levels |= $type;
60+
}
61+
} else {
62+
$levels = $this->levels;
63+
}
64+
65+
if ($this->scream) {
66+
$handler->screamAt($levels);
67+
}
68+
if ($this->scope) {
69+
$handler->scopeAt($levels & ~\E_USER_DEPRECATED & ~\E_DEPRECATED);
70+
} else {
71+
$handler->scopeAt(0, true);
72+
}
73+
$this->logger = $this->deprecationLogger = $this->levels = null;
74+
}
75+
if (null !== $this->throwAt) {
76+
$handler->throwAt($this->throwAt, true);
77+
}
78+
}
79+
80+
private function setDefaultLoggers(ErrorHandler $handler): void
81+
{
82+
if (\is_array($this->levels)) {
83+
$levelsDeprecatedOnly = [];
84+
$levelsWithoutDeprecated = [];
85+
foreach ($this->levels as $type => $log) {
86+
if (\E_DEPRECATED == $type || \E_USER_DEPRECATED == $type) {
87+
$levelsDeprecatedOnly[$type] = $log;
88+
} else {
89+
$levelsWithoutDeprecated[$type] = $log;
90+
}
91+
}
92+
} else {
93+
$levelsDeprecatedOnly = $this->levels & (\E_DEPRECATED | \E_USER_DEPRECATED);
94+
$levelsWithoutDeprecated = $this->levels & ~\E_DEPRECATED & ~\E_USER_DEPRECATED;
95+
}
96+
97+
$defaultLoggerLevels = $this->levels;
98+
if ($this->deprecationLogger && $levelsDeprecatedOnly) {
99+
$handler->setDefaultLogger($this->deprecationLogger, $levelsDeprecatedOnly);
100+
$defaultLoggerLevels = $levelsWithoutDeprecated;
101+
}
102+
103+
if ($this->logger && $defaultLoggerLevels) {
104+
$handler->setDefaultLogger($this->logger, $defaultLoggerLevels);
105+
}
106+
}
107+
}

‎EventListener/DebugHandlersListener.php

Lines changed: 11 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
namespace Symfony\Component\HttpKernel\EventListener;
1313

14-
use Psr\Log\LoggerInterface;
1514
use Symfony\Component\Console\ConsoleEvents;
1615
use Symfony\Component\Console\Event\ConsoleEvent;
1716
use Symfony\Component\Console\Output\ConsoleOutputInterface;
@@ -21,7 +20,7 @@
2120
use Symfony\Component\HttpKernel\KernelEvents;
2221

2322
/**
24-
* Configures errors and exceptions handlers.
23+
* Sets an exception handler.
2524
*
2625
* @author Nicolas Grekas <[email protected]>
2726
*
@@ -33,35 +32,19 @@ class DebugHandlersListener implements EventSubscriberInterface
3332
{
3433
private string|object|null $earlyHandler;
3534
private ?\Closure $exceptionHandler;
36-
private ?LoggerInterface $logger;
37-
private ?LoggerInterface $deprecationLogger;
38-
private array|int|null $levels;
39-
private ?int $throwAt;
40-
private bool $scream;
41-
private bool $scope;
4235
private bool $firstCall = true;
4336
private bool $hasTerminatedWithException = false;
4437

4538
/**
46-
* @param callable|null $exceptionHandler A handler that must support \Throwable instances that will be called on Exception
47-
* @param array|int|null $levels An array map of E_* to LogLevel::* or an integer bit field of E_* constants
48-
* @param int|null $throwAt Thrown errors in a bit field of E_* constants, or null to keep the current value
49-
* @param bool $scream Enables/disables screaming mode, where even silenced errors are logged
50-
* @param bool $scope Enables/disables scoping mode
39+
* @param callable|null $exceptionHandler A handler that must support \Throwable instances that will be called on Exception
5140
*/
52-
public function __construct(callable $exceptionHandler = null, LoggerInterface $logger = null, array|int|null $levels = \E_ALL, ?int $throwAt = \E_ALL, bool $scream = true, bool $scope = true, LoggerInterface $deprecationLogger = null)
41+
public function __construct(callable $exceptionHandler = null)
5342
{
5443
$handler = set_exception_handler('is_int');
5544
$this->earlyHandler = \is_array($handler) ? $handler[0] : null;
5645
restore_exception_handler();
5746

5847
$this->exceptionHandler = null === $exceptionHandler ? null : $exceptionHandler(...);
59-
$this->logger = $logger;
60-
$this->levels = $levels ?? \E_ALL;
61-
$this->throwAt = \is_int($throwAt) ? $throwAt : (null === $throwAt ? null : ($throwAt ? \E_ALL : null));
62-
$this->scream = $scream;
63-
$this->scope = $scope;
64-
$this->deprecationLogger = $deprecationLogger;
6548
}
6649

6750
/**
@@ -77,40 +60,6 @@ public function configure(object $event = null): void
7760
}
7861
$this->firstCall = $this->hasTerminatedWithException = false;
7962

80-
$handler = set_exception_handler('is_int');
81-
$handler = \is_array($handler) ? $handler[0] : null;
82-
restore_exception_handler();
83-
84-
if (!$handler instanceof ErrorHandler) {
85-
$handler = $this->earlyHandler;
86-
}
87-
88-
if ($handler instanceof ErrorHandler) {
89-
if ($this->logger || $this->deprecationLogger) {
90-
$this->setDefaultLoggers($handler);
91-
if (\is_array($this->levels)) {
92-
$levels = 0;
93-
foreach ($this->levels as $type => $log) {
94-
$levels |= $type;
95-
}
96-
} else {
97-
$levels = $this->levels;
98-
}
99-
100-
if ($this->scream) {
101-
$handler->screamAt($levels);
102-
}
103-
if ($this->scope) {
104-
$handler->scopeAt($levels & ~\E_USER_DEPRECATED & ~\E_DEPRECATED);
105-
} else {
106-
$handler->scopeAt(0, true);
107-
}
108-
$this->logger = $this->deprecationLogger = $this->levels = null;
109-
}
110-
if (null !== $this->throwAt) {
111-
$handler->throwAt($this->throwAt, true);
112-
}
113-
}
11463
if (!$this->exceptionHandler) {
11564
if ($event instanceof KernelEvent) {
11665
if (method_exists($kernel = $event->getKernel(), 'terminateWithException')) {
@@ -136,41 +85,21 @@ public function configure(object $event = null): void
13685
}
13786
}
13887
if ($this->exceptionHandler) {
88+
$handler = set_exception_handler('is_int');
89+
$handler = \is_array($handler) ? $handler[0] : null;
90+
restore_exception_handler();
91+
92+
if (!$handler instanceof ErrorHandler) {
93+
$handler = $this->earlyHandler;
94+
}
95+
13996
if ($handler instanceof ErrorHandler) {
14097
$handler->setExceptionHandler($this->exceptionHandler);
14198
}
14299
$this->exceptionHandler = null;
143100
}
144101
}
145102

146-
private function setDefaultLoggers(ErrorHandler $handler): void
147-
{
148-
if (\is_array($this->levels)) {
149-
$levelsDeprecatedOnly = [];
150-
$levelsWithoutDeprecated = [];
151-
foreach ($this->levels as $type => $log) {
152-
if (\E_DEPRECATED == $type || \E_USER_DEPRECATED == $type) {
153-
$levelsDeprecatedOnly[$type] = $log;
154-
} else {
155-
$levelsWithoutDeprecated[$type] = $log;
156-
}
157-
}
158-
} else {
159-
$levelsDeprecatedOnly = $this->levels & (\E_DEPRECATED | \E_USER_DEPRECATED);
160-
$levelsWithoutDeprecated = $this->levels & ~\E_DEPRECATED & ~\E_USER_DEPRECATED;
161-
}
162-
163-
$defaultLoggerLevels = $this->levels;
164-
if ($this->deprecationLogger && $levelsDeprecatedOnly) {
165-
$handler->setDefaultLogger($this->deprecationLogger, $levelsDeprecatedOnly);
166-
$defaultLoggerLevels = $levelsWithoutDeprecated;
167-
}
168-
169-
if ($this->logger && $defaultLoggerLevels) {
170-
$handler->setDefaultLogger($this->logger, $defaultLoggerLevels);
171-
}
172-
}
173-
174103
public static function getSubscribedEvents(): array
175104
{
176105
$events = [KernelEvents::REQUEST => ['configure', 2048]];

‎Tests/Debug/ErrorHandlerConfiguratorTest.php

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpKernel\Tests\Debug;
13+
14+
use PHPUnit\Framework\TestCase;
15+
use Psr\Log\LoggerInterface;
16+
use Psr\Log\LogLevel;
17+
use Symfony\Component\ErrorHandler\ErrorHandler;
18+
use Symfony\Component\HttpKernel\Debug\ErrorHandlerConfigurator;
19+
20+
class ErrorHandlerConfiguratorTest extends TestCase
21+
{
22+
public function testConfigure()
23+
{
24+
$logger = $this->createMock(LoggerInterface::class);
25+
$configurator = new ErrorHandlerConfigurator($logger);
26+
$handler = new ErrorHandler();
27+
28+
$configurator->configure($handler);
29+
30+
$loggers = $handler->setLoggers([]);
31+
32+
$this->assertArrayHasKey(\E_DEPRECATED, $loggers);
33+
$this->assertSame([$logger, LogLevel::INFO], $loggers[\E_DEPRECATED]);
34+
}
35+
36+
/**
37+
* @dataProvider provideLevelsAssignedToLoggers
38+
*/
39+
public function testLevelsAssignedToLoggers(bool $hasLogger, bool $hasDeprecationLogger, array|int $levels, array|int|null $expectedLoggerLevels, array|int|null $expectedDeprecationLoggerLevels)
40+
{
41+
$handler = $this->createMock(ErrorHandler::class);
42+
43+
$expectedCalls = [];
44+
$logger = null;
45+
$deprecationLogger = null;
46+
47+
if ($hasDeprecationLogger) {
48+
$deprecationLogger = $this->createMock(LoggerInterface::class);
49+
if (null !== $expectedDeprecationLoggerLevels) {
50+
$expectedCalls[] = [$deprecationLogger, $expectedDeprecationLoggerLevels];
51+
}
52+
}
53+
54+
if ($hasLogger) {
55+
$logger = $this->createMock(LoggerInterface::class);
56+
if (null !== $expectedLoggerLevels) {
57+
$expectedCalls[] = [$logger, $expectedLoggerLevels];
58+
}
59+
}
60+
61+
$handler
62+
->expects($this->exactly(\count($expectedCalls)))
63+
->method('setDefaultLogger')
64+
->withConsecutive(...$expectedCalls);
65+
66+
$configurator = new ErrorHandlerConfigurator($logger, $levels, null, true, true, $deprecationLogger);
67+
68+
$configurator->configure($handler);
69+
}
70+
71+
public static function provideLevelsAssignedToLoggers(): iterable
72+
{
73+
yield [false, false, 0, null, null];
74+
yield [false, false, \E_ALL, null, null];
75+
yield [false, false, [], null, null];
76+
yield [false, false, [\E_WARNING => LogLevel::WARNING, \E_USER_DEPRECATED => LogLevel::NOTICE], null, null];
77+
78+
yield [true, false, \E_ALL, \E_ALL, null];
79+
yield [true, false, \E_DEPRECATED, \E_DEPRECATED, null];
80+
yield [true, false, [], null, null];
81+
yield [true, false, [\E_WARNING => LogLevel::WARNING, \E_DEPRECATED => LogLevel::NOTICE], [\E_WARNING => LogLevel::WARNING, \E_DEPRECATED => LogLevel::NOTICE], null];
82+
83+
yield [false, true, 0, null, null];
84+
yield [false, true, \E_ALL, null, \E_DEPRECATED | \E_USER_DEPRECATED];
85+
yield [false, true, \E_ERROR, null, null];
86+
yield [false, true, [], null, null];
87+
yield [false, true, [\E_ERROR => LogLevel::ERROR, \E_DEPRECATED => LogLevel::DEBUG], null, [\E_DEPRECATED => LogLevel::DEBUG]];
88+
89+
yield [true, true, 0, null, null];
90+
yield [true, true, \E_ALL, \E_ALL & ~(\E_DEPRECATED | \E_USER_DEPRECATED), \E_DEPRECATED | \E_USER_DEPRECATED];
91+
yield [true, true, \E_ERROR, \E_ERROR, null];
92+
yield [true, true, \E_USER_DEPRECATED, null, \E_USER_DEPRECATED];
93+
yield [true, true, [\E_ERROR => LogLevel::ERROR, \E_DEPRECATED => LogLevel::DEBUG], [\E_ERROR => LogLevel::ERROR], [\E_DEPRECATED => LogLevel::DEBUG]];
94+
yield [true, true, [\E_ERROR => LogLevel::ALERT], [\E_ERROR => LogLevel::ALERT], null];
95+
yield [true, true, [\E_USER_DEPRECATED => LogLevel::NOTICE], null, [\E_USER_DEPRECATED => LogLevel::NOTICE]];
96+
}
97+
}

0 commit comments

Comments
 (0)