-
Notifications
You must be signed in to change notification settings - Fork 538
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
Easier granular control of retry interceptor without losing built-in retry-after
handling
#3728
Comments
retry-after
handling
The suggestion of the callback is exactly what we wanted to avoid to give more ergonomics to provide inversion of control and hint the handler when to execute the further retry. For the handling of the |
@metcoder95 what's |
Of returning
|
My proposal is not touching the existing An extra This separates whether we should do a retry and actually do a retry into 2 functions. |
But yes, simply exposing the built-in async retry (err, { state, opts }, cb) {
// bail out error early with cb(err)
// ...
return cb(err);
return RetryHandler.RetryAfter(err, { state, opts }, cb);
} |
I'm in sync with providing a The |
IMHO, if we're going to do that, we should return the retry timeout directly as undici also implements exponential backoff in the default Otherwise, when a custom |
That was somehow the point, provide out-of-box implementation that its a good default, and leave more customized implementations up to the implementer. One thing that I've been thinking about, is to maybe provider other retry strategies out-of-the-box; e.g. linea retry, circuit breaker maybe, and more down the line. The |
This Would Solve...
I want more granular and flexible control over retry behavior. For example, instead of retrying only with specific error codes, I want to bail out of retries with specific error codes. Similarly, instead of retrying with specific http status codes, I want to bail out with specific http status code ranges (e.g.,
statusCode > 400 && statusCode < 599 && statusCode !== 401 && statusCode !== 403 && statusCode !== 404 && statusCode !== 405
), or bail out of retries with a specific header.Currently, I can accomplish this with the
retry
callback in the retry option, but if I provide aretry
callback, I will lose undici's built-inretry-after
header handling (which is inRetryHandler[kRetryHandlerDefaultRetry]
):undici/lib/handler/retry-handler.js
Line 41 in 1bc83ea
undici/lib/handler/retry-handler.js
Line 107 in 1bc83ea
I want to have both custom retry bailouts and retain undici's built-in
retry-after
handling.The Implementation Should Look Like...
A
shouldRetry
callback in the retry option should receivecontext
(which containsstate
andopt
),header
,method
,statusCode
, anderrorCode
. The callback should return a boolean or a number to determine whether the retry should continue and how long it should wait before the next retry.I Have Also Considered...
Exposing
RetryHandler[kRetryHandlerDefaultRetry]
asRetryHandler['onRetryAfter']
, which can be then called inside the customretry
callback.The text was updated successfully, but these errors were encountered: