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

[proposal] Don't retry if no error.code #379

Closed
reconbot opened this issue Sep 24, 2017 · 2 comments
Closed

[proposal] Don't retry if no error.code #379

reconbot opened this issue Sep 24, 2017 · 2 comments

Comments

@reconbot
Copy link
Contributor

reconbot commented Sep 24, 2017

I was thinking about my previous issue with testing #187 reading the outcome of #222 and I was looking at the source of is-retry-allowed and the aws-sdk and I realized that we retry on unknown errors, while AWS and possibly others, do not.

I'd like to propose to only retry if we have a known retryable error code.

(note: I considered moving this upstream to is-retry-allowed but I think it's a got concern first and foremost. It's weird we have a default true and a whitelist in that module.)

(related: nock/nock#898 I've been pushing for this for nock because it's really annoying when developing, but I'm starting to think got should solve this. I'm not 100% yet)

@sindresorhus
Copy link
Owner

I would be ok with that, as we already have a good whitelist and blacklist, so the error.code check would only apply to unknown errors.

@reconbot Can you think of any downside with doing this?

@floatdrop @alextes @lukechilds Thoughts?

@lukechilds
Copy link
Contributor

lukechilds commented Oct 3, 2017

I agree, retries should be whitelist only.

There may be multiple proprietary errors along the lines of API_LIMIT_REACHED which should obviously not be retried.

Edit: Oh wait, those are Node.js errors right? Not HTTP errors. So moot point about API limit responses, but still agree it should be whitelist only.

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