ms-rest-js icon indicating copy to clipboard operation
ms-rest-js copied to clipboard

Allow customization of status codes to retry in exponentialRetryPolicy

Open mikechamberlain opened this issue 6 years ago • 1 comments

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 avatar Mar 15 '19 04:03 mikechamberlain

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

jeremymeng avatar Jan 27 '21 00:01 jeremymeng