8000 bug #34082 Revert "[Messenger] Fix exception message of failed messag… · symfony/symfony@2e17488 · GitHub
[go: up one dir, main page]
Skip to content

Commit 2e17488

Browse files
TobionTobion
committed
bug #34082 Revert "[Messenger] Fix exception message of failed message is dropped (Tobion)
This PR was merged into the 4.3 branch. Discussion ---------- Revert "[Messenger] Fix exception message of failed message is dropped | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | | License | MIT | Doc PR | This reverts #33600 because it makes the message grow for each retry until AMQP cannot handle it anymore. On each retry, the full exception trace is added to the message. So in our case on the 5th retry, the message is too big for the AMQP library to encode it. AMQP extension then throws the exception > Library error: table too large for buffer (ref. alanxz/rabbitmq-c#224 and php-amqp/php-amqp#131) when trying to publish the message. To solve this, I suggest to revert #33600 (this PR) and merge #32341 instead which does not re-add the exception on each failure. Btw, the above problem causes other problematic side-effects of Symfony messenger. As the new retry message fails to be published with an exception, the old (currently processed message) also does not get removed (acknowledged) from the delay queue. So rabbitmq redelivers the message and the same thing happens forever. This can block the consumers and have a huge toll on your service. That's just another case for #32055 (comment). I'll try to fix this in another PR. Commits ------- 3dbe924 Revert "[Messenger] Fix exception message of failed message is dropped on retry"
2 parents 5da67f9 + 3dbe924 commit 2e17488

File tree

2 files changed

+8
-61
lines changed

2 files changed

+8
-61
lines changed

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

Lines changed: 7 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,12 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Console\Tester\CommandTester;
16-
use Symfony\Component\Debug\Exception\FlattenException;
1716
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
1817
use Symfony\Component\Messenger\Command\FailedMessagesRetryCommand;
1918
use Symfony\Component\Messenger\Envelope;
2019
use Symfony\Component\Messenger\MessageBusInterface;
21-
use Symfony\Component\Messenger\Retry\RetryStrategyInterface;
22-
use Symfony\Component\Messenger\Stamp\ReceivedStamp;
23-
use Symfony\Component\Messenger\Stamp\RedeliveryStamp;
2420
use Symfony\Component\Messenger\Transport\Receiver\ListableReceiverInterface;
21+
use Symfony\Component\Messenger\Worker;
2522

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

40-
$command = new FailedMessagesRetryCommand('failure_receiver', $receiver, $bus, $dispatcher);
37+
$command = new FailedMessagesRetryCommand(
38+
'failure_receiver',
39+
$receiver,
40+
$bus,
41+
$dispatcher
42+
);
4143

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

4547
$this->assertStringContainsString('[OK]', $tester->getDisplay());
4648
}
47-
48-
public function testExceptionOnRetry()
49-
{
50-
$receiver = $this->createMock(ListableReceiverInterface::class);
51-
$receiver->expects($this->once())->method('find')->with(10)->willReturn(new Envelope(new \stdClass()));
52-
// message will eventually be ack'ed in Worker
53-
$receiver->expects($this->once())->method('ack');
54-
55-
$dispatcher = $this->createMock(EventDispatcherInterface::class);
56-
$bus = $this->createMock(MessageBusInterface::class);
57-
// the bus should be called in the worker
58-
$bus->expects($this->at(0))
59-
->method('dispatch')
60-
->with($this->callback(function (Envelope $envelope) {
61-
$lastReceivedStamp = $envelope->last(ReceivedStamp::class);
62-
63-
return $lastReceivedStamp instanceof ReceivedStamp && \is_string($lastReceivedStamp->getTransportName());
64-
}))
65-
->will($this->throwException(new \Exception('Mock test exception')));
66-
67-
$bus->expects($this->at(1))
68-
->method('dispatch')
69-
->with($this->callback(function (Envelope $envelope) {
70-
$lastRedeliveryStamp = $envelope->last(RedeliveryStamp::class);
71-
72-
return $lastRedeliveryStamp instanceof RedeliveryStamp &&
73-
\is_string($lastRedeliveryStamp->getExceptionMessage()) &&
74-
$lastRedeliveryStamp->getFlattenException() instanceof< 8000 /span> FlattenException;
75-
}))
76-
->willReturn(new Envelope(new \stdClass()));
77-
78-
$retryStrategy = $this->createMock(RetryStrategyInterface::class);
79-
$retryStrategy->expects($this->once())->method('isRetryable')->with($this->isInstanceOf(Envelope::class))->willReturn(true);
80-
81-
$command = new FailedMessagesRetryCommand('failure_receiver', $receiver, $bus, $dispatcher, $retryStrategy);
82-
83-
$tester = new CommandTester($command);
84-
$tester->execute(['id' => [10]]);
85-
86-
$this->assertStringContainsString('[OK]', $tester->getDisplay());
87-
}
8849
}

‎src/Symfony/Component/Messenger/Worker.php

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
namespace Symfony\Component\Messenger;
1313

1414
use Psr\Log\LoggerInterface;
15-
use Symfony\Component\Debug\Exception\FlattenException;
1615
use Symfony\Component\EventDispatcher\LegacyEventDispatcherProxy;
1716
use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent;
1817
use Symfony\Component\Messenger\Event\WorkerMessageHandledEvent;
@@ -154,7 +153,7 @@ private function handleMessage(Envelope $envelope, ReceiverInterface $receiver,
154153

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

160159
// re-send the message
@@ -217,17 +216,4 @@ private function shouldRetry(\Throwable $e, Envelope $envelope, RetryStrategyInt
217216

218217
return $retryStrategy->isRetryable($envelope);
219218
}
220-
221-
private function flattenedException(\Throwable $throwable): ?FlattenException
222-
{
223-
if (!class_exists(FlattenException::class)) {
224-
return null;
225-
}
226-
227-
if ($throwable instanceof HandlerFailedException) {
228-
$throwable = $throwable->getNestedExceptions()[0];
229-
}
230-
231-
return FlattenException::createFromThrowable($throwable);
232-
}
233219
}

0 commit comments

Comments
 (0)
0