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

remove retry logic #100

Closed
wants to merge 1 commit into from
Closed

remove retry logic #100

wants to merge 1 commit into from

Conversation

grosser
Copy link
Contributor

@grosser grosser commented May 2, 2019

fixes #37
fixes #89

@tenderlove @timanovsky @drbrain

not sure if this breaks RFC 2616 but idk what part of the readme it was referring to when it mentioned the RFC, I'd assume the closing the connection which is still there

@grosser
Copy link
Contributor Author

grosser commented May 7, 2019

@drbrain does this look good ? ... if ye I'll start fixing the tests

@grosser
Copy link
Contributor Author

grosser commented May 8, 2019

@drbrain looks like the right direction ?

@grosser
Copy link
Contributor Author

grosser commented May 13, 2019

@drbrain looks like the right direction ? (I still need to change all the tests if yes)

@grosser
Copy link
Contributor Author

grosser commented May 16, 2019

added tests

@tenderlove are you able to merge since @drbrain seems to ignore this PR ? :(

@grosser
Copy link
Contributor Author

grosser commented May 16, 2019

releasing this as net-http-persistent-retry so I can use it in my other projects and can declare an actual dependency, will yank if this ever gets merged 🤷‍♂

grosser added a commit to grosser/faraday that referenced this pull request May 16, 2019
…implified

(instead of `Faraday::ConnectionFailed`)

In v3 net-http-persistent rescue a lot of exceptions and retries them or re-raises them as Net::HTTP::Persistent::Error which is complicated and unnecessary.

When drbrain/net-http-persistent#100 gets merged (or when using [net-http-persistent-retry](https://github.com/grosser/net-http-persistent) which is a fork of net-http-persistent with that PR applied) the error wrapping/retry goes away and it will start raising "normal" errors like `Net::OpenTimeout` which is a `Timeout::Error`.

/cc @drbrain @mislav @iMacTia
@grosser grosser mentioned this pull request May 25, 2019
32 tasks
drbrain pushed a commit that referenced this pull request Apr 30, 2020
drbrain added a commit that referenced this pull request Apr 30, 2020
@drbrain
Copy link
Owner

drbrain commented Apr 30, 2020

Merged, see log above.

@drbrain drbrain closed this Apr 30, 2020
@grosser
Copy link
Contributor Author

grosser commented Apr 30, 2020

awesome, thx! :D

xuxiangyang added a commit to xuxiangyang/thrift_rack that referenced this pull request May 11, 2020
stim371 added a commit to stim371/faraday that referenced this pull request Oct 1, 2020
The net-http-persistent gem removed the #retry_change_requests adapter configuration with drbrain/net-http-persistent#100 in v4.0.0.
flavorjones added a commit to flavorjones/mechanize that referenced this pull request Jan 23, 2021
Net::HTTP::Persistent removed `#retry_change_requests` in v4.0.0 and
later, so these tests fail if we've pulled in a more recent version
than that.

Related to drbrain/net-http-persistent#100
flavorjones added a commit to sparklemotion/mechanize that referenced this pull request Jan 23, 2021
Net::HTTP::Persistent removed `#retry_change_requests` in v4.0.0 and
later, so these tests fail if we've pulled in a more recent version
than that.

Related to drbrain/net-http-persistent#100
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

Successfully merging this pull request may close these issues.

Net::HTTP::Persistent::Error: too many connection resets Retry of idempotent requests not working?
2 participants