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

Net::HTTP adapter retries requests automatically #774

Closed
iMacTia opened this issue Feb 21, 2018 · 3 comments · Fixed by #799
Closed

Net::HTTP adapter retries requests automatically #774

iMacTia opened this issue Feb 21, 2018 · 3 comments · Fixed by #799

Comments

@iMacTia
Copy link
Member

iMacTia commented Feb 21, 2018

Basic Info

  • Faraday Version: any
  • Ruby Version: 2.5+

Issue description

When using Net::HTTP as adapter, users get surprised by the retry strategy provided by Net::HTTP. This brings to different "issues" (e.g. #612 and #771).
There's not much we can do for Ruby <= 2.4, however Ruby 2.5 provides a new max_retries config for Net::HTTP so Faraday should set that automatically when available.

Steps to reproduce

See issues #612 and #771.

mattheworiordan added a commit to ably/ably-ruby that referenced this issue Apr 27, 2018
…P requests twice

See lostisland/faraday#774 and ankane/the-ultimate-guide-to-ruby-timeouts#8.

Unbelievable that it does this unconditionally.  Saying that, including a new HTTP library seems overkill.
@michielboekhoff
Copy link
Contributor

I've been looking at this issue. One approach mentioned in #771 is to set the default to 0, but change it depending on the amount of retries configured. We would have to make the Retry middleware aware of the underlying adapter (as some adapters might not support this), and conditionally apply this logic in the env. The adapter would also have to know about the amount of retries.

As an alternative to said approach, we can set it to do no retries, instead keeping the retry logic confined to the Retry middleware. Whatever the outcome, I'd be more than happy to pick this up @iMacTia .

@iMacTia
Copy link
Member Author

iMacTia commented May 17, 2018

I was definitely thinking at the second option.
We already have the Retry middleware that can take care of retrying failed requests.
Adapters (in this case Net::HTTP) should never retry requests on their own.

All we need to do in this case is to tell Net::HTTP adapter to set max_retries to 0, if this property is available, of course

@michielboekhoff
Copy link
Contributor

@iMacTia - I couldn't agree more. In that case, I shall pick this up.

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

Successfully merging a pull request may close this issue.

2 participants