Skip to content

Commit

Permalink
bug #33600 [Messenger] Fix exception message of failed message is dro…
Browse files Browse the repository at this point in the history
…pped on retry (tienvx)

This PR was merged into the 4.3 branch.

Discussion
----------

[Messenger] Fix exception message of failed message is dropped on retry

| Q             | A
| ------------- | ---
| Branch?       | 4.3 <!-- see below -->
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #32719
| License       | MIT
| Doc PR        | NA <!-- required for new features -->
<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.

Additionally (see https://symfony.com/roadmap):
 - Always add tests and ensure they pass.
 - Never break backward compatibility (see https://symfony.com/bc).
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too.)
 - Features and deprecations must be submitted against branch 4.4.
 - Legacy code removals go to the master branch.
-->

Commits
-------

8f9f44e [Messenger] Fix exception message of failed message is dropped on retry
  • Loading branch information
fabpot committed Sep 17, 2019
2 parents 23f64d5 + 8f9f44e commit 2ae76f9
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 8 deletions.
Expand Up @@ -13,12 +13,15 @@

use PHPUnit\Framework\TestCase;
use Symfony\Component\Console\Tester\CommandTester;
use Symfony\Component\Debug\Exception\FlattenException;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\Messenger\Command\FailedMessagesRetryCommand;
use Symfony\Component\Messenger\Envelope;
use Symfony\Component\Messenger\MessageBusInterface;
use Symfony\Component\Messenger\Retry\RetryStrategyInterface;
use Symfony\Component\Messenger\Stamp\ReceivedStamp;
use Symfony\Component\Messenger\Stamp\RedeliveryStamp;
use Symfony\Component\Messenger\Transport\Receiver\ListableReceiverInterface;
use Symfony\Component\Messenger\Worker;

class FailedMessagesRetryCommandTest extends TestCase
{
Expand All @@ -34,16 +37,52 @@ public function testBasicRun()
// the bus should be called in the worker
$bus->expects($this->exactly(2))->method('dispatch')->willReturn(new Envelope(new \stdClass()));

$command = new FailedMessagesRetryCommand(
'failure_receiver',
$receiver,
$bus,
$dispatcher
);
$command = new FailedMessagesRetryCommand('failure_receiver', $receiver, $bus, $dispatcher);

$tester = new CommandTester($command);
$tester->execute(['id' => [10, 12]]);

$this->assertStringContainsString('[OK]', $tester->getDisplay());
}

public function testExceptionOnRetry()
{
$receiver = $this->createMock(ListableReceiverInterface::class);
$receiver->expects($this->once())->method('find')->with(10)->willReturn(new Envelope(new \stdClass()));
// message will eventually be ack'ed in Worker
$receiver->expects($this->once())->method('ack');

$dispatcher = $this->createMock(EventDispatcherInterface::class);
$bus = $this->createMock(MessageBusInterface::class);
// the bus should be called in the worker
$bus->expects($this->at(0))
->method('dispatch')
->with($this->callback(function (Envelope $envelope) {
$lastReceivedStamp = $envelope->last(ReceivedStamp::class);

return $lastReceivedStamp instanceof ReceivedStamp && \is_string($lastReceivedStamp->getTransportName());
}))
->will($this->throwException(new \Exception('Mock test exception')));

$bus->expects($this->at(1))
->method('dispatch')
->with($this->callback(function (Envelope $envelope) {
$lastRedeliveryStamp = $envelope->last(RedeliveryStamp::class);

return $lastRedeliveryStamp instanceof RedeliveryStamp &&
\is_string($lastRedeliveryStamp->getExceptionMessage()) &&
$lastRedeliveryStamp->getFlattenException() instanceof FlattenException;
}))
->willReturn(new Envelope(new \stdClass()));

$retryStrategy = $this->createMock(RetryStrategyInterface::class);
$retryStrategy->expects($this->once())->method('isRetryable')->with($this->isInstanceOf(Envelope::class))->willReturn(true);

$command = new FailedMessagesRetryCommand('failure_receiver', $receiver, $bus, $dispatcher, $retryStrategy);

$tester = new CommandTester($command);
$tester->execute(['id' => [10]]);

$this->assertStringContainsString('[OK]', $tester->getDisplay());
}
}
16 changes: 15 additions & 1 deletion src/Symfony/Component/Messenger/Worker.php
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\Messenger;

use Psr\Log\LoggerInterface;
use Symfony\Component\Debug\Exception\FlattenException;
use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent;
use Symfony\Component\Messenger\Event\WorkerMessageHandledEvent;
use Symfony\Component\Messenger\Event\WorkerMessageReceivedEvent;
Expand Down Expand Up @@ -152,7 +153,7 @@ private function handleMessage(Envelope $envelope, ReceiverInterface $receiver,

// add the delay and retry stamp info + remove ReceivedStamp
$retryEnvelope = $envelope->with(new DelayStamp($delay))
->with(new RedeliveryStamp($retryCount, $transportName))
->with(new RedeliveryStamp($retryCount, $transportName, $throwable->getMessage(), $this->flattenedException($throwable)))
->withoutAll(ReceivedStamp::class);

// re-send the message
Expand Down Expand Up @@ -215,4 +216,17 @@ private function shouldRetry(\Throwable $e, Envelope $envelope, RetryStrategyInt

return $retryStrategy->isRetryable($envelope);
}

private function flattenedException(\Throwable $throwable): ?FlattenException
{
if (!class_exists(FlattenException::class)) {
return null;
}

if ($throwable instanceof HandlerFailedException) {
$throwable = $throwable->getNestedExceptions()[0];
}

return FlattenException::createFromThrowable($throwable);
}
}

0 comments on commit 2ae76f9

Please sign in to comment.