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 customization of status codes to retry in exponentialRetryPolicy #333

Open
mikechamberlain opened this issue Mar 15, 2019 · 1 comment

Comments

@mikechamberlain
Copy link

mikechamberlain commented Mar 15, 2019

Describe the problem

In exponentialRetryPolicy, the status codes to retry are hard coded in the shouldRetry functionas follows:

function shouldRetry(policy: ExponentialRetryPolicy, statusCode: number | undefined, retryData: RetryData): boolean {
  if (statusCode == undefined || (statusCode < 500 && statusCode !== 408) || statusCode === 501 || statusCode === 505) {
    return false;
  }
  ...

While this is a good default, it's still very specific and cannot be assumed to be the desired behavior in all cases. For instance, when there is no internet, statusCode would be undefined, but this is hardcoded to not retry, In our case we would like to override this behavior to retry when there is no internet. In the general case, this behavior should be configurable.

The solution I'd like

The ability to specify which status codes should result in a retry. Something like:

export function exponentialRetryPolicy(
   retryCount?: number, 
   retryInterval?: number, 
   minRetryInterval?: number, 
   maxRetryInterval?: number,
   shouldRetryOnStatus?: (statusCode: number) => bool) : RequestPolicyFactory {
...
}

(Though clearly the number of arguments to this function is getting out of control, so a better solution might be desirable.)

The alternatives I've considered

To work around this, we copy-pasted the entire retry policy into our user code to change this one small piece.

@mikechamberlain mikechamberlain changed the title Allow custom shouldRetry logic in exponentialRetryPolicy Allow customization of status codes to retry in exponentialRetryPolicy Mar 15, 2019
@jeremymeng
Copy link
Contributor

Maybe we could add an option to allow users to override our logic, something like (response, error) => boolean?

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