From 49bb7435f1939d8d1d1114b872cd22bc13a22e99 Mon Sep 17 00:00:00 2001 From: Piet Steinhart Date: Wed, 17 Jul 2019 22:40:10 +0200 Subject: [PATCH] [Messenger] fixed UnrecoverableExceptionInterface handling in Worker (fixes #32325) --- .../Component/Messenger/Tests/WorkerTest.php | 32 +++++++++++++++++++ src/Symfony/Component/Messenger/Worker.php | 14 ++++++++ 2 files changed, 46 insertions(+) diff --git a/src/Symfony/Component/Messenger/Tests/WorkerTest.php b/src/Symfony/Component/Messenger/Tests/WorkerTest.php index c82652fe1d29..ac8c2a7ad840 100644 --- a/src/Symfony/Component/Messenger/Tests/WorkerTest.php +++ b/src/Symfony/Component/Messenger/Tests/WorkerTest.php @@ -17,6 +17,7 @@ use Symfony\Component\Messenger\Event\WorkerMessageHandledEvent; use Symfony\Component\Messenger\Event\WorkerMessageReceivedEvent; use Symfony\Component\Messenger\Event\WorkerStoppedEvent; +use Symfony\Component\Messenger\Exception\HandlerFailedException; use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException; use Symfony\Component\Messenger\MessageBusInterface; use Symfony\Component\Messenger\Retry\RetryStrategyInterface; @@ -120,6 +121,37 @@ public function testDispatchCausesRetry() $this->assertSame(1, $receiver->getAcknowledgeCount()); } + public function testUnrecoverableMessageHandlingExceptionPreventsRetries() + { + $envelope1 = new Envelope(new DummyMessage('Unwrapped Exception'), [new SentStamp('Some\Sender', 'transport1')]); + $envelope2 = new Envelope(new DummyMessage('Wrapped Exception'), [new SentStamp('Some\Sender', 'transport1')]); + + $receiver = new DummyReceiver([ + [$envelope1], + [$envelope2], + ]); + + $bus = $this->getMockBuilder(MessageBusInterface::class)->getMock(); + $bus->expects($this->at(0))->method('dispatch')->willThrowException(new UnrecoverableMessageHandlingException()); + $bus->expects($this->at(1))->method('dispatch')->willThrowException( + new HandlerFailedException($envelope2, [new UnrecoverableMessageHandlingException()]) + ); + + $retryStrategy = $this->getMockBuilder(RetryStrategyInterface::class)->getMock(); + $retryStrategy->expects($this->never())->method('isRetryable')->willReturn(true); + + $worker = new Worker(['transport1' => $receiver], $bus, ['transport1' => $retryStrategy]); + $worker->run([], function (?Envelope $envelope) use ($worker) { + // stop after the messages finish + if (null === $envelope) { + $worker->stop(); + } + }); + + // message was rejected + $this->assertSame(2, $receiver->getRejectCount()); + } + public function testDispatchCausesRejectWhenNoRetry() { $receiver = new DummyReceiver([ diff --git a/src/Symfony/Component/Messenger/Worker.php b/src/Symfony/Component/Messenger/Worker.php index a51cd3506fe5..d0c98b76ff05 100644 --- a/src/Symfony/Component/Messenger/Worker.php +++ b/src/Symfony/Component/Messenger/Worker.php @@ -191,6 +191,20 @@ private function dispatchEvent($event) private function shouldRetry(\Throwable $e, Envelope $envelope, RetryStrategyInterface $retryStrategy): bool { + // if ALL nested Exceptions are an instance of UnrecoverableExceptionInterface we should not retry + if ($e instanceof HandlerFailedException) { + $shouldNotRetry = true; + foreach ($e->getNestedExceptions() as $nestedException) { + if (!$nestedException instanceof UnrecoverableExceptionInterface) { + $shouldNotRetry = false; + break; + } + } + if ($shouldNotRetry) { + return false; + } + } + if ($e instanceof UnrecoverableExceptionInterface) { return false; }