diff --git a/lib/faraday/request/retry.rb b/lib/faraday/request/retry.rb index 0459b67a..08bc8376 100644 --- a/lib/faraday/request/retry.rb +++ b/lib/faraday/request/retry.rb @@ -19,7 +19,12 @@ module Faraday # interval that is random between 0.1 and 0.15 # class Request::Retry < Faraday::Middleware - class Options < Faraday::Options.new(:max, :interval, :interval_randomness, :backoff_factor, :exceptions) + + IDEMPOTENT_METHODS = [:delete, :get, :head, :options, :put] + + class Options < Faraday::Options.new(:max, :interval, :interval_randomness, :backoff_factor, :exceptions, :retry_if) + DEFAULT_CHECK = lambda { |env,exception| false } + def self.from(value) if Fixnum === value new(value) @@ -49,6 +54,10 @@ module Faraday Error::TimeoutError]) end + def retry_if + self[:retry_if] ||= DEFAULT_CHECK + end + end # Public: Initialize middleware @@ -66,6 +75,12 @@ module Faraday # given as Class, Module, or String. (default: # [Errno::ETIMEDOUT, Timeout::Error, # Error::TimeoutError]) + # retry_if - block that will receive the env object and the exception raised + # and should decide if the code should retry still the action or + # not independent of the retry count. This would be useful + # if the exception produced is non-recoverable or if the + # the HTTP method called is not idempotent. + # (defaults to return false) def initialize(app, options = nil) super(app) @options = Options.from(options) @@ -85,8 +100,8 @@ module Faraday begin env[:body] = request_body # after failure env[:body] is set to the response body @app.call(env) - rescue @errmatch - if retries > 0 + rescue @errmatch => exception + if retries > 0 && retry_request?(env, exception) retries -= 1 sleep sleep_amount(retries + 1) retry @@ -114,5 +129,12 @@ module Faraday end matcher end + + private + + def retry_request?(env, exception) + IDEMPOTENT_METHODS.include?(env[:method]) || @options.retry_if.call(env, exception) + end + end end diff --git a/test/middleware/retry_test.rb b/test/middleware/retry_test.rb index 7c5a1193..845b4465 100644 --- a/test/middleware/retry_test.rb +++ b/test/middleware/retry_test.rb @@ -10,35 +10,37 @@ module Middleware Faraday.new do |b| b.request :retry, *retry_args b.adapter :test do |stub| - stub.post('/unstable') { - @times_called += 1 - @explode.call @times_called - } + ['get', 'post'].each do |method| + stub.send(method, '/unstable') { + @times_called += 1 + @explode.call @times_called + } + end end end end def test_unhandled_error @explode = lambda {|n| raise "boom!" } - assert_raises(RuntimeError) { conn.post("/unstable") } + assert_raises(RuntimeError) { conn.get("/unstable") } assert_equal 1, @times_called end def test_handled_error @explode = lambda {|n| raise Errno::ETIMEDOUT } - assert_raises(Errno::ETIMEDOUT) { conn.post("/unstable") } + assert_raises(Errno::ETIMEDOUT) { conn.get("/unstable") } assert_equal 3, @times_called end def test_legacy_max_retries @explode = lambda {|n| raise Errno::ETIMEDOUT } - assert_raises(Errno::ETIMEDOUT) { conn(1).post("/unstable") } + assert_raises(Errno::ETIMEDOUT) { conn(1).get("/unstable") } assert_equal 2, @times_called end def test_new_max_retries @explode = lambda {|n| raise Errno::ETIMEDOUT } - assert_raises(Errno::ETIMEDOUT) { conn(:max => 3).post("/unstable") } + assert_raises(Errno::ETIMEDOUT) { conn(:max => 3).get("/unstable") } assert_equal 4, @times_called end @@ -46,7 +48,7 @@ module Middleware @explode = lambda {|n| raise Errno::ETIMEDOUT } started = Time.now assert_raises(Errno::ETIMEDOUT) { - conn(:max => 2, :interval => 0.1).post("/unstable") + conn(:max => 2, :interval => 0.1).get("/unstable") } assert_in_delta 0.2, Time.now - started, 0.04 end @@ -70,7 +72,7 @@ module Middleware retry_middleware.sleep_amount_retries = [2, 1] assert_raises(Errno::ETIMEDOUT) { - retry_middleware.call({}) + retry_middleware.call({:method => :get}) } assert_empty retry_middleware.sleep_amount_retries @@ -101,9 +103,45 @@ module Middleware def test_custom_exceptions @explode = lambda {|n| raise "boom!" } assert_raises(RuntimeError) { - conn(:exceptions => StandardError).post("/unstable") + conn(:exceptions => StandardError).get("/unstable") } assert_equal 3, @times_called end + + def test_should_stop_retrying_if_block_returns_false_checking_env + @explode = lambda {|n| raise Errno::ETIMEDOUT } + check = lambda { |env,exception| env[:method] != :post } + assert_raises(Errno::ETIMEDOUT) { + conn(:retry_if => check).post("/unstable") + } + assert_equal 1, @times_called + end + + def test_should_stop_retrying_if_block_returns_false_checking_exception + @explode = lambda {|n| raise Errno::ETIMEDOUT } + check = lambda { |env,exception| !exception.kind_of?(Errno::ETIMEDOUT) } + assert_raises(Errno::ETIMEDOUT) { + conn(:retry_if => check).post("/unstable") + } + assert_equal 1, @times_called + end + + def test_should_not_call_retry_if_for_idempotent_methods + @explode = lambda {|n| raise Errno::ETIMEDOUT } + check = lambda { |env,exception| raise "this should have never been called" } + assert_raises(Errno::ETIMEDOUT) { + conn(:retry_if => check).get("/unstable") + } + assert_equal 3, @times_called + end + + def test_should_not_retry_for_non_idempotent_method + @explode = lambda {|n| raise Errno::ETIMEDOUT } + assert_raises(Errno::ETIMEDOUT) { + conn.post("/unstable") + } + assert_equal 1, @times_called + end + end end