8000 bug #31425 [Messenger] On failure retry, make message appear received… · symfony/symfony@7800396 · GitHub
[go: up one dir, main page]
Skip to content

Commit 7800396

Browse files
fabpotfabpot
committed
bug #31425 [Messenger] On failure retry, make message appear received from original sender (weaverryan)
This PR was squashed before being merged into the 4.3 branch (closes #31425). Discussion ---------- [Messenger] On failure retry, make message appear received from original sender | Q | A | ------------- | --- | Branch? | master (4.3) | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #31413 | License | MIT | Doc PR | symfony/symfony-docs#11236 Fixes a bug when using the transport-based handler config from #30958. This also adds a pretty robust integration test that dispatches a complex message with transport-based handler config, failures, failure transport, etc - and verifies the correct behavior. Cheers! Commits ------- 80b5df2 [Messenger] On failure retry, make message appear received from original sender
2 parents fe0f324 + 80b5df2 commit 7800396

16 files changed

+410
-70
lines changed

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1659,6 +1659,7 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
16591659
'before' => [
16601660
['id' => 'add_bus_name_stamp_middleware'],
16611661
['id' => 'dispatch_after_current_bus'],
1662+
['id' => 'failed_message_processing_middleware'],
16621663
],
16631664
'after' => [
16641665
['id' => 'send_message'],

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@
4848
<argument type="service" id="validator" />
4949
</service>
5050

51+
<service id="messenger.middleware.failed_message_processing_middleware" class="Symfony\Component\Messenger\Middleware\FailedMessageProcessingMiddleware" />
52+
5153
<service id="messenger.middleware.traceable" class="Symfony\Component\Messenger\Middleware\TraceableMiddleware" abstract="true">
5254
<argument type="service" id="debug.stopwatch" />
5355
</service>

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -753,6 +753,7 @@ public function testMessengerWithMultipleBuses()
753753
$this->assertEquals([
754754
['id' => 'add_bus_name_stamp_middleware', 'arguments' => ['messenger.bus.commands']],
755755
['id' => 'dispatch_after_current_bus'],
756+
['id' => 'failed_message_processing_middleware'],
756757
['id' => 'send_message'],
757758
['id' => 'handle_message'],
758759
], $container->getParameter('messenger.bus.commands.middleware'));
@@ -761,6 +762,7 @@ public function testMessengerWithMultipleBuses()
761762
$this->assertEquals([
762763
['id' => 'add_bus_name_stamp_middleware', 'arguments' => ['messenger.bus.events']],
763764
['id' => 'dispatch_after_current_bus'],
765+
['id' => 'failed_message_processing_middleware'],
764766
['id' => 'with_factory', 'arguments' => ['foo', true, ['bar' => 'baz']]],
765767
['id' => 'send_message'],
766768
['id' => 'handle_message'],

‎src/Symfony/Component/Messenger/Command/AbstractFailedMessagesCommand.php

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Component\Console\Helper\Dumper;
1616
use Symfony\Component\Console\Style\SymfonyStyle;
1717
use Symfony\Component\Messenger\Envelope;
18+
use Symfony\Component\Messenger\Stamp\RedeliveryStamp;
1819
use Symfony\Component\Messenger\Stamp\SentToFailureTransportStamp;
1920
use Symfony\Component\Messenger\Stamp\TransportMessageIdStamp;
2021
use Symfony\Component\Messenger\Transport\Receiver\MessageCountAwareInterface;
@@ -59,8 +60,10 @@ protected function displaySingleMessage(Envelope $envelope, SymfonyStyle $io)
5960
{
6061
$io->title('Failed Message Details');
6162

62-
/** @var SentToFailureTransportStamp $sentToFailureTransportStamp */
63+
/** @var SentToFailureTransportStamp|null $sentToFailureTransportStamp */
6364
$sentToFailureTransportStamp = $envelope->last(SentToFailureTransportStamp::class);
65+
/** @var RedeliveryStamp|null $lastRedeliveryStamp */
66+
$lastRedeliveryStamp = $envelope->last(RedeliveryStamp::class);
6467

6568
$rows = [
6669
['Class', \get_class($envelope->getMessage())],
@@ -70,25 +73,34 @@ protected function displaySingleMessage(Envelope $envelope, SymfonyStyle $io)
7073
$rows[] = ['Message Id', $id];
7174
}
7275

76+
$flattenException = null === $lastRedeliveryStamp ? null : $lastRedeliveryStamp->getFlattenException();
7377
if (null === $sentToFailureTransportStamp) {
7478
$io->warning('Message does not appear to have been sent to this transport after failing');
7579
} else {
7680
$rows = array_merge($rows, [
77-
['Failed at', $sentToFailureTransportStamp->getSentAt()->format('Y-m-d H:i:s')],
78-
['Error', $sentToFailureTransportStamp->getExceptionMessage()],
79-
['Error Class', $sentToFailureTransportStamp->getFlattenException() ? $sentToFailureTransportStamp->getFlattenException()->getClass() : '(unknown)'],
81+
['Failed at', null === $lastRedeliveryStamp ? '' : $lastRedeliveryStamp->getRedeliveredAt()->format('Y-m-d H:i:s')],
82+
['Error', null === $lastRedeliveryStamp ? '' : $lastRedeliveryStamp->getExceptionMessage()],
83+
['Error Class', null === $flattenException ? '(unknown)' : $flattenException->getClass()],
8084
['Transport', $sentToFailureTransportStamp->getOriginalReceiverName()],
8185
]);
8286
}
8387

8488
$io->table([], $rows);
8589

90+
/** @var RedeliveryStamp[] $redeliveryStamps */
91+
$redeliveryStamps = $envelope->all(RedeliveryStamp::class);
92+
$io->writeln(' Message history:');
93+
foreach ($redeliveryStamps as $redeliveryStamp) {
94+
$io->writeln(sprintf(' * Message failed and redelivered to the <info>%s</info> transport at <info>%s</info>', $redeliveryStamp->getSenderClassOrAlias(), $redeliveryStamp->getRedeliveredAt()->format('Y-m-d H:i:s')));
95+
}
96+
$io->newLine();
97+
8698
if ($io->isVeryVerbose()) {
8799
$io->title('Message:');
88100
$dump = new Dumper($io);
89101
$io->writeln($dump($envelope->getMessage()));
90102
$io->title('Exception:');
91-
$io->writeln($sentToFailureTransportStamp->getFlattenException()->getTraceAsString());
103+
$io->writeln(null === $flattenException ? '(no data)' : $flattenException->getTraceAsString());
92104
} else {
93105
$io->writeln(' Re-run command with <info>-vv</info> to see more message & error details.');
94106
}

‎src/Symfony/Component/Messenger/Command/FailedMessagesRetryCommand.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ private function runInteractive(SymfonyStyle $io, bool $shouldForce)
154154
}
155155

156156
// avoid success message if nothing was processed
157-
if (1 < $count) {
157+
if (1 <= $count) {
158158
$io->success('All failed messages have been handled or removed!');
159159
}
160160
}

‎src/Symfony/Component/Messenger/Command/FailedMessagesShowCommand.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
use Symfony\Component\Console\Output\ConsoleOutputInterface;
1919
use Symfony\Component\Console\Output\OutputInterface;
2020
use Symfony\Component\Console\Style\SymfonyStyle;
21-
use Symfony\Component\Messenger\Stamp\SentToFailureTransportStamp;
21+
use Symfony\Component\Messenger\Stamp\RedeliveryStamp;
2222
use Symfony\Component\Messenger\Transport\Receiver\ListableReceiverInterface;
2323

2424
/**
@@ -83,14 +83,14 @@ private function listMessages(SymfonyStyle $io, int $max)
8383

8484
$rows = [];
8585
foreach ($envelopes as $envelope) {
86-
/** @var SentToFailureTransportStamp $sentToFailureTransportStamp */
87-
$sentToFailureTransportStamp = $envelope->last(SentToFailureTransportStamp::class);
86+
/** @var RedeliveryStamp|null $lastRedeliveryStamp */
87+
$lastRedeliveryStamp = $envelope->last(RedeliveryStamp::class);
8888

8989
$rows[] = [
9090
$this->getMessageId($envelope),
9191
\get_class($envelope->getMessage()),
92-
null === $sentToFailureTransportStamp ? '' : $sentToFailureTransportStamp->getSentAt()->format('Y-m-d H:i:s'),
93-
null === $sentToFailureTransportStamp ? '' : $sentToFailureTransportStamp->getExceptionMessage(),
92+
null === $lastRedeliveryStamp ? '' : $lastRedeliveryStamp->getRedeliveredAt()->format('Y-m-d H:i:s'),
93+
null === $lastRedeliveryStamp ? '' : $lastRedeliveryStamp->getExceptionMessage(),
9494
];
9595
}
9696

‎src/Symfony/Component/Messenger/EventListener/SendFailedMessageToFailureTransportListener.php

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616
use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent;
1717
use Symfony\Component\Messenger\Exception\HandlerFailedException;
1818
use Symfony\Component\Messenger\MessageBusInterface;
19+
use Symfony\Component\Messenger\Stamp\DelayStamp;
1920
use Symfony\Component\Messenger\Stamp\ReceivedStamp;
2021
use Symfony\Component\Messenger\Stamp\RedeliveryStamp;
21-
use Symfony\Component\Messenger\Stamp\SentStamp;
2222
use Symfony\Component\Messenger\Stamp\SentToFailureTransportStamp;
2323
use Symfony\Component\Messenger\Stamp\TransportMessageIdStamp;
2424

@@ -51,11 +51,8 @@ public function onMessageFailed(WorkerMessageFailedEvent $event)
5151
$envelope = $event->getEnvelope();
5252

5353
// avoid re-sending to the failed sender
54-
foreach ($envelope->all(SentStamp::class) as $sentStamp) {
55-
/** @var SentStamp $sentStamp */
56-
if ($sentStamp->getSenderAlias() === $this->failureSenderAlias) {
57-
return;
58-
}
54+
if (null !== $envelope->last(SentToFailureTransportStamp::class)) {
55+
return;
5956
}
6057

6158
// remove the received stamp so it's redelivered
@@ -67,8 +64,9 @@ public function onMessageFailed(WorkerMessageFailedEvent $event)
6764
$flattenedException = \class_exists(FlattenException::class) ? FlattenException::createFromThrowable($throwable) : null;
6865
$envelope = $envelope->withoutAll(ReceivedStamp::class)
6966
->withoutAll(TransportMessageIdStamp::class)
70-
->with(new SentToFailureTransportStamp($throwable->getMessage(), $event->getReceiverName(), $flattenedException))
71-
->with(new RedeliveryStamp(0, $this->failureSenderAlias));
67+
->with(new SentToFailureTransportStamp($event->getReceiverName()))
68+
->with(new DelayStamp(0))
69+
->with(new RedeliveryStamp(0, $this->failureSenderAlias, $throwable->getMessage(), $flattenedException));
7270

7371
if (null !== $this->logger) {
7472
$this->logger->info('Rejected message {class} will be sent to the failure transport {transport}.', [

‎src/Symfony/Component/Messenger/Middleware/FailedMessageProcessingMiddleware.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Messenger\Middleware;
13+
14+
use Symfony\Component\Messenger\Envelope;
15+
use Symfony\Component\Messenger\Stamp\ReceivedStamp;
16+
use Symfony\Component\Messenger\Stamp\SentToFailureTransportStamp;
17+
18+
/**
19+
* @author Ryan Weaver <ryan@symfonycasts.com>
20+
*
21+
* @experimental in 4.3
22+
*/
23+
class FailedMessageProcessingMiddleware implements MiddlewareInterface
24+
{
25+
public function handle(Envelope $envelope, StackInterface $stack): Envelope
26+
{
27+
// look for "received" messages decorated with the SentToFailureTransportStamp
28+
/** @var SentToFailureTransportStamp|null $sentToFailureStamp */
29+
$sentToFailureStamp = $envelope->last(SentToFailureTransportStamp::class);
30+
if (null !== $sentToFailureStamp && null !== $envelope->last(ReceivedStamp::class)) {
31+
// mark the message as "received" from the original transport
32+
// this guarantees the same behavior as when originally received
33+
$envelope = $envelope->with(new ReceivedStamp($sentToFailureStamp->getOriginalReceiverName()));
34+
}
35+
36+
return $stack->next()->handle($envelope, $stack);
37+
}
38+
}

‎src/Symfony/Component/Messenger/Stamp/RedeliveryStamp.php

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\Messenger\Stamp;
1313

14+
use Symfony\Component\Debug\Exception\FlattenException;
15+
1416
/**
1517
* Stamp applied when a messages needs to be redelivered.
1618
*
@@ -20,14 +22,20 @@ class RedeliveryStamp implements StampInterface
2022
{
2123
private $retryCount;
2224
private $senderClassOrAlias;
25+
private $redeliveredAt;
26+
private $exceptionMessage;
27+
private $flattenException;
2328

2429
/**
2530
* @param string $senderClassOrAlias Alias from SendersLocator or just the class name
2631
*/
27-
public function __construct(int $retryCount, string $senderClassOrAlias)
32+
public function __construct(int $retryCount, string $senderClassOrAlias, string $exceptionMessage = null, FlattenException $flattenException = null)
2833
{
2934
$this->retryCount = $retryCount;
3035
$this->senderClassOrAlias = $senderClassOrAlias;
36+
$this->exceptionMessage = $exceptionMessage;
37+
$this->flattenException = $flattenException;
38+
$this->redeliveredAt = new \DateTimeImmutable();
3139
}
3240

3341
public function getRetryCount(): int
@@ -36,12 +44,27 @@ public function getRetryCount(): int
3644
}
3745

3846
/**
39-
* Needed for this class to serialize through Symfony's serializer.
47+
* The target sender this should be redelivered to.
4048
*
4149
* @internal
4250
*/
4351
public function getSenderClassOrAlias(): string
4452
{
4553
return $this->senderClassOrAlias;
4654
}
55+
56+
public function getExceptionMessage(): ?string
57+
{
58+
return $this->exceptionMessage;
59+
}
60+
61+
public function getFlattenException(): ?FlattenException
62+
{
63+
return $this->flattenException;
64+
}
65+
66+
public function getRedeliveredAt(): \DateTimeInterface
67+
{
68+
return $this->redeliveredAt;
69+
}
4770
}

‎src/Symfony/Component/Messenger/Stamp/SentToFailureTransportStamp.php

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111

1212
namespace Symfony\Component\Messenger\Stamp;
1313

14-
use Symfony\Component\Debug\Exception\FlattenException;
15-
1614
/**
1715
* Stamp applied when a message is sent to the failure transport.
1816
*
@@ -22,36 +20,15 @@
2220
*/
2321
class SentToFailureTransportStamp implements StampInterface
2422
{
25-
private $exceptionMessage;
2623
private $originalReceiverName;
27-
private $flattenException;
28-
private $sentAt;
2924

30-
public function __construct(string $exceptionMessage, string $originalReceiverName, FlattenException $flattenException = null)
25+
public function __construct(string $originalReceiverName)
3126
{
32-
$this->exceptionMessage = $exceptionMessage;
3327
$this->originalReceiverName = $originalReceiverName;
34-
$this->flattenException = $flattenException;
35-
$this->sentAt = new \DateTimeImmutable();
36-
}
37-
38-
public function getExceptionMessage(): string
39-
{
40-
return $this->exceptionMessage;
4128
}
4229

4330
public function getOriginalReceiverName(): string
4431
{
4532
return $this->originalReceiverName;
4633
}
47-
48-
public function getFlattenException(): ?FlattenException
49-
{
50-
return $this->flattenException;
51-
}
52-
53-
public function getSentAt(): \DateTimeInterface
54-
{
55-
return $this->sentAt;
56-
}
5734
}

‎src/Symfony/Component/Messenger/Tests/Command/FailedMessagesShowCommandTest.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Component\Console\Tester\CommandTester;
1616
use Symfony\Component\Messenger\Command\FailedMessagesShowCommand;
1717
use Symfony\Component\Messenger\Envelope;
18+
use Symfony\Component\Messenger\Stamp\RedeliveryStamp;
1819
use Symfony\Component\Messenger\Stamp\SentToFailureTransportStamp;
1920
use Symfony\Component\Messenger\Stamp\TransportMessageIdStamp;
2021
use Symfony\Component\Messenger\Transport\Receiver\ListableReceiverInterface;
@@ -26,10 +27,12 @@ class FailedMessagesShowCommandTest extends TestCase
2627
{
2728
public function testBasicRun()
2829
{
29-
$sentToFailureStamp = new SentToFailureTransportStamp('Things are bad!', 'async');
30+
$sentToFailureStamp = new SentToFailureTransportStamp('async');
31+
$redeliveryStamp = new RedeliveryStamp(0, 'failure_receiver', 'Things are bad!');
3032
$envelope = new Envelope(new \stdClass(), [
3133
new TransportMessageIdStamp(15),
3234
$sentToFailureStamp,
35+
$redeliveryStamp,
3336
]);
3437
$receiver = $this->createMock(ListableReceiverInterface::class);
3538
$receiver->expects($this->once())->method('find')->with(15)->willReturn($envelope);
@@ -48,10 +51,11 @@ public function testBasicRun()
4851
Message Id 15
4952
Failed at %s
5053
Error Things are bad!
51-
Error Class (unknown)
54+
Error Class (unknown)
55+
Transport async
5256
EOF
5357
,
54-
$sentToFailureStamp->getSentAt()->format('Y-m-d H:i:s')),
58+
$redeliveryStamp->getRedeliveredAt()->format('Y-m-d H:i:s')),
5559
$tester->getDisplay(true));
5660
}
5761
}

‎src/Symfony/Component/Messenger/Tests/EventListener/SendFailedMessageToFailureTransportListenerTest.php

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
use Symfony\Component\Messenger\MessageBusInterface;
2020
use Symfony\Component\Messenger\Stamp\ReceivedStamp;
2121
use Symfony\Component\Messenger\Stamp\RedeliveryStamp;
22-
use Symfony\Component\Messenger\Stamp\SentStamp;
2322
use Symfony\Component\Messenger\Stamp\SentToFailureTransportStamp;
2423
use Symfony\Component\Messenger\Stamp\TransportMessageIdStamp;
2524

@@ -35,13 +34,13 @@ public function testItDispatchesToTheFailureTransport()
3534
/** @var SentToFailureTransportStamp $sentToFailureTransportStamp */
3635
$sentToFailureTransportStamp = $envelope->last(SentToFailureTransportStamp::class);
3736
$this->assertNotNull($sentToFailureTransportStamp);
38-
$this->assertSame('no!', $sentToFailureTransportStamp->getExceptionMessage());
3937
$this->assertSame('my_receiver', $sentToFailureTransportStamp->getOriginalReceiverName());
40-
$this->assertSame('no!', $sentToFailureTransportStamp->getFlattenException()->getMessage());
4138

4239
/** @var RedeliveryStamp $redeliveryStamp */
4340
$redeliveryStamp = $envelope->last(RedeliveryStamp::class);
4441
$this->assertSame('failure_sender', $redeliveryStamp->getSenderClassOrAlias());
42+
$this->assertSame('no!', $redeliveryStamp->getExceptionMessage());
43+
$this->assertSame('no!', $redeliveryStamp->getFlattenException()->getMessage());
4544

4645
$this->assertNull($envelope->last(ReceivedStamp::class));
4746
$this->assertNull($envelope->last(TransportMessageIdStamp::class));
@@ -65,11 +64,11 @@ public function testItGetsNestedHandlerFailedException()
6564
$bus = $this->createMock(MessageBusInterface::class);
6665
$bus->expects($this->once())->method('dispatch')->with($this->callback(function ($envelope) {
6766
/** @var Envelope $envelope */
68-
/** @var SentToFailureTransportStamp $sentToFailureTransportStamp */
69-
$sentToFailureTransportStamp = $envelope->last(SentToFailureTransportStamp::class);
70-
$this->assertNotNull($sentToFailureTransportStamp);
71-
$this->assertSame('I am inside!', $sentToFailureTransportStamp->getExceptionMessage());
72-
$this->assertSame('Exception', $sentToFailureTransportStamp->getFlattenException()->getClass());
67+
/** @var RedeliveryStamp $redeliveryStamp */
68+
$redeliveryStamp = $envelope->last(RedeliveryStamp::class);
69+
$this->assertNotNull($redeliveryStamp);
70+
$this->assertSame('I am inside!', $redeliveryStamp->getExceptionMessage());
71+
$this->assertSame('Exception', $redeliveryStamp->getFlattenException()->getClass());
7372

7473
return true;
7574
}))->willReturn(new Envelope(new \stdClass()));
@@ -112,7 +111,7 @@ public function testDoNotRedeliverToFailed()
112111
);
113112

114113
$envelope = new Envelope(new \stdClass(), [
115-
new SentStamp('MySender', 'failure_sender'),
114+
new SentToFailureTransportStamp('my_receiver'),
116115
]);
117116
$event = new WorkerMessageFailedEvent($envelope, 'my_receiver', new \Exception(''), false);
118117

0 commit comments

Comments
 (0)
0