Movatterモバイル変換

[0]ホーム
URL:

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

 Sign up
Appearance settings

Commit0e287ec

Browse 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 failedand retried messages.This is a spin-off from#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 firstThe default logic for determining the log severity for failed messageshas been (since#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 attemptThis PR implements the exact same behaviour but in a way that allowsusers to augment or override the default behaviour with their ownspecific needs. For example, having specific exceptions logged as`EMERGENCY` or to log them as `NOTICE` until or including the lastattempt.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 parentcc93fb0 commit0e287ec

File tree

8 files changed

+256
-39
lines changed

8 files changed

+256
-39
lines changed

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

Lines changed: 9 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,13 @@
1111
namespaceSymfony\Component\Messenger\EventListener;
1212

1313
usePsr\Container\ContainerInterface;
14+
usePsr\Log\LogLevel;
1415
usePsr\Log\LoggerInterface;
1516
useSymfony\Component\EventDispatcher\EventSubscriberInterface;
1617
useSymfony\Component\Messenger\Envelope;
1718
useSymfony\Component\Messenger\Event\WorkerMessageFailedEvent;
1819
useSymfony\Component\Messenger\Event\WorkerMessageRetriedEvent;
19-
useSymfony\Component\Messenger\Exception\HandlerFailedException;
20-
useSymfony\Component\Messenger\Exception\RecoverableExceptionInterface;
2120
useSymfony\Component\Messenger\Exception\RuntimeException;
22-
useSymfony\Component\Messenger\Exception\UnrecoverableExceptionInterface;
2321
useSymfony\Component\Messenger\Retry\RetryStrategyInterface;
2422
useSymfony\Component\Messenger\Stamp\DelayStamp;
2523
useSymfony\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 = ($retryStrategy !==null)
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-
privatefunctionshouldRetry(\Throwable$e,Envelope$envelope,RetryStrategyInterface$retryStrategy):bool
125-
{
126-
if ($einstanceof RecoverableExceptionInterface) {
127-
returntrue;
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 ($einstanceof HandlerFailedException) {
133-
$shouldNotRetry =true;
134-
foreach ($e->getNestedExceptions()as$nestedException) {
135-
if ($nestedExceptioninstanceof RecoverableExceptionInterface) {
136-
returntrue;
137-
}
138-
139-
if (!$nestedExceptioninstanceof UnrecoverableExceptionInterface) {
140-
$shouldNotRetry =false;
141-
break;
142-
}
143-
}
144-
if ($shouldNotRetry) {
145-
returnfalse;
146-
}
147-
}
148-
149-
if ($einstanceof UnrecoverableExceptionInterface) {
150-
returnfalse;
151-
}
152-
153-
return$retryStrategy->isRetryable($envelope,$e);
154-
}
155-
156127
privatefunctiongetRetryStrategyForTransport(string$alias): ?RetryStrategyInterface
157128
{
158129
if ($this->retryStrategyLocator->has($alias)) {

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,42 @@ 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 we should not retry
72+
*
73+
* @param \Throwable $throwable
74+
* @return bool
75+
*/
76+
publicfunctionisUnrecoverable():bool
77+
{
78+
foreach ($this->exceptionsas$nestedException) {
79+
if (!$nestedExceptioninstanceof UnrecoverableExceptionInterface) {
80+
returnfalse;
81+
}
82+
}
83+
84+
returntrue;
85+
}
86+
87+
/**
88+
* Determine if failure was recoverable
89+
*
90+
* If one or more nested Exceptions is an instance of RecoverableExceptionInterface we should retry
91+
*
92+
* @param \Throwable $throwable
93+
* @return bool
94+
*/
95+
publicfunctionisRecoverable():bool
96+
{
97+
foreach ($this->exceptionsas$nestedException) {
98+
if ($nestedExceptioninstanceof RecoverableExceptionInterface) {
99+
returntrue;
100+
}
101+
}
102+
103+
returnfalse;
104+
}
67105
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
*/
3333
class MultiplierRetryStrategyimplements RetryStrategyInterface
3434
{
35+
use RetryLogSeverityTrait;
36+
3537
privateint$maxRetries;
3638
privateint$delayMilliseconds;
3739
privatefloat$multiplier;
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
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+
namespaceSymfony\Component\Messenger\Retry;
13+
14+
usePsr\Log\LogLevel;
15+
useSymfony\Component\Messenger\Envelope;
16+
useSymfony\Component\Messenger\Exception\HandlerFailedException;
17+
useSymfony\Component\Messenger\Exception\RecoverableExceptionInterface;
18+
useSymfony\Component\Messenger\Exception\UnrecoverableExceptionInterface;
19+
20+
/**
21+
* Trait for retry strategies containing default log severity rules
22+
*/
23+
trait RetryLogSeverityTrait
24+
{
25+
26+
/**
27+
* @param \Throwable|null $throwable The cause of the failed handling
28+
*/
29+
abstractpublicfunctionisRetryable(Envelope$message,\Throwable$throwable =null):bool;
30+
31+
/**
32+
* @return string The \Psr\Log\LogLevel log severity
33+
*/
34+
publicfunctiongetLogSeverity(Envelope$message,\Throwable$throwable =null):string
35+
{
36+
// A failure is either unrecoverable, recoverable or neutral
37+
$isUnrecoverable = ($throwableinstanceof HandlerFailedException)
38+
?$throwable->isUnrecoverable()
39+
:$throwableinstanceof UnrecoverableExceptionInterface;
40+
41+
$isRecoverable = ($throwableinstanceof HandlerFailedException)
42+
?$throwable->isRecoverable()
43+
:$throwableinstanceof RecoverableExceptionInterface;
44+
45+
if ($isUnrecoverable) {
46+
return LogLevel::CRITICAL;
47+
}
48+
49+
if ($isRecoverable) {
50+
return LogLevel::WARNING;
51+
}
52+
53+
return$this->isRetryable($message,$throwable)
54+
? LogLevel::WARNING
55+
: LogLevel::CRITICAL;
56+
}
57+
}

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

Lines 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
publicfunctiongetWaitingTime(Envelope$message,\Throwable$throwable =null):int;
35+
36+
/**
37+
* @return string The \Psr\Log\LogLevel log severity
38+
*/
39+
publicfunctiongetLogSeverity(Envelope$message,\Throwable$throwable =null):string;
3440
}

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

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@
1313

1414
usePHPUnit\Framework\TestCase;
1515
usePsr\Container\ContainerInterface;
16+
usePsr\Log\LoggerInterface;
1617
useSymfony\Component\Messenger\Envelope;
1718
useSymfony\Component\Messenger\Event\WorkerMessageFailedEvent;
1819
useSymfony\Component\Messenger\EventListener\SendFailedMessageForRetryListener;
1920
useSymfony\Component\Messenger\Exception\RecoverableMessageHandlingException;
21+
useSymfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException;
2022
useSymfony\Component\Messenger\Retry\RetryStrategyInterface;
2123
useSymfony\Component\Messenger\Stamp\DelayStamp;
2224
useSymfony\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+
publicfunctiontestUsesRetryStrategyToDetermineLogSeverityForRecoverableExceptions()
197+
{
198+
$sender =$this->createMock(SenderInterface::class);
199+
$sender->expects($this->once())->method('send')->willReturn(newEnvelope(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 =newSendFailedMessageForRetryListener($senderLocator,$retryStrategyLocator,$logger);
213+
214+
$exception =newRecoverableMessageHandlingException('retry');
215+
$envelope =newEnvelope(new \stdClass());
216+
$event =newWorkerMessageFailedEvent($envelope,'my_receiver',$exception);
217+
218+
$listener->onMessageFailed($event);
219+
}
220+
221+
publicfunctiontestUsesRetryStrategyToDetermineLogSeverityForUnrecoverableExceptions()
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 =newSendFailedMessageForRetryListener($senderLocator,$retryStrategyLocator,$logger);
235+
236+
$exception =newUnrecoverableMessageHandlingException('retry');
237+
$envelope =newEnvelope(new \stdClass());
238+
$event =newWorkerMessageFailedEvent($envelope,'my_receiver',$exception);
239+
240+
$listener->onMessageFailed($event);
241+
}
193242
}

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
usePHPUnit\Framework\TestCase;
66
useSymfony\Component\Messenger\Envelope;
77
useSymfony\Component\Messenger\Exception\HandlerFailedException;
8+
useSymfony\Component\Messenger\Exception\RecoverableMessageHandlingException;
9+
useSymfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException;
810
useSymfony\Component\Messenger\Tests\Fixtures\MyOwnChildException;
911
useSymfony\Component\Messenger\Tests\Fixtures\MyOwnException;
1012

@@ -57,4 +59,53 @@ public function testThatNestedExceptionClassAreNotFoundIfNotPresent()
5759
$handlerException =newHandlerFailedException($envelope, [$exception]);
5860
$this->assertCount(0,$handlerException->getNestedExceptionOfClass(MyOwnException::class));
5961
}
62+
63+
publicfunctiontestFailureIsConsideredUnrecoverableWithoutNestedExceptions()
64+
{
65+
$envelope =newEnvelope(new \stdClass());
66+
67+
$handlerException =newHandlerFailedException($envelope, []);
68+
$this->assertTrue($handlerException->isUnrecoverable());
69+
}
70+
71+
publicfunctiontestFailureIsConsideredUnrecoverableWithSingleNestedUnrecoverableExceptions()
72+
{
73+
$envelope =newEnvelope(new \stdClass());
74+
75+
$handlerException =newHandlerFailedException($envelope, [newUnrecoverableMessageHandlingException()]);
76+
$this->assertTrue($handlerException->isUnrecoverable());
77+
}
78+
79+
publicfunctiontestFailureIsConsideredUnrecoverableWithMultipleNestedUnrecoverableExceptions()
80+
{
81+
$envelope =newEnvelope(new \stdClass());
82+
83+
$handlerException =newHandlerFailedException($envelope, [newUnrecoverableMessageHandlingException(),newUnrecoverableMessageHandlingException()]);
84+
$this->assertTrue($handlerException->isUnrecoverable());
85+
}
86+
87+
publicfunctiontestFailureIsNotConsideredUnrecoverableWhenNotAllExceptionsAreUnrecoverable()
88+
{
89+
$envelope =newEnvelope(new \stdClass());
90+
91+
$handlerException =newHandlerFailedException($envelope, [newUnrecoverableMessageHandlingException(),new \LogicException()]);
92+
$this->assertFalse($handlerException->isUnrecoverable());
93+
}
94+
95+
publicfunctiontestFailureIsConsideredRecoverableWithAnyNestedRecoverableExceptions()
96+
{
97+
$envelope =newEnvelope(new \stdClass());
98+
99+
$handlerException =newHandlerFailedException($envelope, [new \LogicException(),newRecoverableMessageHandlingException()]);
100+
$this->assertTrue($handlerException->isRecoverable());
101+
}
102+
103+
publicfunctiontestFailureIsConsideredNeutralWithNoRecoverableOrUnrecoverableExceptions()
104+
{
105+
$envelope =newEnvelope(new \stdClass());
106+
107+
$handlerException =newHandlerFailedException($envelope, [new \LogicException()]);
108+
$this->assertFalse($handlerException->isUnrecoverable());
109+
$this->assertFalse($handlerException->isRecoverable());
110+
}
60111
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp