That's a very interesting idea. So if I understand it correctly, it would allow users to configure things when they dispatch the messages to the bus. For example, in order to configure the serialization groups used when serializing the message or enabling the validation, this would look like this:

$bus->dispatch(new SerializerContextAwareMessage(
    new MessageToValidate(
        new MyMessage(/* ... */)
    ),
    ['groups' => [...]]
));

1. Serialization survival. I believe that such "feature" needs to survive serialization/deserialization. Right now, the "transport" works because we use the message class as a reference when de-serializing. How would you approach this in such scenario?

2. Implemented interface? It definitely exposes some "logic" when dispatching the message. If you dispatch it multiple times from multiple places, it means you need to "copy" this logic. Expect the ability to have different "rules" for the same message (which I don't believe is a good idea) what would be the advantage over a message implementing a certain interface? I guess "decoupling" is an answer but here, the "coupling" is still around IMHO.

1. [...] How would you approach this in such scenario?

I'd expect some opinions first before deciding if we want to support it or even if it's is a concept only acting before sending the message :) But we could find a solution for sure.

2. [...] Except the ability to have different "rules" for the same message (which I don't believe is a good idea) [...] I guess "decoupling" is an answer but here, the "coupling" is still around IMHO.

That's the point to me indeed. Don't mix infrastructural requirements with my messages. This is also the most flexible approach as implementing such interfaces would be cumbersome, inheriting multiple base classes won't be possible appart from using traits, and I want to keep my messages simple wrappers around the data they exist for, no more. Composing through these wrappers solves this and is only the responsibility of the dispatcher (I don't mind the infrastructural stuff knowing about my messages, so "coupling" in this way is not an issue). As you mentioned it, this also allows having "different rules" for the same message, according to the place it was dispatched. Can still be relevant for the metrics middleware example aforementioned for instance.

Alright, I definitely like the idea. The problem that we are trying to solve here will happen both ways (before/after sending/receiving the message), so we need this "configuration" to survive encoding/decoding or otherwise, it won't be a valid solution for lots of use cases.

Do you have an idea to tackle this serialization problem? Nesting the message within multiple wrappers sound like a source of troubles when doing the de-serialization. It also poses the problem of communication with 3rd party systems. Let's get the example of an application communicating with another one via RabbitMq. If they want to configure the retry mechanism (an example that would use such configuration) then the serialized message within the queue would be too different (the message is now within another one) for the one before and... it wouldn't work anymore.

An alternative to your idea is a more flat option.

$message = new ConfiguredMessage($message);
$message = $message->with(new RetryStrategyConfiguration(/* ... */));
$message = $message->with(new SerializationConfiguration(/* ... */));

$bus->dispatch($message);

1. The ConfiguredMessage would be final and non-serializable. It would contain the same sort of helper (the unwrap you propose) but could be called get(string $configurationFqcn) or similar
2. Same very nice mechanism that you propose on the additional interface on middlewares
3. The encoder/decoder would serialize all the "configurations" one by one in different headers (so we don't "break" the message)

WDYT? 🤔

That looks nice to me, I should give it a try :)

RetryStrategyConfiguration and cie could all implements a Configuration interface so we might be able to use the serializer's discriminator map. It would require more configuration and might not be very extensible.

I see some red flags when you have classes with feature interfaces (like our messages) and you try to wrap (decorate) them in other classes. We had the similar situation with php-cache/cache-bundle and we decided to create PHP classes on the fly to support the wrapped functionality and the feature interfaces. Not a super solid solution..

Im not sure I understand why this is needed. Or I mean, why we need this abstraction to solve the problems mentioned. We mention validation groups, but from a CQRS perspective, messages are for one purpose only. Ie you would never need to validate the same message in two different ways.

About the issue when you only want to retry some messages: See here what HTTPlug is doing. Sure, HTTP is simpler here. If you got something super complex, them maybe you should implement your own RetryPlugin. For the vast majority of the users I assume they want to retry all messages until they pass.

At the moment Im 👎 for this PR.

If one still believe that this is a really good feature, I see to problem implementing this in a third party library. The Messenger component is so awesome and flexible so one could easily create a nyholm/wrapped-messeges-bundle.

I see some red flags when you have classes with feature interfaces (like our messages) and you try to wrap (decorate) them in other classes.

@Nyholm can you elaborate on these ones? You actually did not mention any of these "red flags" 😉 Also, what do you mean by "classes with feature interfaces"? Last but not least, do you acknowledge that you saw that in the implementation, the middleware or handler won't receive the wrapped message, right? It's only if you implement a specific interface that you do so I don't see understand the issue.

About the issue when you only want to retry some messages: See here what HTTPlug is doing.

Right now, without such feature proposed here, we simply can't (or I don't see how at least 😃) have a generic retry feature, unfortunately. In order to have such a feature (broader than just the AmqpExt adapter), we need to pass some metadata down to the adapters in some way (an interface on the message or via wrapping it).

Of course. In PHP cache we have cache adapters for different types of storages. Some storage/adapter may support tagging, some support hierarchy cache. That is why we have "feature interfaces" like TaggingAdapter or HierarchyAdapter. So a consuming library my check for that interface before they try to use tags. This is fine and it works. I think this is similar to what been discussed with messages in the MessengerComponent. We are discussing DelayedMessageInterface etc.

When we later wanted to decorate our adapters with a TraceableAdapter to be used with Symfony's WebProfiler DataCollector we will run into issues because the decorated service will no longer implement the feature interface. I fear (the "red flags") that we move the Messenger component in the same direction. With wrapped messages nobody would see our DelayedMessageInterface.

I do recognize that FooCacheAdapter is a service and our messages are value objects.

Last but not least, do you acknowledge that you saw that in the implementation, the middleware or handler won't receive the wrapped message, right?

No, I did not see that. So it only the bus that handle the WrappedMessage?

Right now, without such feature proposed here, we simply can't (or I don't see how at least 😃) have a generic retry feature, unfortunately.

I think the logic of "how a message should be retired" should not belong to the message. The message itself should not care at all. It is the RetryMiddleware that should handle that. Of the top of my head, here is an example:

class RetryMiddleware {
    private $decider; // A service that decides if a message should be retried or not
    private $publisher; // A service that pushes the message to a new(?) bus or different(?) queue

    // ...

    public function handle($message, callable $next)
    {
        try {
            $result = $next($message);
        } catch (\Throwable $e) {
            if($this->decider->decide($message, $e)) {
                $this->publisher->publish($message, $e);
            }
            throw $e;
        }

        return $result;
    }
}

Last but not least, do you acknowledge that you saw that in the implementation, the middleware or handler won't receive the wrapped message, right?

No, I did not see that. So it only the bus that handle the WrappedMessage?

Sorry if the description was unclear/minimalistic. I did open this RFC through a PR in order to give examples through code rather than a big description so we can discuss all its aspects right now through (partial) implementation.

The idea is to make wrapper transparent in most cases when directly consuming a message like in middlewares. If a middleware needs to access the wrappers and not only the original message, it has to implement a specific marker interface, used by the bus to send either the message and its wrappers around, or just the message otherwise. That's what the WrappedMessageAwareMiddlewareInterface is about in the current version of this PR.

So, right now, I think the red flags you mentioned actually won't be an issue for middlewares.

the decorated service will no longer implement the feature interface. I fear (the "red flags") that we move the Messenger component in the same direction. With wrapped messages nobody would see our DelayedMessageInterface.

You can still implement feature interface on your messages. But that's precisely what I'd want to avoid actually, because I don't want my messages to be anything else than VO around the data they exist for. Starting implementing feature interfaces related to infrastructural concerns is not the responsibility of these objects, but rather on the callers's side.

Im not sure I understand why this is needed. Or I mean, why we need this abstraction to solve the problems mentioned. We mention validation groups, but from a CQRS perspective, messages are for one purpose only. Ie you would never need to validate the same message in two different ways.

We introduced ChainHandler allowing to return multiple results from a single dispatched message and MessageSubscriberInterface to have the ability to easily have a single handler handling multiple messages types. These are not very common to me neither, but still can be useful in some cases. I think we shouldn't assume too much there would never be a need for more flexibility here. As I said myself, the example with validation groups is uncommon but still useful in most advanced cases where the data inside the message is complex but still considered a single unit to handle.

If one still believe that this is a really good feature, I see to problem implementing this in a third party library. The Messenger component is so awesome and flexible so one could easily create a nyholm/wrapped-messeges-bundle.

