Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Messenger] [Amqp] Handle AMQPConnectionException when publishing a message. #54167

Merged
merged 1 commit into from Mar 5, 2024

Conversation

jwage
Copy link
Contributor

@jwage jwage commented Mar 5, 2024

Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #36538 Fix #48241
License MIT

If you have a message handler that dispatches messages to another queue, you can encounter AMQPConnectionException with the message "Library error: a SSL error occurred" or "a socket error occurred" depending on if you are using tls or not or if you are running behind a load balancer or not.

You can manually reproduce this issue by dispatching a message where the handler then dispatches another message to a different queue, then go to rabbitmq admin and close the connection manually, then dispatch another message and when the message handler goes to dispatch the other message, you will get this exception:

a socket error occurred
#0 /vagrant/vendor/symfony/amqp-messenger/Transport/AmqpTransport.php(60): Symfony\Component\Messenger\Bridge\Amqp\Transport\AmqpSender->send()
#1 /vagrant/vendor/symfony/messenger/Middleware/SendMessageMiddleware.php(62): Symfony\Component\Messenger\Bridge\Amqp\Transport\AmqpTransport->send()
#2 /vagrant/vendor/symfony/messenger/Middleware/FailedMessageProcessingMiddleware.php(34): Symfony\Component\Messenger\Middleware\SendMessageMiddleware->handle()
#3 /vagrant/vendor/symfony/messenger/Middleware/DispatchAfterCurrentBusMiddleware.php(61): Symfony\Component\Messenger\Middleware\FailedMessageProcessingMiddleware->handle()
#4 /vagrant/vendor/symfony/messenger/Middleware/RejectRedeliveredMessageMiddleware.php(41): Symfony\Component\Messenger\Middleware\DispatchAfterCurrentBusMiddleware->handle()
#5 /vagrant/vendor/symfony/messenger/Middleware/AddBusNameStampMiddleware.php(37): Symfony\Component\Messenger\Middleware\RejectRedeliveredMessageMiddleware->handle()
#6 /vagrant/vendor/symfony/messenger/Middleware/TraceableMiddleware.php(40): Symfony\Component\Messenger\Middleware\AddBusNameStampMiddleware->handle()
#7 /vagrant/vendor/symfony/messenger/MessageBus.php(70): Symfony\Component\Messenger\Middleware\TraceableMiddleware->handle()
#8 /vagrant/vendor/symfony/messenger/TraceableMessageBus.php(38): Symfony\Component\Messenger\MessageBus->dispatch()
#9 /vagrant/src/Messenger/MessageBus.php(37): Symfony\Component\Messenger\TraceableMessageBus->dispatch()
#10 /vagrant/vendor/symfony/mailer/Mailer.php(66): App\Messenger\MessageBus->dispatch()
#11 /vagrant/src/Mailer/Mailer.php(83): Symfony\Component\Mailer\Mailer->send()
#12 /vagrant/src/Mailer/Mailer.php(96): App\Mailer\Mailer->send()
#13 /vagrant/src/MessageHandler/Trading/StrategySubscriptionMessageHandler.php(118): App\Mailer\Mailer->sendEmail()
#14 /vagrant/src/MessageHandler/Trading/StrategySubscriptionMessageHandler.php(72): App\MessageHandler\Trading\StrategySubscriptionMessageHandler->handle()
#15 /vagrant/vendor/symfony/messenger/Middleware/HandleMessageMiddleware.php(152): App\MessageHandler\Trading\StrategySubscriptionMessageHandler->__invoke()
#16 /vagrant/vendor/symfony/messenger/Middleware/HandleMessageMiddleware.php(91): Symfony\Component\Messenger\Middleware\HandleMessageMiddleware->callHandler()
#17 /vagrant/vendor/symfony/messenger/Middleware/SendMessageMiddleware.php(71): Symfony\Component\Messenger\Middleware\HandleMessageMiddleware->handle()
#18 /vagrant/vendor/symfony/messenger/Middleware/FailedMessageProcessingMiddleware.php(34): Symfony\Component\Messenger\Middleware\SendMessageMiddleware->handle()
#19 /vagrant/vendor/symfony/messenger/Middleware/DispatchAfterCurrentBusMiddleware.php(68): Symfony\Component\Messenger\Middleware\FailedMessageProcessingMiddleware->handle()
#20 /vagrant/vendor/symfony/messenger/Middleware/RejectRedeliveredMessageMiddleware.php(41): Symfony\Component\Messenger\Middleware\DispatchAfterCurrentBusMiddleware->handle()
#21 /vagrant/vendor/symfony/messenger/Middleware/AddBusNameStampMiddleware.php(37): Symfony\Component\Messenger\Middleware\RejectRedeliveredMessageMiddleware->handle()
#22 /vagrant/vendor/symfony/messenger/Middleware/TraceableMiddleware.php(40): Symfony\Component\Messenger\Middleware\AddBusNameStampMiddleware->handle()
#23 /vagrant/vendor/symfony/messenger/MessageBus.php(70): Symfony\Component\Messenger\Middleware\TraceableMiddleware->handle()
#24 /vagrant/vendor/symfony/messenger/TraceableMessageBus.php(38): Symfony\Component\Messenger\MessageBus->dispatch()
#25 /vagrant/vendor/symfony/messenger/RoutableMessageBus.php(54): Symfony\Component\Messenger\TraceableMessageBus->dispatch()
#26 /vagrant/vendor/symfony/messenger/Worker.php(162): Symfony\Component\Messenger\RoutableMessageBus->dispatch()
#27 /vagrant/vendor/symfony/messenger/Worker.php(109): Symfony\Component\Messenger\Worker->handleMessage()
#28 /vagrant/vendor/symfony/messenger/Command/ConsumeMessagesCommand.php(238): Symfony\Component\Messenger\Worker->run()
#29 /vagrant/vendor/symfony/console/Command/Command.php(326): Symfony\Component\Messenger\Command\ConsumeMessagesCommand->execute()
#30 /vagrant/vendor/symfony/console/Application.php(1096): Symfony\Component\Console\Command\Command->run()
#31 /vagrant/vendor/symfony/framework-bundle/Console/Application.php(126): Symfony\Component\Console\Application->doRunCommand()
#32 /vagrant/vendor/symfony/console/Application.php(324): Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand()
#33 /vagrant/vendor/symfony/framework-bundle/Console/Application.php(80): Symfony\Component\Console\Application->doRun()
#34 /vagrant/vendor/symfony/console/Application.php(175): Symfony\Bundle\FrameworkBundle\Console\Application->doRun()
#35 /vagrant/vendor/symfony/runtime/Runner/Symfony/ConsoleApplicationRunner.php(49): Symfony\Component\Console\Application->run()
#36 /vagrant/vendor/autoload_runtime.php(29): Symfony\Component\Runtime\Runner\Symfony\ConsoleApplicationRunner->run()
#37 /vagrant/bin/console(11): require_once('...')
#38 {main}

