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][RFC] Allow handler to choose the transport #30634

Closed
weaverryan opened this issue Mar 21, 2019 · 7 comments
Closed

[Messenger][RFC] Allow handler to choose the transport #30634

weaverryan opened this issue Mar 21, 2019 · 7 comments
Labels
Messenger RFC RFC = Request For Comments (proposals about features that you want to be discussed)

Comments

@weaverryan
Copy link
Member

weaverryan commented Mar 21, 2019

Hi!

Currently, messages are "routes" to the transport by their class/type in config:

framework:
    messenger:
        transports:
            async: '...'
            async2: '...'

        routing:
            # Route your messages to the transports
            'App\Message\CoolNotification': async
            'App\Message\SmsNotification': [async, async2]

This has a few downsides:

  1. If a message has 2 handlers, there is no way to send one handler to one transport (e.g. async) and another handle to another (e.g. sync).

  2. There is no way for a third-party to put a message into a bus and specify which transport it should go to. For example, the mailer might want a way to route all of its handled messages to a mailer transport that the user can then configure (there are a few missing pieces for this, but that's for a separate issue).

Proposal

Allow each handler to (optionally) specify what transport they should be sent to. This would be done via an interface + autowiring or via a tag:

class MyMessageHandler implements TransportAwareHandlerInterface
{
    public function __invoke(MyMessage $myMessage)
    {
    }

    public function getTransport()
    {
        return 'async';
    }
}

Or via the tag:

services:
    App\MessageHandler\MyMessageHandler:
        tags:
            - { name: messenger.message_handler, transport: async }

If the async transport doesn't exist, an exception would be thrown.

This would become the main way that we recommend routing your messages, which means that we would recommend that the class->transport framework.messenger.routing not be used, unless you want to send a message to a queue that will be handled outside of your app (and so, there is no sender).

This would play nicely with messages dispatched by 3rd party bundles. For example, supposes the mailer dispatches a SendMailMessage, which is handled by its SendMailMessageHandler. And the handler says it should go to a mail transport (in another issue, we still need to allow 3rd party bundle to add transports & allow the user to map them, but ignore that for now). The user could then configure how it wants the mail transport to work (e.g. async) AND attach a second handler to SendMailMessage called LogSentMailMessagesHandler, which they decide to handle on a sync transport.

So, the question is: do we want to continue to route on a message-by-message basis? Or do we want to allow users to route on a handler-by-handler basis?

@javiereguiluz javiereguiluz added RFC RFC = Request For Comments (proposals about features that you want to be discussed) Messenger labels Mar 21, 2019
@weaverryan
Copy link
Member Author

tl;dr

Suppose someone dispatched a UserRegisteredMessage and they need 5 handlers for this: 2 sync (logging, some DB tweaks) and 3 async (welcome email, etc). How can we support that? Or is there a better pattern that user should use?

Allowing configuration of each handler is probably a rabbit hole. Would we then allow each handler to have its own retry config? Priority? That’s easy stuff to do via Stamps on a message. But configuring per-handler? That would be complex.

@f2r
Copy link
Contributor

f2r commented Mar 24, 2019

We could create a "handlers" section beside transports/routing to specify which transport we want to associate some handlers

@Koc
Copy link
Contributor

Koc commented Mar 24, 2019

What about changing routing configuration to something like this:

    routing:
        App\Messages\Commands\SendEmail:
            -   transport: mail
                handler: App\Handlers\Command\SendEmailHandler
                # no transport defined, so this handler executes synchonously
            -   handler: App\Handlers\Command\LogSentMailMessagesHandler
        App\Messages\Commands\ImportCategories:
            -   transport: amqp
                handler: App\Handlers\Command\ImportCategoriesHandler
        App\Messages\Commands\ActualizePrice:
                # no handler defined, so this message just sends to external subsystem and other application handled it
            -   transport: kafka

? This allows define all message to transport/bus/handler associations in one place. For now message to transport defined in config but handler to bus defined in service tag

@weaverryan
Copy link
Member Author

I'd prefer the tag-based config (or interface-based, which then just adds the tag). The existing tag already supports a bunch of things, including which bus to bind your handler to. Adding transport config to that seems like a logical next step.

A few complicating questions/issues

  1. Suppose a message has 5 handlers and the class is set in routing to go to the async transport. But, 2 of its handlers are set to go to amqp. Which should be sent to async then? Just the 3 remaining? Or all 5? You might immediately think "only 3!". But what if async is actually an external transport that won't be handled by Symfony. Wouldn't you want all 5 messages sent there, even though you'd like to additionally handle 2 of them by a handler in your app? It's tricky :).

  2. ALL ways to configure how a message is transported, like delays, etc - and future things like routing key, are done via envelopes on the message. Thus, there would be no way (I mean, it's possible, but we're paying for it in complexity) to start adding envelopes for some handlers, but not other handlers.

@weaverryan
Copy link
Member Author

I'm going to close this for now. We can revisit later - it's certainly not something that's in the scope for me to work on for 4.3.

@ruudk
Copy link
Contributor

ruudk commented Aug 25, 2019

Suppose someone dispatched a UserRegisteredMessage and they need 5 handlers for this: 2 sync (logging, some DB tweaks) and 3 async (welcome email, etc). How can we support that? Or is there a better pattern that user should use?

I come from SimpleBus where we only have 2 "transports", sync and async and this works exactly like that. Handlers define how they want to invoke the message, sync or async. They do this by either registering to the sync command bus or to the async command bus. This is done with tags. This works really well.

We have 2 very large applications that heavily use SimpleBus with its async capabilities. For a developer, it's very easy to add another async handler, by just adding a file in a directory. With the magic of auto-wire, we automatically tag handlers in the Asynchronous directory to be async.

SimpleBus knows about all the handlers, and will only write to the async transport if there is somebody listening to it.

Let's say there are 2 sync and 2 async handlers for a given message. If a developer would remove 1 handler.php file, it would still work. No configuration required. If the developer then removes the last async handler.php, it would still work with the 2 sync handlers. SimpleBus simply doesn't send to the async transport anymore.

I'm in the progress of migrating our SimpleBus project to Messenger, but I'm stuck on this point. I need to build a routing table of all my messages (commands + events) and see if there are async handlers or subscribers. I then need to check if there are sync handlers. Then I need to keep this list up to date. When a developer removes a handler.php later on, they also need to update this routing map.

To me, it doesn't make sense that the message is the key in the routing. I feel that the handers should be the one who are in charge of saying "give me messages of type X". Whenever Messenger handles a message of type X, it knows who's interested.

@weaverryan Can we maybe re-open this issue?

@diimpp
Copy link

diimpp commented Dec 2, 2020

I've come across a need to specify which transport would be used on per message basis, e.g. in some cases I want a message to be in sync and in some async.

Do I understand correctly, that this no possible unless I create copy of the message for sync and async and route them respectively?

Would it make sense to add such ability with Stamp/Middleware in the main package, if it's even possible?

Just found out that it's already proposed #38200

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Messenger RFC RFC = Request For Comments (proposals about features that you want to be discussed)
Projects
None yet
Development

No branches or pull requests

6 participants