Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 573b8b9

Browse filesBrowse files
author
Joris Steyn
committed
Allow fine-grained control in log severity of failures
This commit adds a new interface method on the `RetryStrategyInterface` allowing custom retry strategies to control the log severity of failed and retried messages. This is a spin-off from symfony#43160 | Q | A | ------------- | --- | Branch? | 6.0 | Bug fix? | no | BC break? | yes - custom retry strategies require including the new trait or implementing the new method | New feature? | yes - changelog not yet updated | Deprecations? | no | Tickets | - | License | MIT | Doc PR | not yet, collecting feedback first The default logic for determining the log severity for failed messages has been (since symfony#43492): - recoverable exceptions are logged as warning - unrecoverable exceptions are logged as critical - neutral exceptions are logged as warning after a non-final attempt - neutral exceptions are logged as critical after a final attempt This PR implements the exact same behaviour but in a way that allows users to augment or override the default behaviour with their own specific needs. For example, having specific exceptions logged as `EMERGENCY` or to log them as `NOTICE` until or including the last attempt. The refactoring done to make this possible is: - move the logic determining if an exception is recoverable/unrecoverable/neutral to the `HandlerFailedException` so it can be used by retry strategy objects - add a new interface method on the `RetryStrategyInterface` (BC break!) - implement the existing behaviour using the new mechanism in a trait, this way, custom retry strategies can easily re-use the default behaviour
1 parent cc93fb0 commit 573b8b9
Copy full SHA for 573b8b9

File tree

Expand file treeCollapse file tree

8 files changed

+269
-49
lines changed
Filter options
Expand file treeCollapse file tree

8 files changed

+269
-49
lines changed

‎src/Symfony/Component/Messenger/EventListener/SendFailedMessageForRetryListener.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Messenger/EventListener/SendFailedMessageForRetryListener.php
+9-38Lines changed: 9 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,12 @@
1212

1313
use Psr\Container\ContainerInterface;
1414
use Psr\Log\LoggerInterface;
15+
use Psr\Log\LogLevel;
1516
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
1617
use Symfony\Component\Messenger\Envelope;
1718
use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent;
1819
use Symfony\Component\Messenger\Event\WorkerMessageRetriedEvent;
19-
use Symfony\Component\Messenger\Exception\HandlerFailedException;
20-
use Symfony\Component\Messenger\Exception\RecoverableExceptionInterface;
2120
use Symfony\Component\Messenger\Exception\RuntimeException;
22-
use Symfony\Component\Messenger\Exception\UnrecoverableExceptionInterface;
2321
use Symfony\Component\Messenger\Retry\RetryStrategyInterface;
2422
use Symfony\Component\Messenger\Stamp\DelayStamp;
2523
use Symfony\Component\Messenger\Stamp\RedeliveryStamp;
@@ -59,7 +57,7 @@ public function onMessageFailed(WorkerMessageFailedEvent $event)
5957
'class' => \get_class($message),
6058
];
6159

62-
$shouldRetry = $retryStrategy && $this->shouldRetry($throwable, $envelope, $retryStrategy);
60+
$shouldRetry = $retryStrategy && $retryStrategy->isRetryable($envelope, $throwable);
6361

6462
$retryCount = RedeliveryStamp::getRetryCountFromEnvelope($envelope);
6563
if ($shouldRetry) {
@@ -68,9 +66,10 @@ public function onMessageFailed(WorkerMessageFailedEvent $event)
6866
++$retryCount;
6967

7068
$delay = $retryStrategy->getWaitingTime($envelope, $throwable);
69+
$severity = $retryStrategy->getLogSeverity($envelope, $throwable);
7170

7271
if (null !== $this->logger) {
73-
$this->logger->warning('Error thrown while handling message {class}. Sending for retry #{retryCount} using {delay} ms delay. Error: "{error}"', $context + ['retryCount' => $retryCount, 'delay' => $delay, 'error' => $throwable->getMessage(), 'exception' => $throwable]);
72+
$this->logger->log($severity, 'Error thrown while handling message {class}. Sending for retry #{retryCount} using {delay} ms delay. Error: "{error}"', $context + ['retryCount' => $retryCount, 'delay' => $delay, 'error' => $throwable->getMessage(), 'exception' => $throwable]);
7473
}
7574

7675
// add the delay and retry stamp info
@@ -84,7 +83,11 @@ public function onMessageFailed(WorkerMessageFailedEvent $event)
8483
}
8584
} else {
8685
if (null !== $this->logger) {
87-
$this->logger->critical('Error thrown while handling message {class}. Removing from transport after {retryCount} retries. Error: "{error}"', $context + ['retryCount' => $retryCount, 'error' => $throwable->getMessage(), 'exception' => $throwable]);
86+
$severity = (null !== $retryStrategy)
87+
? $retryStrategy->getLogSeverity($envelope, $throwable)
88+
: LogLevel::CRITICAL;
89+
90+
$this->logger->log($severity, 'Error thrown while handling message {class}. Removing from transport after {retryCount} retries. Error: "{error}"', $context + ['retryCount' => $retryCount, 'error' => $throwable->getMessage(), 'exception' => $throwable]);
8891
}
8992
}
9093
}
@@ -121,38 +124,6 @@ public static function getSubscribedEvents(): array
121124
];
122125
}
123126

124-
private function shouldRetry(\Throwable $e, Envelope $envelope, RetryStrategyInterface $retryStrategy): bool
125-
{
126-
if ($e instanceof RecoverableExceptionInterface) {
127-
return true;
128-
}
129-
130-
// if one or more nested Exceptions is an instance of RecoverableExceptionInterface we should retry
131-
// if ALL nested Exceptions are an instance of UnrecoverableExceptionInterface we should not retry
132-
if ($e instanceof HandlerFailedException) {
133-
$shouldNotRetry = true;
134-
foreach ($e->getNestedExceptions() as $nestedException) {
135-
if ($nestedException instanceof RecoverableExceptionInterface) {
136-
return true;
137-
}
138-
139-
if (!$nestedException instanceof UnrecoverableExceptionInterface) {
140-
$shouldNotRetry = false;
141-
break;
142-
}
143-
}
144-
if ($shouldNotRetry) {
145-
return false;
146-
}
147-
}
148-
149-
if ($e instanceof UnrecoverableExceptionInterface) {
150-
return false;
151-
}
152-
153-
return $retryStrategy->isRetryable($envelope, $e);
154-
}
155-
156127
private function getRetryStrategyForTransport(string $alias): ?RetryStrategyInterface
157128
{
158129
if ($this->retryStrategyLocator->has($alias)) {

‎src/Symfony/Component/Messenger/Exception/HandlerFailedException.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Messenger/Exception/HandlerFailedException.php
+36Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,40 @@ function ($exception) use ($exceptionClassName) {
6464
)
6565
);
6666
}
67+
68+
/**
69+
* Determine if failure was unrecoverable.
70+
*
71+
* If ALL nested Exceptions are an instance of UnrecoverableExceptionInterface the failure is unrecoverable
72+
*
73+
* @param \Throwable $throwable
74+
*/
75+
public function isUnrecoverable(): bool
76+
{
77+
foreach ($this->exceptions as $nestedException) {
78+
if (!$nestedException instanceof UnrecoverableExceptionInterface) {
79+
return false;
80+
}
81+
}
82+
83+
return true;
84+
}
85+
86+
/**
87+
* Determine if failure was recoverable.
88+
*
89+
* If one or more nested Exceptions is an instance of RecoverableExceptionInterface the failure is recoverable
90+
*
91+
* @param \Throwable $throwable
92+
*/
93+
public function isRecoverable(): bool
94+
{
95+
foreach ($this->exceptions as $nestedException) {
96+
if ($nestedException instanceof RecoverableExceptionInterface) {
97+
return true;
98+
}
99+
}
100+
101+
return false;
102+
}
67103
}

