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

Error raised if someone goes offline and attempts a request #66

Closed
troym9731 opened this issue Apr 5, 2021 · 5 comments
Closed

Error raised if someone goes offline and attempts a request #66

troym9731 opened this issue Apr 5, 2021 · 5 comments

Comments

@troym9731
Copy link

Hi! I'm running into an issue where, if someone attempts a request while they are offline, result.headers on line 40 blows up because err.response is undefined. I'm unsure of the proper way to go about fixing this, or if there is something I should be doing with the axios instance before passing it to axios-fetch to avoid this issue altogether.

Any help would be greatly appreciated!

@mdlavin
Copy link
Member

mdlavin commented Apr 7, 2021

That's a case I haven't thought of before. This project is based around having axios-fetch behave just like fetch does, so my first question is "How does fetch behave in the offline case?".

If you know that, I have a feeling that the code here could be updated to match the fetch behavior if .response is missing

@troym9731
Copy link
Author

So it seems like it fails with a TypeError, and is one of the few cases that fetch returns a rejected promise.

Here is a post I found detailing the different error messages based on the browser: https://medium.com/vinh-rocks/how-to-handle-networkerror-when-using-fetch-ff2663220435

And here is the line of a test case in node-fetch for handling a network failure: https://github.com/node-fetch/node-fetch/blob/master/test/main.js#L110

So what would be your preferred path forward?

@mdlavin
Copy link
Member

mdlavin commented Apr 7, 2021

The simplest thing I can think of that might work is to change:

} catch (err) {
  result = err.response;
}

into

} catch (err) {
  // Non-HTTP failures should be re-thrown. This will handle lower level network failures
  if (!err.response) throw err;
  // HTTP level failures can be translated into fetch style responses
  result = err.response;
}

Is it possible for you to try a change like that and see if it fixes your problem?

@troym9731
Copy link
Author

That appears to work! Would you like me to open a PR with those changes, or would it be simpler for you to just commit the change yourself?

@jkdowdle
Copy link

Unless someone feels otherwise going to go ahead and close this issue. The proposed solution was implemented in #78 and released in @lifeomic/axios-fetch@2.0.2 #79

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

3 participants