Express messages with transport that doesn't support express fails to startup endpoint

[ramonsmits]

I tried to send an express message using different transports and got the following exception using the SQL Transport

System.AggregateException: One or more errors occurred. ---> System.Exception: The configured transport does not support non-durable messages but some messages have been configured to be non-durable (e.g. by using the [Express] attribute). Make the messages durable, or use a transport supporting non-durable messages.
   at NServiceBus.Features.MessageDurabilityFeature.Convention(Type messageType, Boolean doesSupportNonDurableDelivery, Boolean defaultToDurableMessages, Func`2 durabilityConvention) 

It makes more sense to write a WARN log entry if we detect express messages but the transport does not support non-durable messaging. An exception would make sense if it was the other way around so trying to send a durable message via a non-durable transport as that could result in potential unwanted message loss.

This is happening with V6 RC and not in V5 making this a breaking change.

Workaround

@andreasohlund mentioned a potential workaround by setting a convention by doing the following during initialization:

 configuration.Convertions.ExpressMessages(t=>false)

[timbussmann]

In general I think a warning would be good enough, but then we document some specific behavior for express messages which we wouldn't follow when using express messages on transport which don't support them.

I don't think this can be a real user issue.

[ramonsmits]

If that is the case then why do we don't get a similar exception when having Express messages but MSMQ transport isn't doing non-transaction queue access as that is the only way to actually get a performance benefit for express messages on MSMQ.

[andreasohlund]

That sounds like a bug to me?

[timbussmann]

btw. docs states:

The SQL Server transport has no support for this setting and it is ignored.

and similar for transports which do not support non-durable delivery.

[ramonsmits]

@timbussmann What do you mean with your comment? The doc states it is ignored and the exception clearly indicates its not ignored?

[timbussmann]

just wanted to mention that the docs diverge from the current v6 behavior so we need to fix docs or fix the behavior.

[timbussmann]

I still think it adds no real value to support express messages on non-transactional transports. Users specify a certain expected behavior which they won't get (e.g. performance improvement which won't be there, "purged" messages which won't be purged, ...) so I'd say let's keep the exception, adjust the docs and close this issue?

[danielmarbach]

I think we agree that for non-transactional transports the expected behavior like @timbussmann outlined is just not there. I wonder though whether throwing is a good option. I tend to agree with @ramonsmits that a WARN statement would be enough. Since there is no real problem if we would let users specify it.

[timbussmann]

I don't see any new arguments coming up on this discussion. As long as we don't have a case of the current behavior blocking a single user (or even causing pain) I don't see any priority in this.

[ramonsmits]

@timbussmann

we don't have a case of the current behavior blocking a single user (or even causing pain)

We have a user - me - that was hindered by this hence me creating this issue as I think that exception is pretty annoying and not needed.

I have a much more concrete case for you: Customers storing messages types + conventions are stored in a separate assembly. Each endpoint can have their own conventions. That is how I have done this in the past so that maintainers of different services that originally didn't communicate at all eventually exchanged events.

Also, I don't look at the express attribute from the angle of performance but from durability and might that also be the title of the doco "Non-Durable Messaging". From a design perspective I would apply this attribute on messages that do not need to be durable from a business perspective and NOT to make it perform better.

We should look at this from the perspective how it should behave, and them make sure all transports align accordingly which currently isn't the case.

Either we document this breaking change or we convert the exception into a WARN. The last has my preference.

[andreasohlund]

We should look at this from the perspective how it should behave, and them make sure all transports align accordingly which currently isn't the case.

The trend I see is that it's having the core provide api's for "cross transport" features like Durable messages/TTBR etc is a loosing battle and leads to leaky abstractions and pressure on transports to try to support things that they shouldn't be.

I don't think this is urgent but an alternative path here could be to:

1. Deprecate the core Express attribute and convention
2. Reintroduce the [Express] attribute and convention in a MSMQ package (when the transport is split out from the core this could live in there). We can then also tighten up the validation that eg. the queue needs to be created as non-transactional for "Express" to have any effect at all.
3. Have the transports that truly support non durable message add native API's to allow for this. This would mean that we could use lingo that aligns better. Eg RabbitMQ would use "Persistent messages" and not Express. Sql would not support this at all etc.

[timbussmann]

Customers storing messages types + conventions are stored in a separate assembly. Each endpoint can have their own conventions. That is how I have done this in the past so that maintainers of different services that originally didn't communicate at all eventually exchanged events.

so different services may consider the same message type as an express message and others don't? That sounds incredible dangerous. I think this would even be a good point for throwing an exception to make sure every service is aware this is an express message?

I agree with @andreasohlund , it seems to be rather something transport specific. But that's also a different story. So +1 to move express messages out of the core and to close this issue. Considering that daniel and brandon alread gave their thumbs, @andreasohlund do you want to raise an issue for your proposal and close this issue?

[ramonsmits]

@timbussmann I'm fine with having transport specific and moving this attribute and convention away from core.

[andreasohlund]

New issue raised to track the actionable items https://github.com/Particular/NServiceBus/issues/4252