TODO:

  • Add test for retry logic when publishing messages

@carsonbot
Copy link

Hey!

Thanks for your PR. You are targeting branch "7.1" but it seems your PR description refers to branch "6.4".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

@carsonbot carsonbot changed the title [Messenger][Amqp] Handle AMQPConnectionException when publishing a message. [Messenger] [Amqp] Handle AMQPConnectionException when publishing a message. Mar 5, 2024
@jwage jwage changed the base branch from 7.1 to 6.4 March 5, 2024 20:22
@jwage jwage force-pushed the amqp-connection-exception-retries branch from 0ea9ca5 to 1dfcd20 Compare March 5, 2024 20:24
if (++$retries <= $maxRetries) {
$this->clear();

goto retry;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cost years of public bashing, but I guess we can afford it 🥲

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💘 goto, no shame :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hehehe I like it too. I feel like imo it reads better than a loop specifically for retry logic.

@chalasr chalasr force-pushed the amqp-connection-exception-retries branch from 6479742 to f123370 Compare March 5, 2024 20:59
@chalasr
Copy link
Member

chalasr commented Mar 5, 2024

Thank you @jwage.

@chalasr chalasr merged commit e43b198 into symfony:6.4 Mar 5, 2024
8 of 9 checks passed
@jwage jwage deleted the amqp-connection-exception-retries branch March 5, 2024 21:04
@Nyholm
Copy link
Member

Nyholm commented Mar 6, 2024

Lovely. Thank you

@jwage
Copy link
Contributor Author

jwage commented Mar 20, 2024

@nicolas-grekas @chalasr @Nyholm thinking more about this. Does this retry open up the possibility for a double publish? What if the write succeeds on the server but the client loses the connection and exception is thrown before getting the response to the write? Then we retry and double publish. Since this isn't idempotent and we have no guarantees, I don't think we can generically retry.

@dunglas see the above. This is related to our discussion about not being able to safely retry non idempotent queries automatically in Doctrine.

Asked here as well php-amqp/php-amqp#548

This was referenced Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Messenger] Reconnect after connection was closed
5 participants