Add option to disable in-memory retries of throttling while keeping other retries

[MatDumoulin]

Is your feature request related to a problem? Please describe the problem.

Working on an heavy background processing service, we have to make a lot of requests to the Graph Api. The default RetryHandler provided with the SDK will Task.Delay for the duration of the "retry-after" header leading to our task pipeline being idle for a significant amount of time.

Wanting to improve the throughput of our system, we decided to implement a custom mechanism to handle throttling which allows us to process other tenants while considering the "retry-after" delay suggested by the Graph Api for this tenant. Since we have an alternative, we would like to opt-out of the default retries from the SDK.

With the current settings exposed by the Graph SDK, the only option for removing the default throttling retries is to set the "MaxRetry" to 0. This has as a side effect to also remove the retries for 504s (which are not documented as being throttling).

Describe the solution you'd like.

Have a way to disable the default retries on throttling while keeping other default retries in the SDK.

I tried to do it with the "ShouldRetry" option, but, similar to this other case, the short circuit operator in place for throttling is being invoked before that method, leading to the ShouldRetry method being ignored for throttling.

I tried inheritance on the RetryHandler as a way to overwrite the short circuit operator itself but the method is private. For it to work, one would have to overwrite the SendAsync and SendRetryAsync methods but it comes at the cost of opting out of code maintained by the SDK/Kiota.

Additional context?

No response

[andrueastman]

Thanks for raising this @MatDumoulin

Out of curiosity, are you handling the custom retry mechanism by using another alternative retry handler?

I believe changing the short circuit behavior may be behaviorally breaking for some scenarios but one thing that could be done is to make the method protected virtual to allow for a similar customization for your scenario. Thoughts?

[MatDumoulin]

Thank you for the quick answer @andrueastman

Out of curiosity, are you handling the custom retry mechanism by using another alternative retry handler?

We are not. Instead, we handle the exception with responseStatusCode=429 outside of the SDK and switching to another tenant when such exception happens.

I believe changing the short circuit behavior may be behaviorally breaking for some scenarios but one thing that could be done is to make the method protected virtual to allow for a similar customization for your scenario. Thoughts?

Making the method protected virtual would indeed allow me to override the behaviour as I would like to, without introducing a breaking change. That would be a sufficient change.

If I were to do this investigation again, I would have also appreciated if the RetryHandlerOptions documentation mentioned that the SDK bypasses the ShouldRetry option in case of 429, 503 and 504. Documenting this behaviour feels to me like a quick win to help others to use the SDK.

[MatDumoulin]

Hi @andrueastman,

Following up on this thread to see if the protected virtual proposition is the one that will be implemented. Thank you!