Skip to content

Commit c6bbff0

Browse files
fabpotfabpot
committed
bug #59513 [Messenger ] Extract retry delay from nested RecoverableExceptionInterface (AydinHassan)
This PR was merged into the 7.2 branch. Discussion ---------- [Messenger ] Extract retry delay from nested `RecoverableExceptionInterface` | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | n/a | License | MIT Hey, In #57915 the ability to add a retry delay to `RecoverableExceptionInterface` was implemented. However, I noticed while debugging that it in my case the custom delay was not being applied because `RecoverableExceptionInterface` is wrapped in a `HandlerFailedException` in: https://github.com/symfony/symfony/blob/7a16efefdee2d282cd6ccfd6ecb0b82ff5a06936/src/Symfony/Component/Messenger/Middleware/HandleMessageMiddleware.php#L124 For detecting whether we should retry we already unwrap `HandlerFailedException` so I implemented a similar method to fetch the delay, simply getting it from the first `RecoverableExceptionInterface`. Maybe we need something more advanced, so your feedback there would be appreciated! Commits ------- 4a02222 [Messenger ] Extract retry delay from nested `RecoverableExceptionInterface`
2 parents 90103f1 + 4a02222 commit c6bbff0

File tree

2 files changed

+82
-6
lines changed

2 files changed

+82
-6
lines changed

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

+25-6
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,7 @@ public function onMessageFailed(WorkerMessageFailedEvent $event): void
6363

6464
++$retryCount;
6565

66-
$delay = null;
67-
if ($throwable instanceof RecoverableExceptionInterface && method_exists($throwable, 'getRetryDelay')) {
68-
$delay = $throwable->getRetryDelay();
69-
}
70-
71-
$delay ??= $retryStrategy->getWaitingTime($envelope, $throwable);
66+
$delay = $this->getWaitingTime($envelope, $throwable, $retryStrategy);
7267

7368
$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]);
7469

@@ -148,6 +143,30 @@ private function shouldRetry(\Throwable $e, Envelope $envelope, RetryStrategyInt
148143
return $retryStrategy->isRetryable($envelope, $e);
149144
}
150145

146+
private function getWaitingTime(Envelope $envelope, \Throwable $throwable, RetryStrategyInterface $retryStrategy): int
147+
{
148+
$delay = null;
149+
if ($throwable instanceof RecoverableExceptionInterface && method_exists($throwable, 'getRetryDelay')) {
150+
$delay = $throwable->getRetryDelay();
151+
}
152+
153+
if ($throwable instanceof HandlerFailedException) {
154+
foreach ($throwable->getWrappedExceptions() as $nestedException) {
155+
if (!$nestedException instanceof RecoverableExceptionInterface
156+
|| !method_exists($nestedException, 'getRetryDelay')
157+
|| 0 > $retryDelay = $nestedException->getRetryDelay() ?? -1
158+
) {
159+
continue;
160+
}
161+
if ($retryDelay < ($delay ?? \PHP_INT_MAX)) {
162+
$delay = $retryDelay;
163+
}
164+
}
165+
}
166+
167+
return $delay ?? $retryStrategy->getWaitingTime($envelope, $throwable);
168+
}
169+
151170
private function getRetryStrategyForTransport(string $alias): ?RetryStrategyInterface
152171
{
153172
if ($this->retryStrategyLocator->has($alias)) {

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

+57
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent;
1919
use Symfony\Component\Messenger\Event\WorkerMessageRetriedEvent;
2020
use Symfony\Component\Messenger\EventListener\SendFailedMessageForRetryListener;
21+
use Symfony\Component\Messenger\Exception\HandlerFailedException;
2122
use Symfony\Component\Messenger\Exception\RecoverableMessageHandlingException;
2223
use Symfony\Component\Messenger\Retry\RetryStrategyInterface;
2324
use Symfony\Component\Messenger\Stamp\DelayStamp;
@@ -108,6 +109,62 @@ public function testRecoverableExceptionRetryDelayOverridesStrategy()
108109
$listener->onMessageFailed($event);
109110
}
110111

112+
/**
113+
* @dataProvider provideRetryDelays
114+
*/
115+
public function testWrappedRecoverableExceptionRetryDelayOverridesStrategy(array $retries, int $expectedDelay)
116+
{
117+
$sender = $this->createMock(SenderInterface::class);
118+
$sender->expects($this->once())->method('send')->willReturnCallback(function (Envelope $envelope) use ($expectedDelay) {
119+
$delayStamp = $envelope->last(DelayStamp::class);
120+
$redeliveryStamp = $envelope->last(RedeliveryStamp::class);
121+
122+
$this->assertInstanceOf(DelayStamp::class, $delayStamp);
123+
$this->assertSame($expectedDelay, $delayStamp->getDelay());
124+
125+
$this->assertInstanceOf(RedeliveryStamp::class, $redeliveryStamp);
126+
$this->assertSame(1, $redeliveryStamp->getRetryCount());
127+
128+
return $envelope;
129+
});
130+
$senderLocator = new Container();
131+
$senderLocator->set('my_receiver', $sender);
132+
$retryStrategy = $this->createMock(RetryStrategyInterface::class);
133+
$retryStrategy->expects($this->never())->method('isRetryable');
134+
$retryStrategy->expects($this->never())->method('getWaitingTime');
135+
$retryStrategyLocator = new Container();
136+
$retryStrategyLocator->set('my_receiver', $retryStrategy);
137+
138+
$listener = new SendFailedMessageForRetryListener($senderLocator, $retryStrategyLocator);
139+
140+
$envelope = new Envelope(new \stdClass());
141+
$exception = new HandlerFailedException(
142+
$envelope,
143+
array_map(fn (int $retry) => new RecoverableMessageHandlingException('retry', retryDelay: $retry), $retries)
144+
);
145+
$event = new WorkerMessageFailedEvent($envelope, 'my_receiver', $exception);
146+
147+
$listener->onMessageFailed($event);
148+
}
149+
150+
public static function provideRetryDelays(): iterable
151+
{
152+
yield 'one_exception' => [
153+
[1235],
154+
1235,
155+
];
156+
157+
yield 'multiple_exceptions' => [
158+
[1235, 2000, 1000],
159+
1000,
160+
];
161+
162+
yield 'zero_delay' => [
163+
[0, 2000, 1000],
164+
0,
165+
];
166+
}
167+
111168
public function testEnvelopeIsSentToTransportOnRetry()
112169
{
113170
$exception = new \Exception('no!');

0 commit comments

Comments
 (0)