[SPIKE] Spike raw dispatch pipeline

[danielmarbach]

Attention: Does not even compile. I didn't want to spend too much time on this

While brainstorming how we could get rid of the builder calls I looked how we could get rid of the builder calls for the dispatcher. This is an attempt to do so.

• Only one builder call in the core
• Dedicated dispatch pipeline which is allowed to access the builder
• Allows to intercept all outgoing messages even with message driven pub sub
  • Solves a lot of hacks downstreams need to do for Atts and message stamping. For example in ASQ we had to hack the serializer to be able to stamp all messages
  • Makes the dispatch part finally extendable
• FeatureStartupTasks no longer need to inject IDispatcher directly. They can just treat the message session as Raw. I went with an additional interface to minimize breaking changes and hide the raw capability from the eyes of the standard user.
• Satellites and Recoverability can also easily access the dispatch pipeline over the MessageContext or ErrorContext
• The timeout poller dispatcher access hasn't been spiked yet but since it is a core infrastructure I wouldn't worry too much

For me beside reducing the builder calls the fact that it allows properly extend the raw dispatches is a huge plus.

[danielmarbach]

In theory the extension method could also be scoped to FeatureStartupTask only like the following:

static IMessageSessionRaw Raw(this FeatureStartupTask task, IMessageSession session);

usage

var rawSession = this.Raw(session);
rawSession.Dispatch()

so that it can't be abused on endpoint level for example. But on the other-hand why not allow it?

[SzymonPobiega]

I really really like it. Calling the dispatcher directly was one of the most painful things I experience when trying to extend V6.

[timbussmann]

looks like a promising approach to me. I'm not yet entirely convinced how we would expose the "raw" API, as it seems like this shouldn't be on the same level as pipeline-based operations (at least not inside message handlers). Might be good enough to put the extension methods into a dedicated namespace though.

[SzymonPobiega]

@danielmarbach can I also have a switch for disabling the main pipeline? ;-) Then I would be able to drop support for NSB.Raw

[danielmarbach]

So do I get the blessing to do this for real from @Particular/nservicebus-maintainers ?

[timbussmann]

@danielmarbach listed it as a topic for the next weekly sync.

[timbussmann]

we discussed this on the weekly sync and we agreed that the listed changes are definitely desirable improvements as long as we prevent this API being available on the messagehandlercontext.

There are certainly a few things to discuss further, e.g. naming, but in general we think this is worth doing 👍 Ideas around the NSB.Raw functionality are something we definitely see as something outside the scope of this PR.

[danielmarbach]

Cool. Let me know how we should proceed with this.

[danielmarbach]

@Particular/nservicebus-maintainers so can I go ahead with this? Since this enables us to remove hacky code in ASQ acceptance tests where we need to wrap the serializer and add a pipeline step in order to be able to stamp all outgoing messages

[andreasohlund]

@danielmarbach will this involve breaking changes or can this be shipped in a minor version?

[danielmarbach]

@andreasohlund I tried to do it without breaking changes. Still I implemented IExtendable on error context and switch pipelineterminators to stages. Not sure about the implications of that.

[timbussmann]

that shouldn't be a blocker for a minor then?

so can I go ahead with this?

as 7.1 would be the earliest version to bring that in, feel free to decide when to start polishing this in order to avoid too much effort in keeping your branch up-to-date. Maybe hold back till RC phase?

[danielmarbach]

Had a good discussion with @andreasohlund today and we reiterated on the original decision decisions around message driven pubsub, timeouts, recoverability, and satellites. We concluded the following:

• Satellites should not go through a pipeline at all (we want to stay with this)
• Extending the timeout manager can be dangerous and we'd lose potentially the guarantees we are trying to fulfill with the current design
• MessageDrivenPubsub is about to die sooner than later
• Recoverability should instead of doing raw dispatches hand things over to the transport and let it make an explicit decision
• If suddenly everything goes through the dispatch pipeline the users behavior might be invoked with stuff they never anticipated for. Suttle issue in the user dispatch behavior could influence timeout manager etc drastically which is not a risk we want to take

From ASQ perspective there is a workaround. We can register a hook in the settings of type Func<IDispatchMessages, IDispatchMessages> which allows ATTs to decorate the dispatch calls and stamp each and every message. Closing this.

[SzymonPobiega]

Where is the handling of Func<IDispatchMessages, IDispatchMessages> ?

[danielmarbach]

@SzymonPobiega we have to hack it into the transport for now as a "magic hidden settings key"

[danielmarbach]

@SzymonPobiega are you saying you'd have like the messagesession.Raw() part of this PR?

[SzymonPobiega]

@danielmarbach well, when in FST you usually need to send raw messages, not regular ones. In the current version you need to pass in a dispatcher as the session you get is pretty much useless. That's an inconvenience bu not a huge problem.

[ramonsmits]

Yes! This would be so awesome for some scenarios!