‎src/Symfony/Component/Messenger/Retry/MultiplierRetryStrategy.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Messenger/Retry/MultiplierRetryStrategy.php
+2-10Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
*/
3333
class MultiplierRetryStrategy implements RetryStrategyInterface
3434
{
35+
use RetryRecoveryBehaviorTrait;
36+
3537
private int $maxRetries;
3638
private int $delayMilliseconds;
3739
private float $multiplier;
@@ -63,16 +65,6 @@ public function __construct(int $maxRetries = 3, int $delayMilliseconds = 1000,
6365
$this->maxDelayMilliseconds = $maxDelayMilliseconds;
6466
}
6567

66-
/**
67-
* @param \Throwable|null $throwable The cause of the failed handling
68-
*/
69-
public function isRetryable(Envelope $message, \Throwable $throwable = null): bool
70-
{
71-
$retries = RedeliveryStamp::getRetryCountFromEnvelope($message);
72-
73-
return $retries < $this->maxRetries;
74-
}
75-
7668
/**
7769
* @param \Throwable|null $throwable The cause of the failed handling
7870
*/

‎src/Symfony/Component/Messenger/Retry/RetryRecoveryBehaviorTrait.php

Copy file name to clipboard
+80Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
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\Messenger\Retry;
13+
14+
use Psr\Log\LogLevel;
15+
use Symfony\Component\Messenger\Envelope;
16+
use Symfony\Component\Messenger\Exception\HandlerFailedException;
17+
use Symfony\Component\Messenger\Exception\RecoverableExceptionInterface;
18+
use Symfony\Component\Messenger\Exception\UnrecoverableExceptionInterface;
19+
use Symfony\Component\Messenger\Stamp\RedeliveryStamp;
20+
21+
/**
22+
* Trait for retry strategies containing default log severity rules.
23+
*/
24+
trait RetryRecoveryBehaviorTrait
25+
{
26+
/**
27+
* @param \Throwable|null $throwable The cause of the failed handling
28+
*/
29+
public function isRetryable(Envelope $message, \Throwable $throwable = null): bool
30+
{
31+
if (null !== $throwable) {
32+
// A failure is either unrecoverable, recoverable or neutral
33+
if ($this->isUnrecoverable($throwable)) {
34+
return false;
35+
}
36+
37+
if ($this->isRecoverable($throwable)) {
38+
return true;
39+
}
40+
}
41+
42+
$retries = RedeliveryStamp::getRetryCountFromEnvelope($message);
43+
44+
return $retries < $this->maxRetries;
45+
}
46+
47+
/**
48+
* @return string The \Psr\Log\LogLevel log severity
49+
*/
50+
public function getLogSeverity(Envelope $message, \Throwable $throwable = null): string
51+
{
52+
return $this->isRetryable($message, $throwable)
53+
? LogLevel::WARNING
54+
: LogLevel::CRITICAL;
55+
}
56+
57+
/**
58+
* Determine if exception was unrecoverable.
59+
*
60+
* Unrecoverable exceptions should never be retried
61+
*/
62+
private function isUnrecoverable(\Throwable $throwable): bool
63+
{
64+
return ($throwable instanceof HandlerFailedException)
65+
? $throwable->isUnrecoverable()
66+
: $throwable instanceof UnrecoverableExceptionInterface;
67+
}
68+
69+
/**
70+
* Determine if exception was recoverable.
71+
*
72+
* Recoverable exceptions should always be retried
73+
*/
74+
private function isRecoverable(\Throwable $throwable): bool
75+
{
76+
return ($throwable instanceof HandlerFailedException)
77+
? $throwable->isRecoverable()
78+
: $throwable instanceof RecoverableExceptionInterface;
79+
}
80+
}

‎src/Symfony/Component/Messenger/Retry/RetryStrategyInterface.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Messenger/Retry/RetryStrategyInterface.php
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
* @author Fabien Potencier <fabien@symfony.com>
1818
* @author Grégoire Pineau <lyrixx@lyrixx.info>
1919
* @author Ryan Weaver <ryan@symfonycasts.com>
20+
* @author Joris Steyn <symfony@j0r1s.nl>
2021
*/
2122
interface RetryStrategyInterface
2223
{
@@ -31,4 +32,9 @@ public function isRetryable(Envelope $message, \Throwable $throwable = null): bo
3132
* @return int The time to delay/wait in milliseconds
3233
*/
3334
public function getWaitingTime(Envelope $message, \Throwable $throwable = null): int;
35+
36+
/**
37+
* @return string The \Psr\Log\LogLevel log severity
38+
*/
39+
public function getLogSeverity(Envelope $message, \Throwable $throwable = null): string;
3440
}

‎src/Symfony/Component/Messenger/Tests/EventListener/SendFailedMessageForRetryListenerTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Messenger/Tests/EventListener/SendFailedMessageForRetryListenerTest.php
+50-1Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Psr\Container\ContainerInterface;
16+
use Psr\Log\LoggerInterface;
1617
use Symfony\Component\Messenger\Envelope;
1718
use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent;
1819
use Symfony\Component\Messenger\EventListener\SendFailedMessageForRetryListener;
1920
use Symfony\Component\Messenger\Exception\RecoverableMessageHandlingException;
21+
use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException;
2022
use Symfony\Component\Messenger\Retry\RetryStrategyInterface;
2123
use Symfony\Component\Messenger\Stamp\DelayStamp;
2224
use Symfony\Component\Messenger\Stamp\RedeliveryStamp;
@@ -63,7 +65,7 @@ public function testRecoverableStrategyCausesRetry()
6365
$senderLocator->expects($this->once())->method('has')->willReturn(true);
6466
$senderLocator->expects($this->once())->method('get')->willReturn($sender);
6567
$retryStategy = $this->createMock(RetryStrategyInterface::class);
66-
$retryStategy->expects($this->never())->method('isRetryable');
68+
$retryStategy->expects($this->once())->method('isRetryable')->willReturn(true);
6769
$retryStategy->expects($this->once())->method('getWaitingTime')->willReturn(1000);
6870
$retryStrategyLocator = $this->createMock(ContainerInterface::class);
6971
$retryStrategyLocator->expects($this->once())->method('has')->willReturn(true);
@@ -190,4 +192,51 @@ public function testEnvelopeKeepOnlyTheLast10Stamps()
190192

191193
$listener->onMessageFailed($event);
192194
}
195+
196+
public function testUsesRetryStrategyToDetermineLogSeverityForRecoverableExceptions()
197+
{
198+
$sender = $this->createMock(SenderInterface::class);
199+
$sender->expects($this->once())->method('send')->willReturn(new Envelope(new \stdClass(), []));
200+
$senderLocator = $this->createMock(ContainerInterface::class);
201+
$senderLocator->expects($this->once())->method('has')->willReturn(true);
202+
$senderLocator->expects($this->once())->method('get')->willReturn($sender);
203+
$retryStategy = $this->createMock(RetryStrategyInterface::class);
204+
$retryStategy->expects($this->once())->method('isRetryable')->willReturn(true);
205+
$retryStategy->expects($this->once())->method('getLogSeverity')->willReturn('info');
206+
$retryStrategyLocator = $this->createMock(ContainerInterface::class);
207+
$retryStrategyLocator->expects($this->once())->method('has')->willReturn(true);
208+
$retryStrategyLocator->expects($this->once())->method('get')->willReturn($retryStategy);
209+
$logger = $this->createMock(LoggerInterface::class);
210+
$logger->expects($this->once())->method('log')->with('info');
211+
212+
$listener = new SendFailedMessageForRetryListener($senderLocator, $retryStrategyLocator, $logger);
213+
214+
$exception = new RecoverableMessageHandlingException('retry');
215+
$envelope = new Envelope(new \stdClass());
216+
$event = new WorkerMessageFailedEvent($envelope, 'my_receiver', $exception);
217+
218+
$listener->onMessageFailed($event);
219+
}
220+
221+
public function testUsesRetryStrategyToDetermineLogSeverityForUnrecoverableExceptions()
222+
{
223+
$senderLocator = $this->createMock(ContainerInterface::class);
224+
$retryStategy = $this->createMock(RetryStrategyInterface::class);
225+
$retryStategy->expects($this->once())->method('isRetryable')->willReturn(false);
226+
$retryStategy->expects($this->once())->method('getLogSeverity')->willReturn('info');
227+
$retryStrategyLocator = $this->createMock(ContainerInterface::class);
228+
$retryStrategyLocator->expects($this->once())->method('has')->willReturn(true);
229+
$retryStrategyLocator->expects($this->once())->method('get')->willReturn($retryStategy);
230+
231+
$logger = $this->createMock(LoggerInterface::class);
232+
$logger->expects($this->once())->method('log')->with('info');
233+
234+
$listener = new SendFailedMessageForRetryListener($senderLocator, $retryStrategyLocator, $logger);
235+
236+
$exception = new UnrecoverableMessageHandlingException('retry');
237+
$envelope = new Envelope(new \stdClass());
238+
$event = new WorkerMessageFailedEvent($envelope, 'my_receiver', $exception);
239+
240+
$listener->onMessageFailed($event);
241+
}
193242
}