Even if as I said it should be transparent for middlewares, this requires the bus to be aware of such a feature to choose to provide either the wrapped or unwrapped message to each middlewares in the stack. Third-parties middlewares decorators would also need to be aware of such a feature and implement WrappedMessageAwareMiddlewareInterface or it might be incompatible otherwise. So this is an important decision to take in the core itself to me or we might have some troubles implementing it later.

We should also consider that, currently, there is already a wrapper in the wild (ReceivedMessage) and it already requires a special treatment: #27066. Either the concept should be generalized, or either we might think about another way to handle it.

Thank you for such a detailed answer @ogizanagi. I think that this is a needed feature as well and I'm 👍 to add it (the flat version though, for the reasons I've mentioned).

Hey guys!

As I have very little Message Bus experience, take my opinion as a bit of an outsider/noob :). I don't really have a good impression as to if this is or isn't a good idea, or the possible pitfalls.

However, this note makes a lot of sense to me:

We should also consider that, currently, there is already a wrapper in the wild (ReceivedMessage) and it already requires a special treatment: #27066. Either the concept should be generalized, or either we might think about another way to handle it.

And, in general, even if it’s an edge case, the fact that there is currently no way to “attach” middleware configuration to a message (e.g. validation groups) seems not ideal, at the very least.

I'd also prefer the flat approach. But, more for the reason that messages inside of messages make my head spin a little bit. And, I think there's no disadvantage to the flat version?

the fact that there is currently no way to “attach” middleware configuration to a message (e.g. validation groups) seems not ideal

IMO this should stay like this, the configuration should depend of the bus and message class but the caller shouldn't be able to override this configuration. In the example of the validation group this would allow anyone to pass invalid messages by setting the validation group to one with no constraints.

I've pushed the new flat version. To answer @weaverryan, there isn't any disadvantage to it in comparison with the wrappers approach.
But, whereas it doesn't seem that bad for middlewares, the configured messages also have to go through other layers, like the transport, which feels like the implementation isn't much satisfying. i expected that, now we have some code under our eyes to consider. So let's see if this is going somewhere or not.

For the serialization of flatten configurations, I'd expect each configuration to be \Serializable. PHP serialization (under control) seemed like a good fit for this, as anyway you need to use the same stack on both sender & receiver sides to benefit from configured messages.

Rebase needed after #27129

One more thing: should every item go through transport? We could just ignore non-serializable items when encoding sending. Also ReceivedMessage just is a marker used on receiver's side so no point being serializable.

Yes, that's a fair point. So I hesitated proposing a EnvelopItemInterface::isTransportable(): bool method (or whatever). But simply mentioning this on the interface and the docs might be enough. Your call :)

I've opened PR in @ogizanagi's repository that solve this problem: https://github.com/ogizanagi/symfony/pull/1 , Mark transportable items and ensure that they can be properly serialized. You can cherry pick my commit if this kind of sollution is acceptable.

Thank you @Koc ! Unfortunately, I chose not doing so on purpose because I believe it has the same drawbacks expressed in #26945 (comment).
But as I said, perhaps mentioning this requirement on the docs and the interface might be enough.

You cann't forget implement serialize/deserialize when implementing TransportableEnvelopeItemInterface because got PHP error (TransportableEnvelopeItemInterface extends Serializable). And you shouldn't implement isTransportable all the time.

The point was you can forget implementing TransportableEnvelopeItemInterface with the proposed code (thus, the item is dropped without warning).
Which is the same as you can forget to implement \Serializable before c17ea71 (hence, the item is dropped without warning).

So that's not changing anything to what is expressed in #26945 (comment). Your suggestion is basically just using a dedicated Messenger interface rather than just \Serializable (which is not a bad idea, but not really required here).
EnvelopeItemInterface::isTransportable(): bool makes things clear by requiring to answer the transportable question.

The question is: is it either a DX or documentation issue to solve.

Let's go ahead with this one in 4.1 beta, it's definitely needed.

@sroze: c17ea71 removed from this PR for now. To be reconsidered for next beta, in another PR.

It took time, but here we go, this is in now. Thank you very much @ogizanagi.

Thanks everyone for the discussions & reviews, @sroze for the discussions on Slack and @andersonamuller who gave me a whole new point of view just by finding the proper naming for this 😄

Reverted after detecting an issue. Solution is probably found but that'll wait beta2 :)