Skip to content

Commit

Permalink
feature #36185 [Messenger] Add a \Throwable argument in RetryStrategy…
Browse files Browse the repository at this point in the history
…Interface methods (Benjamin Dos Santos)

This PR was squashed before being merged into the 5.1-dev branch.

Discussion
----------

[Messenger] Add a \Throwable argument in RetryStrategyInterface methods

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix #36182
| License       | MIT

This allows to define new retry strategies based on the exceptions thrown during the last handling.

Commits
-------

5fa9d68 [Messenger] Add a \Throwable argument in RetryStrategyInterface methods
  • Loading branch information
fabpot committed Apr 4, 2020
2 parents 5aeecc2 + 5fa9d68 commit fdd8ac5
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 6 deletions.
2 changes: 2 additions & 0 deletions UPGRADE-5.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
--------
Expand Down
2 changes: 2 additions & 0 deletions UPGRADE-6.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------------
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/Messenger/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

0 comments on commit fdd8ac5

Please sign in to comment.