[Messenger] Add a \Throwable argument in RetryStrategyInterface methods

symfony · fabpot · Apr 4, 2020 · Mar 24, 2020 · Apr 4, 2020 · 5fa9d68e8b071e90c26e2beba53c95e60f83e968
commit 5fa9d68e8b071e90c26e2beba53c95e60f83e968
diff --git a/UPGRADE-5.1.md b/UPGRADE-5.1.md
@@ -73,6 +73,8 @@ Messenger
 * Deprecated Doctrine transport. It has moved to a separate package. Run `composer require symfony/doctrine-messenger` to use the new classes.
 * Deprecated RedisExt transport. It has moved to a separate package. Run `composer require symfony/redis-messenger` to use the new classes.
 * Deprecated use of invalid options in Redis and AMQP connections.
+ * Deprecated *not* declaring a `\Throwable` argument in `RetryStrategyInterface::isRetryable()`
+ * Deprecated *not* declaring a `\Throwable` argument in `RetryStrategyInterface::getWaitingTime()`

 Notifier
 --------

diff --git a/UPGRADE-6.0.md b/UPGRADE-6.0.md
@@ -65,6 +65,8 @@ Messenger
 * Removed Doctrine transport. Run `composer require symfony/doctrine-messenger` to keep the transport in your application.
 * Removed RedisExt transport. Run `composer require symfony/redis-messenger` to keep the transport in your application.
 * Use of invalid options in Redis and AMQP connections now throws an error.
+ * The signature of method `RetryStrategyInterface::isRetryable()` has been updated to `RetryStrategyInterface::isRetryable(Envelope $message, \Throwable $throwable = null)`.
+ * The signature of method `RetryStrategyInterface::getWaitingTime()` has been updated to `RetryStrategyInterface::getWaitingTime(Envelope $message, \Throwable $throwable = null)`.

 PhpUnitBridge
 -------------

diff --git a/src/Symfony/Component/Messenger/CHANGELOG.md b/src/Symfony/Component/Messenger/CHANGELOG.md
@@ -7,6 +7,7 @@ CHANGELOG
 * Moved AmqpExt transport to package `symfony/amqp-messenger`. All classes in `Symfony\Component\Messenger\Transport\AmqpExt` have been moved to `Symfony\Component\Messenger\Bridge\Amqp\Transport`
 * Moved Doctrine transport to package `symfony/doctrine-messenger`. All classes in `Symfony\Component\Messenger\Transport\Doctrine` have been moved to `Symfony\Component\Messenger\Bridge\Doctrine\Transport`
 * Moved RedisExt transport to package `symfony/redis-messenger`. All classes in `Symfony\Component\Messenger\Transport\RedisExt` have been moved to `Symfony\Component\Messenger\Bridge\Redis\Transport`
+* Added support for passing a `\Throwable` argument to `RetryStrategyInterface` methods. This allows to define strategies based on the reason of the handling failure.

 5.0.0
 -----

diff --git a/src/Symfony/Component/Messenger/EventListener/SendFailedMessageForRetryListener.php b/src/Symfony/Component/Messenger/EventListener/SendFailedMessageForRetryListener.php
@@ -58,7 +58,9 @@ public function onMessageFailed(WorkerMessageFailedEvent $event)
            $event->setForRetry();

            ++$retryCount;
-            $delay = $retryStrategy->getWaitingTime($envelope);
+
+            $delay = $retryStrategy->getWaitingTime($envelope, $throwable);
+
            if (null !== $this->logger) {
                $this->logger->error('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]);
            }
@@ -103,7 +105,7 @@ private function shouldRetry(\Throwable $e, Envelope $envelope, RetryStrategyInt
            return false;
        }

-        return $retryStrategy->isRetryable($envelope);
+        return $retryStrategy->isRetryable($envelope, $e);
    }

    private function getRetryStrategyForTransport(string $alias): ?RetryStrategyInterface

diff --git a/src/Symfony/Component/Messenger/Retry/MultiplierRetryStrategy.php b/src/Symfony/Component/Messenger/Retry/MultiplierRetryStrategy.php
@@ -63,14 +63,20 @@ public function __construct(int $maxRetries = 3, int $delayMilliseconds = 1000,
        $this->maxDelayMilliseconds = $maxDelayMilliseconds;
    }

