diff --git a/lib/faraday/error.rb b/lib/faraday/error.rb index dde5f6c0d..23fa6dc76 100644 --- a/lib/faraday/error.rb +++ b/lib/faraday/error.rb @@ -55,8 +55,12 @@ def initialize(ex = nil) class SSLError < ClientError end + class RetriableResponse < ClientError; end + [:ClientError, :ConnectionFailed, :ResourceNotFound, - :ParsingError, :TimeoutError, :SSLError].each do |const| + :ParsingError, :TimeoutError, :SSLError, :RetriableResponse].each do |const| Error.const_set(const, Faraday.const_get(const)) end + + end diff --git a/lib/faraday/request/retry.rb b/lib/faraday/request/retry.rb index 5e5a5742f..8ff07169d 100644 --- a/lib/faraday/request/retry.rb +++ b/lib/faraday/request/retry.rb @@ -23,7 +23,9 @@ class Request::Retry < Faraday::Middleware IDEMPOTENT_METHODS = [:delete, :get, :head, :options, :put] class Options < Faraday::Options.new(:max, :interval, :max_interval, :interval_randomness, - :backoff_factor, :exceptions, :methods, :retry_if, :retry_block) + :backoff_factor, :exceptions, :methods, :retry_if, :retry_block, + :retry_statuses) + DEFAULT_CHECK = lambda { |env,exception| false } def self.from(value) @@ -56,7 +58,8 @@ def backoff_factor def exceptions Array(self[:exceptions] ||= [Errno::ETIMEDOUT, 'Timeout::Error', - Error::TimeoutError]) + Error::TimeoutError, + Faraday::Error::RetriableResponse]) end def methods @@ -71,6 +74,9 @@ def retry_block self[:retry_block] ||= Proc.new {} end + def retry_statuses + Array(self[:retry_statuses] ||= []) + end end # Public: Initialize middleware @@ -106,12 +112,13 @@ def initialize(app, options = nil) @errmatch = build_exception_matcher(@options.exceptions) end - def sleep_amount(retries) - retry_index = @options.max - retries - current_interval = @options.interval * (@options.backoff_factor ** retry_index) - current_interval = [current_interval, @options.max_interval].min - random_interval = rand * @options.interval_randomness.to_f * @options.interval - current_interval + random_interval + def calculate_sleep_amount(retries, env) + retry_after = calculate_retry_after(env) + retry_interval = calculate_retry_interval(retries) + + return if retry_after && retry_after > @options.max_interval + + retry_after && retry_after >= retry_interval ? retry_after : retry_interval end def call(env) @@ -119,16 +126,21 @@ def call(env) request_body = env[:body] begin env[:body] = request_body # after failure env[:body] is set to the response body - @app.call(env) + @app.call(env).tap do |resp| + raise Faraday::Error::RetriableResponse.new(nil, resp) if @options.retry_statuses.include?(resp.status) + end rescue @errmatch => exception if retries > 0 && retry_request?(env, exception) retries -= 1 rewind_files(request_body) @options.retry_block.call(env, @options, retries, exception) - sleep sleep_amount(retries + 1) - retry + if (sleep_amount = calculate_sleep_amount(retries + 1, env)) + sleep sleep_amount + retry + end end - raise + + raise unless exception.is_a?(Faraday::Error::RetriableResponse) end end @@ -167,5 +179,29 @@ def rewind_files(body) end end + # MDN spec for Retry-After header: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After + def calculate_retry_after(env) + response_headers = env[:response_headers] + return unless response_headers + + retry_after_value = env[:response_headers]["Retry-After"] + + # Try to parse date from the header value + begin + datetime = DateTime.rfc2822(retry_after_value) + datetime.to_time - Time.now.utc + rescue ArgumentError + retry_after_value.to_f + end + end + + def calculate_retry_interval(retries) + retry_index = @options.max - retries + current_interval = @options.interval * (@options.backoff_factor ** retry_index) + current_interval = [current_interval, @options.max_interval].min + random_interval = rand * @options.interval_randomness.to_f * @options.interval + + current_interval + random_interval + end end end diff --git a/test/middleware/retry_test.rb b/test/middleware/retry_test.rb index 4a259d159..07b1dd445 100644 --- a/test/middleware/retry_test.rb +++ b/test/middleware/retry_test.rb @@ -10,6 +10,7 @@ def setup def conn(*retry_args) Faraday.new do |b| b.request :retry, *retry_args + b.adapter :test do |stub| ['get', 'post'].each do |method| stub.send(method, '/unstable') do |env| @@ -18,6 +19,22 @@ def conn(*retry_args) env[:body] = nil # simulate blanking out response body @explode.call @times_called end + + stub.send(method, '/throttled') do |env| + @times_called += 1 + @envs << env.dup + + params = env[:params] + + status = (params['status'] || 429).to_i + headers = {} + + retry_after = params['retry_after'] + + headers['Retry-After'] = retry_after if retry_after + + [status, headers, ''] + end end end end @@ -68,7 +85,7 @@ def test_interval assert_in_delta 0.2, Time.now - started, 0.04 end - def test_calls_sleep_amount + def test_calls_calculate_sleep_amount explode_app = MiniTest::Mock.new explode_app.expect(:call, nil, [{:body=>nil}]) def explode_app.call(env) @@ -79,7 +96,7 @@ def explode_app.call(env) class << retry_middleware attr_accessor :sleep_amount_retries - def sleep_amount(retries) + def calculate_sleep_amount(retries, env) self.sleep_amount_retries.delete(retries) 0 end @@ -95,30 +112,30 @@ def sleep_amount(retries) def test_exponential_backoff middleware = Faraday::Request::Retry.new(nil, :max => 5, :interval => 0.1, :backoff_factor => 2) - assert_equal middleware.sleep_amount(5), 0.1 - assert_equal middleware.sleep_amount(4), 0.2 - assert_equal middleware.sleep_amount(3), 0.4 + assert_equal middleware.send(:calculate_retry_interval, 5), 0.1 + assert_equal middleware.send(:calculate_retry_interval, 4), 0.2 + assert_equal middleware.send(:calculate_retry_interval, 3), 0.4 end def test_exponential_backoff_with_max_interval middleware = Faraday::Request::Retry.new(nil, :max => 5, :interval => 1, :max_interval => 3, :backoff_factor => 2) - assert_equal middleware.sleep_amount(5), 1 - assert_equal middleware.sleep_amount(4), 2 - assert_equal middleware.sleep_amount(3), 3 - assert_equal middleware.sleep_amount(2), 3 + assert_equal middleware.send(:calculate_retry_interval, 5), 1 + assert_equal middleware.send(:calculate_retry_interval, 4), 2 + assert_equal middleware.send(:calculate_retry_interval, 3), 3 + assert_equal middleware.send(:calculate_retry_interval, 2), 3 end def test_random_additional_interval_amount middleware = Faraday::Request::Retry.new(nil, :max => 2, :interval => 0.1, :interval_randomness => 1.0) - sleep_amount = middleware.sleep_amount(2) + sleep_amount = middleware.send(:calculate_retry_interval, 2) assert_operator sleep_amount, :>=, 0.1 assert_operator sleep_amount, :<=, 0.2 middleware = Faraday::Request::Retry.new(nil, :max => 2, :interval => 0.1, :interval_randomness => 0.5) - sleep_amount = middleware.sleep_amount(2) + sleep_amount = middleware.send(:calculate_retry_interval, 2) assert_operator sleep_amount, :>=, 0.1 assert_operator sleep_amount, :<=, 0.15 middleware = Faraday::Request::Retry.new(nil, :max => 2, :interval => 0.1, :interval_randomness => 0.25) - sleep_amount = middleware.sleep_amount(2) + sleep_amount = middleware.send(:calculate_retry_interval, 2) assert_operator sleep_amount, :>=, 0.1 assert_operator sleep_amount, :<=, 0.125 end @@ -212,5 +229,53 @@ def test_should_rewind_files_on_retry assert_equal 3, @times_called assert_equal 2, rewound end + + def test_should_retry_retriable_response + params = { status: 429 } + conn(:max => 1, :retry_statuses => 429).get("/throttled", params) + + assert_equal 2, @times_called + end + + def test_should_not_retry_non_retriable_response + params = { status: 503 } + conn(:max => 1, :retry_statuses => 429).get("/throttled", params) + + assert_equal 1, @times_called + end + + def test_interval_if_retry_after_present + started = Time.now + + params = { :retry_after => 0.5 } + conn(:max => 1, :interval => 0.1, :retry_statuses => [429]).get("/throttled", params) + + assert Time.now - started > 0.5 + end + + def test_should_ignore_retry_after_if_less_then_calculated + started = Time.now + + params = { :retry_after => 0.1 } + conn(:max => 1, :interval => 0.2, :retry_statuses => [429]).get("/throttled", params) + + assert Time.now - started > 0.2 + end + + def test_interval_when_retry_after_is_timestamp + started = Time.now + + params = { :retry_after => (Time.now.utc + 2).strftime('%a, %d %b %Y %H:%M:%S GMT') } + conn(:max => 1, :interval => 0.1, :retry_statuses => [429]).get("/throttled", params) + + assert Time.now - started > 1 + end + + def test_should_not_retry_when_retry_after_greater_then_max_interval + params = { :retry_after => (Time.now.utc + 20).strftime('%a, %d %b %Y %H:%M:%S GMT') } + conn(:max => 2, :interval => 0.1, :retry_statuses => [429], max_interval: 5).get("/throttled", params) + + assert_equal 1, @times_called + end end end