From 5fa9d68e8b071e90c26e2beba53c95e60f83e968 Mon Sep 17 00:00:00 2001 From: Benjamin Dos Santos Date: Tue, 24 Mar 2020 10:55:43 +0100 Subject: [PATCH] [Messenger] Add a \Throwable argument in RetryStrategyInterface methods --- UPGRADE-5.1.md | 2 + UPGRADE-6.0.md | 2 + src/Symfony/Component/Messenger/CHANGELOG.md | 1 + .../SendFailedMessageForRetryListener.php | 6 ++- .../Retry/MultiplierRetryStrategy.php | 10 ++++- .../Retry/RetryStrategyInterface.php | 9 ++++- .../SendFailedMessageForRetryListenerTest.php | 37 +++++++++++++++++++ 7 files changed, 61 insertions(+), 6 deletions(-) diff --git a/UPGRADE-5.1.md b/UPGRADE-5.1.md index 314eddedd3..4a6651eec3 100644 --- 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 index cda23de4a3..8e40b35d47 100644 --- 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 index aab9a173af..664ab03e49 100644 --- 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 index 5e654b51d4..1100bb6058 100644 --- a/src/Symfony/Component/Messenger/EventListener/SendFailedMessageForRetryListener.php +++ b/src/Symfony/Component/Messenger/EventListener/SendFailedMessageForRetryListener.php @@ -58,7 +58,9 @@ class SendFailedMessageForRetryListener implements EventSubscriberInterface $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 @@ class SendFailedMessageForRetryListener implements EventSubscriberInterface 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 index 5a4ec9a6b8..cf72f8614f 100644 --- a/src/Symfony/Component/Messenger/Retry/MultiplierRetryStrategy.php +++ b/src/Symfony/Component/Messenger/Retry/MultiplierRetryStrategy.php @@ -63,14 +63,20 @@ class MultiplierRetryStrategy implements RetryStrategyInterface $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 index 85f42a7b29..793ede0652 100644 --- a/src/Symfony/Component/Messenger/Retry/RetryStrategyInterface.php +++ b/src/Symfony/Component/Messenger/Retry/RetryStrategyInterface.php @@ -20,10 +20,15 @@ use Symfony\Component\Messenger\Envelope; */ 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/src/Symfony/Component/Messenger/Tests/EventListener/SendFailedMessageForRetryListenerTest.php b/src/Symfony/Component/Messenger/Tests/EventListener/SendFailedMessageForRetryListenerTest.php index 7008b48a09..627cec232e 100644 --- a/src/Symfony/Component/Messenger/Tests/EventListener/SendFailedMessageForRetryListenerTest.php +++ b/src/Symfony/Component/Messenger/Tests/EventListener/SendFailedMessageForRetryListenerTest.php @@ -76,4 +76,41 @@ class SendFailedMessageForRetryListenerTest extends TestCase $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); + } }