bug symfony#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 symfony#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`

Author: fabpot

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)) {

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!');