Particular / NServiceBus

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

Allow Unrecoverable Exceptions to Be Defined with More Complex Criteria Than Type Matching Only #7093

Open bbrandt opened 2 weeks ago

bbrandt commented 2 weeks ago

Describe the feature.

Is your feature related to a problem? Please describe.

It is not always possible to determine if an exception should trigger a retry or send a message to the error queue based on exception type. One example is EF Core's DbUpdateException or SqlException, which is sometimes transient and sometimes not. When we need to alter recoverability behavior based on these common exception types we are forced to write custom recoverability policies, a place that bugs sometimes like to hide.

Describe the requested feature

Provide a declarative way to define more complex criteria around whether unrecoverable exceptions.

Ex (Once SqlClient implements IsTransient like MySql and Postgres have):
recoverability.AddUnrecoverableException<SqlException>(ex => !ex.IsTransient);
Ex:

https://github.com/dotnet/efcore/blob/main/src/EFCore.SqlServer/Storage/Internal/SqlServerTransientExceptionDetector.cs

recoverability.AddUnrecoverableException<DbUpdateException>(ex => !copyOfSqlServerTransientExceptionDetector.ShouldRetryOn(ex));

Describe alternatives you've considered

Defining our own recoverability policies - Partial customization

Additional Context




From: Andreas Ohlund 
Sent: Wednesday, April 3, 2024 1:42 AM
To: Ben Brandt 
Subject: Re: Recoverability policies
 
I get you now, unfortunately, we only have a more declarative way of skipping recoverability entirely via the "unrecoverable exception" feature

https://docs.particular.net/nservicebus/recoverability/?version=core_8.1#unrecoverable-exceptions

In theory, you could build something similar yourself by defining exception types that you then look for in the recoverability policy and then have your devs throw when appropriate.

//in code
if(ex.Message.Contains("bla"))
    throw new ImmediateRetriesNotNeededException()


//policy
if(ex is ImmediateRetriesNotNeededException)
   //skip retries

Not sure if I like this myself tho since it bleeds into exception names which will get logged etc etc


Would you be able to raise an issue on the https://github.com/Particular/NServiceBus repo with your feature request?

Cheers,

Andreas



On Tue, Apr 2, 2024 at 6:06 PM Ben Brandt  wrote:
Thanks, Andreas.  Yes.  That is similar to what we are doing right now.  What I was thinking more about is if we have a certain SQL Exception type that says the DB is overloaded, we want to skip immediate retries and only do delay retries.  Is there a more fluent way to define this behavior based on properties of the exception instead of dropping into the imperitive way we define a custom recoverability policy.

There is nothing inherently wrong with how custom recoverability policies work in NSB, it is just that they can have bugs, doing things like infinte immediate retry that destroy our system performance.  For some reason we have trouble catching these issues before they bring down production.  If there was a way to configure this behavior, for "skip immediate retry for exceptions matching this Func<Exception, bool>" and still use the default recoverability policy that'd be wonderful.


From: Andreas Ohlund 
Sent: Tuesday, April 2, 2024 5:30 AM
To: Ben Brandt 
Subject: Re: Recoverability policies
 
Hi Ben!

You do have access to the full exception so you should be able to write:


    if (context.Exception.Message.Contains("SQL")  )
    {
        return RecoverabilityAction.Discard("Blah.");
    }

Would that work?

/Andreas

On Thu, Mar 28, 2024 at 10:23 PM Ben Brandt wrote:
Thanks, Andreas!

What's the best way to describe more advanced exception filtering like SqlExceptions that have a SqlErrors[...].Number of a certain value?

Is there anything like this MassTransit feature where a filter function can be specified for to further refine the exception filter?

                r.Ignore<DataException>(x => x.Message.Contains("SQL"));
Exceptions · MassTransit


Best Regards,

Ben Brandt

Architect

 




From: Andreas Ohlund 
Sent: Thursday, March 28, 2024 2:22 AM
To: Ben Brandt 
Subject: Recoverability policies
 
  |   |   -- | -- | --
I looked at the recoverability policy support we have:

https://docs.particular.net/nservicebus/recoverability/custom-recoverability-policy?version=core_8.1

and you should be able to filter on the exception type and the message type. Note that the message type would be a string in the headers called `NServiceBus.EnclosedMessageTypes`

https://docs.particular.net/nservicebus/messaging/headers?version=core_8.1#serialization-headers-nservicebus-enclosedmessagetypes

Let me know if that does the trick for you.

Cheers,

Andreas