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

is Axios Network Error properly detected? #138

Open
dagi3d opened this issue Jun 24, 2020 · 5 comments
Open

is Axios Network Error properly detected? #138

dagi3d opened this issue Jun 24, 2020 · 5 comments

Comments

@dagi3d
Copy link

dagi3d commented Jun 24, 2020

Hi,
I was trying this library to make the app more resilient against possible failed requests because of network errors, and the thing is I am not sure if the default function to detect the network error will work with the current Axios implementation:

when Axios wants to raise the Network Error it sends a null response code:

 request.onerror = function handleError() {
      // Real errors are hidden from us by the browser
      // onerror should only fire if it's a network error
      reject(createError('Network Error', config, null, request));

but in the axios-retry, it will check if there is actually an error code:

export function isNetworkError(error) {
  return (
    !error.response &&
    Boolean(error.code) && // Prevents retrying cancelled requests
    error.code !== 'ECONNABORTED' && // Prevents retrying timed out requests
    isRetryAllowed(error)
  ); // Prevents retrying unsafe errors
}

maybe theres is something I am missing, but is this really working as expected?

@subvertallchris
Copy link

I think we're seeing the same thing. The error object that gets into isNetworkError looks like this when we log it out:

Error: Network Error
    at createError (createError.js:16)
    at XMLHttpRequest.handleError (xhr.js:91)

This object is of type AxiosError. I can call error.message and it returns Network Error. A simple workaround for us is to provide a custom check and fall back to the default:

function errorIsAxiosError(error: Error): error is AxiosError {
  return !!(error as any).isAxiosError;
}

function simpleNetworkErrorCheck(error: Error) {
  if (errorIsAxiosError(error) && error.message === 'Network Error') {
    return true;
  } else {
    return isNetworkOrIdempotentRequestError(error);
  }
}

axiosRetry(axios, { retryCondition: simpleNetworkErrorCheck });

I'm unsure of whether all browsers and platforms will give us that generic "Network Error" message, so I'll check more but it would be great if anyone tries this out and wants to report their findings.

@Uzlopak
Copy link

Uzlopak commented Oct 30, 2020

I think this is because of isRetryAllowed, where ENETUNREACH is in the denylist

https://github.com/sindresorhus/is-retry-allowed/blob/master/index.js

@cbrevik
Copy link

cbrevik commented Nov 4, 2020

Seems related to axios/axios#2351

I did something similar as above here, but instead check for:

const isNetworkError = (error: AxiosError) =>
  error && // just to make sure
  !error.response && // if there is a response, it reached the server and not a network error
  error.code !== 'ECONNABORTED'; // check that it isn't a timeout

And then my retryCondition is:

const retryCondition = (error: AxiosError) =>
  isNetworkError(error) || // my custom check
  isIdempotentRequestError(error); // retry lib check

It's not a perfect network error check, but I'd prefer that to checking for a string message. Ideally this lib should handle it better?

@meshuamam
Copy link

@Uzlopak
Copy link

Uzlopak commented Dec 1, 2022

Well. It got a AxiosError about 14 months ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants