Skip to content

Commit a9e8913

Browse files
mcg-webmcg-web
authored
Merge pull request #663 from ste93cry/fix/use-exception-key-in-monolog-log-context
Use the exception key in the context of the logger
2 parents a60e4f6 + 1c3e39b commit a9e8913

File tree

2 files changed

+104
-75
lines changed

2 files changed

+104
-75
lines changed

‎src/EventListener/ErrorLoggerListener.php‎

Lines changed: 28 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -15,65 +15,55 @@ final class ErrorLoggerListener
1515
{
1616
public const DEFAULT_LOGGER_SERVICE = 'logger';
1717

18-
/** @var LoggerInterface */
18+
/**
19+
* @var LoggerInterface
20+
*/
1921
private $logger;
2022

21-
public function __construct(
22-
LoggerInterface $logger = null
23-
) {
24-
$this->logger = null === $logger ? new NullLogger() : $logger;
23+
public function __construct(?LoggerInterface $logger = null)
24+
{
25+
$this->logger = $logger ?? new NullLogger();
2526
}
2627

2728
public function onErrorFormatting(ErrorFormattingEvent $event): void
2829
{
2930
$error = $event->getError();
31+
$exception = $error->getPrevious();
3032

31-
if ($error->getPrevious()) {
32-
$exception = $error->getPrevious();
33-
if ($exception instanceof UserError) {
34-
if ($exception->getPrevious()) {
35-
$this->log($exception->getPrevious());
36-
}
33+
if (null === $exception) {
34+
return;
35+
}
3736

38-
return;
37+
if ($exception instanceof UserError) {
38+
if ($exception->getPrevious()) {
39+
$this->log($exception->getPrevious());
3940
}
4041

41-
if ($exception instanceof UserWarning) {
42-
if ($exception->getPrevious()) {
43-
$this->log($exception->getPrevious(), LogLevel::WARNING);
44-
}
42+
return;
43+
}
4544

46-
return;
45+
if ($exception instanceof UserWarning) {
46+
if ($exception->getPrevious()) {
47+
$this->log($exception->getPrevious(), LogLevel::WARNING);
4748
}
48-
$this->log($error->getPrevious(), LogLevel::CRITICAL);
49+
50+
return;
4951
}
50-
}
5152

52-
/**
53-
* @param \Throwable $throwable
54-
* @param string $errorLevel
55-
*/
56-
public function log(\Throwable $throwable, string $errorLevel = LogLevel::ERROR): void
57-
{
58-
$this->logger->$errorLevel(self::serializeThrowableObject($throwable), ['throwable' => $throwable]);
53+
$this->log($exception, LogLevel::CRITICAL);
5954
}
6055

61-
/**
62-
* @param \Throwable $throwable
63-
*
64-
* @return string
65-
*/
66-
private static function serializeThrowableObject($throwable): string
56+
public function log(\Throwable $exception, string $errorLevel = LogLevel::ERROR): void
6757
{
6858
$message = \sprintf(
6959
'[GraphQL] %s: %s[%d] (caught throwable) at %s line %s.',
70-
\get_class($throwable),
71-
$throwable->getMessage(),
72-
$throwable->getCode(),
73-
$throwable->getFile(),
74-
$throwable->getLine()
60+
\get_class($exception),
61+
$exception->getMessage(),
62+
$exception->getCode(),
63+
$exception->getFile(),
64+
$exception->getLine()
7565
);
7666

77-
return $message;
67+
$this->logger->log($errorLevel, $message, ['exception' => $exception]);
7868
}
7969
}

‎tests/EventListener/ErrorLoggerListenerTest.php‎

Lines changed: 76 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -9,69 +9,108 @@
99
use Overblog\GraphQLBundle\Error\UserWarning;
1010
use Overblog\GraphQLBundle\Event\ErrorFormattingEvent;
1111
use Overblog\GraphQLBundle\EventListener\ErrorLoggerListener;
12-
use PHPUnit\Framework\Constraint\Constraint;
1312
use PHPUnit\Framework\MockObject\Matcher\Invocation;
1413
use PHPUnit\Framework\MockObject\MockObject;
1514
use PHPUnit\Framework\TestCase;
1615
use Psr\Log\LoggerInterface;
16+
use Psr\Log\LogLevel;
1717

18-
class ErrorLoggerListenerTest extends TestCase
18+
final class ErrorLoggerListenerTest extends TestCase
1919
{
20-
/** @var ErrorLoggerListener */
20+
/**
21+
* @var ErrorLoggerListener
22+
*/
2123
private $listener;
2224

23-
/** @var LoggerInterface|MockObject */
25+
/**
26+
* @var LoggerInterface&MockObject
27+
*/
2428
private $logger;
2529

2630
public function setUp(): void
2731
{
28-
$this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock();
32+
$this->logger = $this->createMock(LoggerInterface::class);
2933
$this->listener = new ErrorLoggerListener($this->logger);
3034
}
3135

3236
/**
33-
* @param Error $error
34-
* @param Invocation $loggerExpects
35-
* @param Constraint|string $loggerMethod
36-
* @param array|null $with
37-
*
38-
* @dataProvider fixtures
37+
* @dataProvider onErrorFormattingDataProvider
3938
*/
40-
public function testOnErrorFormatting(Error $error, $loggerExpects, $loggerMethod, array $with = null): void
39+
public function testOnErrorFormatting(Error $error, Invocation $expectedLoggerCalls, array $expectedLoggerMethodArguments): void
4140
{
42-
$mock = $this->logger->expects($loggerExpects)->method($loggerMethod);
43-
if ($with) {
44-
$mock->with(...$with);
45-
}
41+
$this->logger->expects($expectedLoggerCalls)
42+
->method('log')
43+
->with(...$expectedLoggerMethodArguments);
4644

4745
$this->listener->onErrorFormatting(new ErrorFormattingEvent($error, []));
4846
}
4947

50-
public function fixtures()
48+
public function onErrorFormattingDataProvider(): \Generator
5149
{
52-
try {
53-
throw new \Exception('Ko!');
54-
} catch (\Exception $exception) {
55-
}
56-
$with = [
57-
\sprintf('[GraphQL] %s: %s[%d] (caught throwable) at %s line %s.', \Exception::class, 'Ko!', 0, __FILE__, $exception->getLine()),
58-
['throwable' => $exception],
50+
$exception = new \Exception('Ko!');
51+
52+
yield [
53+
new Error('Basic error'),
54+
$this->never(),
55+
[$this->anything()],
5956
];
6057

61-
return [
62-
[self::createError('Basic error'), $this->never(), $this->anything()],
63-
[self::createError('Wrapped Base UserError without previous', new \GraphQL\Error\UserError('User error message')), $this->never(), $this->anything()],
64-
[self::createError('Wrapped UserError without previous', new UserError('User error message')), $this->never(), $this->anything()],
65-
[self::createError('Wrapped UserWarning without previous', new UserWarning('User warning message')), $this->never(), $this->anything()],
66-
[self::createError('Wrapped unknown exception', $exception), $this->once(), 'critical', $with],
67-
[self::createError('Wrapped Base UserError with previous', new \GraphQL\Error\UserError('User error message', 0, $exception)), $this->once(), 'error', $with],
68-
[self::createError('Wrapped UserError with previous', new UserError('User error message', 0, $exception)), $this->once(), 'error', $with],
69-
[self::createError('Wrapped UserWarning with previous', new UserWarning('User warning message', 0, $exception)), $this->once(), 'warning', $with],
58+
yield [
59+
new Error('Wrapped Base UserError without previous', null, null, null, null, new UserError('User error message')),
60+
$this->never(),
61+
[$this->anything()],
7062
];
71-
}
7263

73-
private static function createError($message, \Exception $exception = null)
74-
{
75-
return new Error($message, null, null, null, null, $exception);
64+
yield [
65+
new Error('Wrapped UserError without previous', null, null, null, null, new UserError('User error message')),
66+
$this->never(),
67+
[$this->anything()],
68+
];
69+
70+
yield [
71+
new Error('Wrapped UserWarning without previous', null, null, null, null, new UserWarning('User warning message')),
72+
$this->never(),
73+
[$this->anything()],
74+
];
75+
76+
yield [
77+
new Error('Wrapped unknown exception', null, null, null, null, $exception),
78+
$this->once(),
79+
[
80+
LogLevel::CRITICAL,
81+
\sprintf('[GraphQL] Exception: Ko![0] (caught throwable) at %s line %s.', __FILE__, $exception->getLine()),
82+
['exception' => $exception],
83+
],
84+
];
85+
86+
yield [
87+
new Error('Wrapped Base UserError with previous', null, null, null, null, new UserError('User error message', 0, $exception)),
88+
$this->once(),
89+
[
90+
LogLevel::ERROR,
91+
\sprintf('[GraphQL] Exception: Ko![0] (caught throwable) at %s line %s.', __FILE__, $exception->getLine()),
92+
['exception' => $exception],
93+
],
94+
];
95+
96+
yield [
97+
new Error('Wrapped UserError with previous', null, null, null, null, new UserError('User error message', 0, $exception)),
98+
$this->once(),
99+
[
100+
LogLevel::ERROR,
101+
\sprintf('[GraphQL] Exception: Ko![0] (caught throwable) at %s line %s.', __FILE__, $exception->getLine()),
102+
['exception' => $exception],
103+
],
104+
];
105+
106+
yield [
107+
new Error('Wrapped UserWarning with previous', null, null, null, null, new UserWarning('User warning message', 0, $exception)),
108+
$this->once(),
109+
[
110+
LogLevel::WARNING,
111+
\sprintf('[GraphQL] Exception: Ko![0] (caught throwable) at %s line %s.', __FILE__, $exception->getLine()),
112+
['exception' => $exception],
113+
],
114+
];
76115
}
77116
}

0 commit comments

Comments
 (0)