-    public function isRetryable(Envelope $message): bool
+    /**
+     * @param \Throwable|null $throwable The cause of the failed handling
+     */
+    public function isRetryable(Envelope $message, \Throwable $throwable = null): bool
    {
        $retries = RedeliveryStamp::getRetryCountFromEnvelope($message);

        return $retries < $this->maxRetries;
    }

-    public function getWaitingTime(Envelope $message): int
+    /**
+     * @param \Throwable|null $throwable The cause of the failed handling
+     */
+    public function getWaitingTime(Envelope $message, \Throwable $throwable = null): int
    {
        $retries = RedeliveryStamp::getRetryCountFromEnvelope($message);


diff --git a/src/Symfony/Component/Messenger/Retry/RetryStrategyInterface.php b/src/Symfony/Component/Messenger/Retry/RetryStrategyInterface.php
@@ -20,10 +20,15 @@
 */
 interface RetryStrategyInterface
 {
-    public function isRetryable(Envelope $message): bool;
+    /**
+     * @param \Throwable|null $throwable The cause of the failed handling
+     */
+    public function isRetryable(Envelope $message/*, \Throwable $throwable = null*/): bool;

    /**
+     * @param \Throwable|null $throwable The cause of the failed handling
+     *
     * @return int The time to delay/wait in milliseconds
     */
-    public function getWaitingTime(Envelope $message): int;
+    public function getWaitingTime(Envelope $message/*, \Throwable $throwable = null*/): int;
 }
diff --git a/...Symfony/Component/Messenger/Tests/EventListener/SendFailedMessageForRetryListenerTest.php b/...Symfony/Component/Messenger/Tests/EventListener/SendFailedMessageForRetryListenerTest.php
@@ -76,4 +76,41 @@ public function testEnvelopeIsSentToTransportOnRetry()

        $listener->onMessageFailed($event);
    }
+
+    public function testEnvelopeIsSentToTransportOnRetryWithExceptionPassedToRetryStrategy()
+    {
+        $exception = new \Exception('no!');
+        $envelope = new Envelope(new \stdClass());
+
+        $sender = $this->createMock(SenderInterface::class);
+        $sender->expects($this->once())->method('send')->willReturnCallback(function (Envelope $envelope) {
+            /** @var DelayStamp $delayStamp */
+            $delayStamp = $envelope->last(DelayStamp::class);
+            /** @var RedeliveryStamp $redeliveryStamp */
+            $redeliveryStamp = $envelope->last(RedeliveryStamp::class);
+
+            $this->assertInstanceOf(DelayStamp::class, $delayStamp);
+            $this->assertSame(1000, $delayStamp->getDelay());
+
+            $this->assertInstanceOf(RedeliveryStamp::class, $redeliveryStamp);
+            $this->assertSame(1, $redeliveryStamp->getRetryCount());
+
+            return $envelope;
+        });
+        $senderLocator = $this->createMock(ContainerInterface::class);
+        $senderLocator->expects($this->once())->method('has')->willReturn(true);
+        $senderLocator->expects($this->once())->method('get')->willReturn($sender);
+        $retryStategy = $this->createMock(RetryStrategyInterface::class);
+        $retryStategy->expects($this->once())->method('isRetryable')->with($envelope, $exception)->willReturn(true);
+        $retryStategy->expects($this->once())->method('getWaitingTime')->with($envelope, $exception)->willReturn(1000);
+        $retryStrategyLocator = $this->createMock(ContainerInterface::class);
+        $retryStrategyLocator->expects($this->once())->method('has')->willReturn(true);
+        $retryStrategyLocator->expects($this->once())->method('get')->willReturn($retryStategy);
+
+        $listener = new SendFailedMessageForRetryListener($senderLocator, $retryStrategyLocator);
+
+        $event = new WorkerMessageFailedEvent($envelope, 'my_receiver', $exception);
+
+        $listener->onMessageFailed($event);
+    }
 }