softonic / axios-retry

Axios plugin that intercepts failed requests and retries them whenever possible
Other
1.9k stars 167 forks source link

isNetworkOrIdempotentRequestError should return boolean #190

Closed Romick2005 closed 2 years ago

Romick2005 commented 2 years ago
/**
 * @param  {Error}  error
 * @return {boolean | Promise}
 */
export function isNetworkOrIdempotentRequestError(error) {
  return isNetworkError(error) || isIdempotentRequestError(error);
}

Where isNetworkError(error: Error): boolean and isNetworkOrIdempotentRequestError(error: Error): boolean. So I assume that isNetworkOrIdempotentRequestError should return only boolean and not a promise as here: https://github.com/softonic/axios-retry/blob/00439f4a9bde3cfe5f9c6f69b22af2b941d7c38e/es/index.mjs#L60 but declared good: isNetworkOrIdempotentRequestError(error: Error): boolean;

This means that retryCondition should be a function that return a boolean value not a promise as indicated by the variable name

async function shouldRetry(retries, retryCondition, currentState, error) {
  const shouldRetryOrPromise = currentState.retryCount < retries && retryCondition(error);

https://github.com/softonic/axios-retry/blob/00439f4a9bde3cfe5f9c6f69b22af2b941d7c38e/es/index.mjs#L213

https://github.com/softonic/axios-retry/blob/00439f4a9bde3cfe5f9c6f69b22af2b941d7c38e/es/index.mjs#L130

that also mean that we do not need this:

  // This could be a promise
  if (typeof shouldRetryOrPromise === 'object') {
    try {
      await shouldRetryOrPromise;
      return true;
    } catch (_err) {
      return false;
    }
  }

or I miss smth?

PeterChen1997 commented 2 years ago

You have missing that user can pass a promise to the retryCondition. The default function is no need for promise, that's right