Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
bbrandt opened this issue Jul 2, 2024 · 0 comments

Comments

@bbrandt
Copy link

bbrandt commented Jul 2, 2024


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


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"));


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:


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`


Let me know if that does the trick for you.

Cheers,

Andreas
@bbrandt bbrandt changed the title Allow More Unrecoverable Exceptions to Be Defined with More Complex Criteria Than Type Matching Only Allow Unrecoverable Exceptions to Be Defined with More Complex Criteria Than Type Matching Only Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants