-
-
Notifications
You must be signed in to change notification settings - Fork 67
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
Ignore window.fetch
network error
#38
Ignore window.fetch
network error
#38
Conversation
readme.md
Outdated
@@ -38,7 +38,7 @@ const run = async () => { | |||
|
|||
Returns a `Promise` that is fulfilled when calling `input` returns a fulfilled promise. If calling `input` returns a rejected promise, `input` is called again until the maximum number of retries is reached. It then rejects with the last rejection reason. | |||
|
|||
It doesn't retry on `TypeError` as that's a user error. | |||
It doesn't retry on `TypeError` as that's a user error. The only exclusion is fetchApi's 'Failed to fetch' error which can be a network error. See [the docs](https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch), and [this issue](https://github.com/whatwg/fetch/issues/526) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this sentence could be written better.
You also need to update index.d.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you help me out with the wording? it seems fine for me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May I suggest the following:
"It doesn't retry on TypeError
as that's a user error. The only exclusion to this logic is when TypeError
is thrown by fetch
's API with the message 'Failed to fetch', which indicates that a request was not successful due to a network error. However, beware that fetch
may throw TypeError
with different error messages on different platforms for similar situations. See whatwg/fetch#526 (comment)."
Better? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping @sindresorhus @oreporan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whatever @sindresorhus suggests as the text...Let me know
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What @acostalima wrote looks good to me.
.vscode/settings.json
Outdated
@@ -0,0 +1 @@ | |||
{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.vscode
should not be checked in.
window.fetch
network error
Fixes #37