From 4a02222bc56223a1cca1dba39d01f6414e81236c Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Wed, 15 Jan 2025 14:41:30 +0100 Subject: [PATCH] [Messenger ] Extract retry delay from nested `RecoverableExceptionInterface` --- .../SendFailedMessageForRetryListener.php | 31 ++++++++-- .../SendFailedMessageForRetryListenerTest.php | 57 +++++++++++++++++++ 2 files changed, 82 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Component/Messenger/EventListener/SendFailedMessageForRetryListener.php b/src/Symfony/Component/Messenger/EventListener/SendFailedMessageForRetryListener.php index f6334173972af..5f3ee445920cd 100644 --- a/src/Symfony/Component/Messenger/EventListener/SendFailedMessageForRetryListener.php +++ b/src/Symfony/Component/Messenger/EventListener/SendFailedMessageForRetryListener.php @@ -63,12 +63,7 @@ public function onMessageFailed(WorkerMessageFailedEvent $event): void ++$retryCount; - $delay = null; - if ($throwable instanceof RecoverableExceptionInterface && method_exists($throwable, 'getRetryDelay')) { - $delay = $throwable->getRetryDelay(); - } - - $delay ??= $retryStrategy->getWaitingTime($envelope, $throwable); + $delay = $this->getWaitingTime($envelope, $throwable, $retryStrategy); $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]); @@ -148,6 +143,30 @@ private function shouldRetry(\Throwable $e, Envelope $envelope, RetryStrategyInt return $retryStrategy->isRetryable($envelope, $e); } + private function getWaitingTime(Envelope $envelope, \Throwable $throwable, RetryStrategyInterface $retryStrategy): int + { + $delay = null; + if ($throwable instanceof RecoverableExceptionInterface && method_exists($throwable, 'getRetryDelay')) { + $delay = $throwable->getRetryDelay(); + } + + if ($throwable instanceof HandlerFailedException) { + foreach ($throwable->getWrappedExceptions() as $nestedException) { + if (!$nestedException instanceof RecoverableExceptionInterface + || !method_exists($nestedException, 'getRetryDelay') + || 0 > $retryDelay = $nestedException->getRetryDelay() ?? -1 + ) { + continue; + } + if ($retryDelay < ($delay ?? \PHP_INT_MAX)) { + $delay = $retryDelay; + } + } + } + + return $delay ?? $retryStrategy->getWaitingTime($envelope, $throwable); + } + private function getRetryStrategyForTransport(string $alias): ?RetryStrategyInterface { if ($this->retryStrategyLocator->has($alias)) { diff --git a/src/Symfony/Component/Messenger/Tests/EventListener/SendFailedMessageForRetryListenerTest.php b/src/Symfony/Component/Messenger/Tests/EventListener/SendFailedMessageForRetryListenerTest.php index cf3c86d7f4ffb..793da81451aa5 100644 --- a/src/Symfony/Component/Messenger/Tests/EventListener/SendFailedMessageForRetryListenerTest.php +++ b/src/Symfony/Component/Messenger/Tests/EventListener/SendFailedMessageForRetryListenerTest.php @@ -18,6 +18,7 @@ use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent; use Symfony\Component\Messenger\Event\WorkerMessageRetriedEvent; use Symfony\Component\Messenger\EventListener\SendFailedMessageForRetryListener; +use Symfony\Component\Messenger\Exception\HandlerFailedException; use Symfony\Component\Messenger\Exception\RecoverableMessageHandlingException; use Symfony\Component\Messenger\Retry\RetryStrategyInterface; use Symfony\Component\Messenger\Stamp\DelayStamp; @@ -108,6 +109,62 @@ public function testRecoverableExceptionRetryDelayOverridesStrategy() $listener->onMessageFailed($event); } + /** + * @dataProvider provideRetryDelays + */ + public function testWrappedRecoverableExceptionRetryDelayOverridesStrategy(array $retries, int $expectedDelay) + { + $sender = $this->createMock(SenderInterface::class); + $sender->expects($this->once())->method('send')->willReturnCallback(function (Envelope $envelope) use ($expectedDelay) { + $delayStamp = $envelope->last(DelayStamp::class); + $redeliveryStamp = $envelope->last(RedeliveryStamp::class); + + $this->assertInstanceOf(DelayStamp::class, $delayStamp); + $this->assertSame($expectedDelay, $delayStamp->getDelay()); + + $this->assertInstanceOf(RedeliveryStamp::class, $redeliveryStamp); + $this->assertSame(1, $redeliveryStamp->getRetryCount()); + + return $envelope; + }); + $senderLocator = new Container(); + $senderLocator->set('my_receiver', $sender); + $retryStrategy = $this->createMock(RetryStrategyInterface::class); + $retryStrategy->expects($this->never())->method('isRetryable'); + $retryStrategy->expects($this->never())->method('getWaitingTime'); + $retryStrategyLocator = new Container(); + $retryStrategyLocator->set('my_receiver', $retryStrategy); + + $listener = new SendFailedMessageForRetryListener($senderLocator, $retryStrategyLocator); + + $envelope = new Envelope(new \stdClass()); + $exception = new HandlerFailedException( + $envelope, + array_map(fn (int $retry) => new RecoverableMessageHandlingException('retry', retryDelay: $retry), $retries) + ); + $event = new WorkerMessageFailedEvent($envelope, 'my_receiver', $exception); + + $listener->onMessageFailed($event); + } + + public static function provideRetryDelays(): iterable + { + yield 'one_exception' => [ + [1235], + 1235, + ]; + + yield 'multiple_exceptions' => [ + [1235, 2000, 1000], + 1000, + ]; + + yield 'zero_delay' => [ + [0, 2000, 1000], + 0, + ]; + } + public function testEnvelopeIsSentToTransportOnRetry() { $exception = new \Exception('no!');