‎src/Symfony/Component/Messenger/Tests/Exception/HandlerFailedExceptionTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Messenger/Tests/Exception/HandlerFailedExceptionTest.php
+43Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
use PHPUnit\Framework\TestCase;
66
use Symfony\Component\Messenger\Envelope;
77
use Symfony\Component\Messenger\Exception\HandlerFailedException;
8+
use Symfony\Component\Messenger\Exception\RecoverableMessageHandlingException;
9+
use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException;
810
use Symfony\Component\Messenger\Tests\Fixtures\MyOwnChildException;
911
use Symfony\Component\Messenger\Tests\Fixtures\MyOwnException;
1012

@@ -57,4 +59,45 @@ public function testThatNestedExceptionClassAreNotFoundIfNotPresent()
5759
$handlerException = new HandlerFailedException($envelope, [$exception]);
5860
$this->assertCount(0, $handlerException->getNestedExceptionOfClass(MyOwnException::class));
5961
}
62+
63+
public function testFailureIsConsideredUnrecoverableWithSingleNestedUnrecoverableExceptions()
64+
{
65+
$envelope = new Envelope(new \stdClass());
66+
67+
$handlerException = new HandlerFailedException($envelope, [new UnrecoverableMessageHandlingException()]);
68+
$this->assertTrue($handlerException->isUnrecoverable());
69+
}
70+
71+
public function testFailureIsConsideredUnrecoverableWithMultipleNestedUnrecoverableExceptions()
72+
{
73+
$envelope = new Envelope(new \stdClass());
74+
75+
$handlerException = new HandlerFailedException($envelope, [new UnrecoverableMessageHandlingException(), new UnrecoverableMessageHandlingException()]);
76+
$this->assertTrue($handlerException->isUnrecoverable());
77+
}
78+
79+
public function testFailureIsNotConsideredUnrecoverableWhenNotAllExceptionsAreUnrecoverable()
80+
{
81+
$envelope = new Envelope(new \stdClass());
82+
83+
$handlerException = new HandlerFailedException($envelope, [new UnrecoverableMessageHandlingException(), new \LogicException()]);
84+
$this->assertFalse($handlerException->isUnrecoverable());
85+
}
86+
87+
public function testFailureIsConsideredRecoverableWithAnyNestedRecoverableExceptions()
88+
{
89+
$envelope = new Envelope(new \stdClass());
90+
91+
$handlerException = new HandlerFailedException($envelope, [new \LogicException(), new RecoverableMessageHandlingException()]);
92+
$this->assertTrue($handlerException->isRecoverable());
93+
}
94+
95+
public function testFailureIsConsideredNeutralWithNoRecoverableOrUnrecoverableExceptions()
96+
{
97+
$envelope = new Envelope(new \stdClass());
98+
99+
$handlerException = new HandlerFailedException($envelope, [new \LogicException()]);
100+
$this->assertFalse($handlerException->isUnrecoverable());
101+
$this->assertFalse($handlerException->isRecoverable());
102+
}
60103
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.