Skip to content

[Messenger] Add doc for default routing for messages #16720

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

Merged
merged 1 commit into from
Oct 5, 2022

Conversation

clemherreman
Copy link
Contributor

@clemherreman clemherreman commented Apr 15, 2022

Hello,

I recently got bit on production after deploying a code change. We created a new FooMessage & FooHandler, but didn't update the messenger.yaml to include routing information.

This caused FooMessage to be directly handled by the HTTP process that dispatched the message, instead of the CLI, consumer process of Messenger. This lead to various issues: missing env var in HTTP context, lack of monitoring/stats as we only looks for messages metrics in the consumer container, not the api one :)

Looking at the code, I discovered you may define a default transport, but couldn't find it in the doc.

Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

👍🏽
I have not verified if this is a feature or not. But assuming you are correct; this feature definitely needs documentation.

@artyuum
Copy link
Contributor

artyuum commented Jul 12, 2022

I've been doing this in few of our projects and it works well. I think I heard about this on the Symfony slack and not in the documentation, so it's a nice addition.

Copy link
Contributor

@HeahDude HeahDude left a comment

Choose a reason for hiding this comment

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

Many thanks for that PR!

@@ -261,7 +261,13 @@ you can configure them to be sent to a transport:

Thanks to this, the ``App\Message\SmsNotification`` will be sent to the ``async``
transport and its handler(s) will *not* be called immediately. Any messages not
matched under ``routing`` will still be handled immediately.
matched under ``routing`` will still be handled immediately, i.e. synchronously.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
matched under ``routing`` will still be handled immediately, i.e. synchronously.
matched under ``routing`` will still be handled immediately (e.g. synchronously).

@clemherreman
Copy link
Contributor Author

Before merging that PR, I'd like to add something.

Using * as a default transport means all "system" messages, sent by the framework itself, will use that transport. That include Symfony\Component\Mailer\Messenger\SendEmailMessage, as the Mailer component detects that the Messenger component is there, and uses it to send email instead of sending it directly.

This caused issues on my project, because SendEmailMessage was not always serializable, if you add an attachment to your email using a PHP resource/stream.

I am not sure how to convey that complex information on the Symfony doc though. Maybe someone with more experience on writing could improve that?

@javiereguiluz
Copy link
Member

Thanks Clement! I reworded your last comment as follows while merging: javiereguiluz@5781a94

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.

8 participants