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 Declarative Definition of Exceptions That Should Not Be Immediately Retried #7094

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.

Some exceptions may be transient, but in a high throughput environment, immediate retries of specific types of exceptions may not be beneficial.

https://docs.particular.net/architecture/recoverability#transient-errors

immediate retries might not be the best approach when the root cause is due to overloaded or throttled resources, as they may exacerbate these problems

Describe the requested feature

Similar to the powerful declarative syntax for Unrecoverable Exceptions described in #7093, allow declaration of exception criteria that should not be immediately retried.

Ex:
recoverability.ImmediateRetriesNotNeededFor<SqlException>(ex => IsTimeout(ex));

Describe alternatives you've considered

If some component of the system is overloaded, then automatic rate limiting endpointConfiguration.Recoverability().OnConsecutiveFailures(...) can slow things down to let the component recover. Automatic rate limiting could be thought of as the chainsaw approach, where ImmediateRetriesNotNeededFor<T>() is like a scalpel. One is good for ice sculptures but the other may be more appropriate when the plastic surgeon approaches your reconstructive surgery.

If there are multiple handlers in a service, for example with a saga, consider if only one handler is accessing a limited resource that may be throwing a specific exception. OnConsecutiveFailures(...) would be slowing message processing of all handlers in the services, but ImmediateRetriesNotNeededFor<T>(...) would limit impact of the slow down.

Additional Context

Related to:




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
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