Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Skip retries for known failures #4154

Closed
indirect opened this issue Dec 13, 2015 · 9 comments
Closed

Skip retries for known failures #4154

indirect opened this issue Dec 13, 2015 · 9 comments

Comments

@indirect
Copy link
Member

When making a network request, Net::HTTP::NotFound is never going to spontaneously start working on retry. Likewise TooManyRedirects, Forbidden, and probably others I'm not thinking of. We should not retry those responses.

homu added a commit that referenced this issue Dec 15, 2015
…retry-on, r=indirect

Add more exception classes not to retry on to the Fetcher

Addresses #4154
@RochesterinNYC
Copy link
Contributor

Any other exception classes to not retry on? There are a few Error types that seem applicable that aren't currently on the list, but they were added in relatively recent versions of the Ruby core lib so adding them to the list throws errors in the Travis builds for the earlier rubies that wouldn't have those Error classes in core lib yet.

@segiddins
Copy link
Member

@RochesterinNYC we can always do ERRORS << NewExceptionClass if defined?(NewExceptionClass)

@mattbrictson
Copy link
Contributor

Actually I ran into this today, I think due to the Rubygems move to Fastly.

For a period of time today gem downloads were sporadically failing with 404s. E.g.:

Not Found 404 (https://rubygems.global.ssl.fastly.net/gems/devise-3.5.3.gem)

Despite my retry setting, Bundler did not retry, due to the recently merged #4155.

I think this is evidence that "once a 404, always a 404" is not necessarily the case. Also, maybe Bundler should retry on 502 as well?

@indirect
Copy link
Member Author

indirect commented Jan 1, 2016

@mattbrictson if Fastly returns a 404 due to a system error, there is no reason to think that retrying will succeed, though. You'll have to wait until the system issue is resolved before it'll work, and mechanically trying over and over doesn't seem like a good way to check for that...

@mattbrictson
Copy link
Contributor

if Fastly returns a 404 due to a system error, there is no reason to think that retrying will succeed

@indirect Fair enough. But isn't that true of all HTTP errors other than perhaps a 503 with an accompanying Retry-After header?

I'm not intending to be pedantic; I am just curious to know what use case --retry is intended to solve. Is it mostly for network issues (socket timeout, connection dropped, etc.) and not for HTTP errors?

@segiddins
Copy link
Member

@mattbrictson I believe that is the main intention, yes.

@indirect
Copy link
Member Author

indirect commented Jan 3, 2016

@mattbrictson yes, it's primarily for network errors and server 500 errors that might be transient or limited to a single backend.

@mattbrictson
Copy link
Contributor

OK, makes sense. Thanks.

@indirect
Copy link
Member Author

indirect commented Jan 3, 2016

closing this as fixed by @RochesterinNYC's recent work

@indirect indirect closed this as completed Jan 3, 2016
homu added a commit that referenced this issue Jan 3, 2016
…giddins

Add `Net#HTTP--` errors safely (only if in version of ruby stdlib)

Continuation of #4154. Some errors in the ruby `Net` module that we might not want to retry on were added in later Ruby versions of the stdlib. This PR allows us to preserve backwards compatibility while adding new  errors.
homu added a commit that referenced this issue Jan 3, 2016
…giddins

Add `Net#HTTP--` errors safely (only if in version of ruby stdlib)

Continuation of #4154. Some errors in the ruby `Net` module that we might not want to retry on were added in later Ruby versions of the stdlib. This PR allows us to preserve backwards compatibility while adding new  errors.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants