search

Particular / NServiceBus

Build, version, and monitor better microservices with the most powerful service platform for .NET
https://particular.net/nservicebus/
Other
2.09k stars 648 forks source link

We don't properly expose conflicting configuration options to users #6491

Open ramonsmits opened 7 years ago

ramonsmits commented 7 years ago

If you specify immediate and delayed retry configuration but set transaction mode to None you get the following log entries.

WARN|NServiceBus.Recoverability|Delayed Retries will be disabled. Delayed retries are not supported when running with TransportTransactionMode.None. Failed messages will be moved to the error queue instead. WARN|NServiceBus.Recoverability|Immediate Retries will be disabled. Immediate Retries are not supported when running with TransportTransactionMode.None. Failed messages will be moved to the error queue instead.

Something similar was with performance counters. You could call the API to enable it, but if you try to enable performance counters on a send-only endpoint this is just ignored.

However, in the recent version of the extracted performance counters package these is now a startup check that prevents the endpoint from starting if it detects that the endpoint is a send-only endpoint.

ERROR|UnhandledException|Metrics are not supported on send only endpoints.Metrics are not supported on send only endpoints.

The fact that immediate and delayed retry configuration is ignored to me seems a bit more worrisome then throwing an exception because performance counters do not make sense for send-only endpoints.

This behavior is strange as the least dangourous configuration conflict is processed as being very severe.

I think something similar is with property encryption. It makes no sense in enabling if we detect there are no messages at all with encrypted properties and then we just ignore.

How should features behave? My personal opinion is that we should always throw exceptions when we detect conflicts even if some can be safely ignored. To me it should never be ignored as it could indicate a deployment configuration error like missing messages and or handler assemblies but based on a lot of current behavior we are very open in accepting configuration conflicts but raise ERROR or WARN log entries.

// cc @Scooletz

mauroservienti commented 7 years ago

I would expect the first to throw and the second to warn, not the other way round.

mauroservienti commented 7 years ago

@ramonsmits @Scooletz in the FLR/SLR + Transaction Mode set to None what happens if in a saga I set a timeout?

dvdstelt commented 7 years ago

For book-keeping, I'm referencing the public issue in Metrics repo : https://github.com/Particular/NServiceBus.Metrics/issues/74