Skip to content

Commit

Permalink
Don't call retry_block if retry won't happen due to max_interval and …
Browse files Browse the repository at this point in the history
…retry_after (#1350)
  • Loading branch information
jrochkind committed Dec 23, 2021
1 parent a555580 commit b1165ea
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 2 deletions.
3 changes: 2 additions & 1 deletion UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ We did our best to make this transition as painless as possible for you, so here
* If you're relying on `Faraday.default_adapter` (e.g. if you use `Faraday.get` or other verb class methods, or not
specifying an adapter in your connection initializer), then you'll now need to set it yourself. It previously
defaulted to `:net_http`, but it now defaults to `:test`. You can do so simply by using the setter:

```ruby
# For example, to use net_http (previous default value, will now require `gem 'faraday-net_http'` in your gemfile)
Faraday.default_adapter = :net_http
Expand Down Expand Up @@ -86,6 +86,7 @@ For more details, see https://github.com/lostisland/faraday/pull/1306
* Remove `Faraday::Response::Middleware`. You can now use the new `on_complete` callback provided by `Faraday::Middleware`.
* Drop `Faraday::UploadIO` in favour of `Faraday::FilePart`.
* `Faraday.default_connection_options` will now be deep-merged into new connections to avoid overriding them (e.g. headers).
* Retry middleware `retry_block` is not called if retry will not happen due to `max_interval`. (#1350)

## Faraday 1.0

Expand Down
2 changes: 1 addition & 1 deletion lib/faraday/request/retry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ def call(env)
if retries.positive? && retry_request?(env, e)
retries -= 1
rewind_files(request_body)
@options.retry_block.call(env, @options, retries, e)
if (sleep_amount = calculate_sleep_amount(retries + 1, env))
@options.retry_block.call(env, @options, retries, e)
sleep sleep_amount
retry
end
Expand Down
28 changes: 28 additions & 0 deletions spec/faraday/request/retry_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,16 @@

it { expect(Time.now - @started).to be_within(0.04).of(0.2) }
end

context 'and retry_block is set' do
let(:options) { [{ retry_block: ->(env, options, retries, exc) { retry_block_calls << [env, options, retries, exc] } }] }
let(:retry_block_calls) { [] }
let(:retry_block_times_called) { retry_block_calls.size }

it 'calls retry block for each retry' do
expect(retry_block_times_called).to eq(2)
end
end
end

context 'when no exception raised' do
Expand Down Expand Up @@ -249,6 +259,24 @@
let(:options) { [{ max: 2, interval: 0.1, max_interval: 5, retry_statuses: 504 }] }

it { expect(times_called).to eq(1) }

context 'and retry_block is set' do
let(:options) do
[{
retry_block: ->(env, options, retries, exc) { retry_block_calls << [env, options, retries, exc] },
max: 2,
max_interval: 5,
retry_statuses: 504
}]
end

let(:retry_block_calls) { [] }
let(:retry_block_times_called) { retry_block_calls.size }

it 'retry_block is not called' do
expect(retry_block_times_called).to eq(0)
end
end
end
end
end

0 comments on commit b1165ea

Please sign in to comment.