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

Allow specifying methods that bypass :retry_if on Request::Retry #437

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 10 additions & 2 deletions lib/faraday/request/retry.rb
Expand Up @@ -22,7 +22,8 @@ class Request::Retry < Faraday::Middleware

IDEMPOTENT_METHODS = [:delete, :get, :head, :options, :put]

class Options < Faraday::Options.new(:max, :interval, :interval_randomness, :backoff_factor, :exceptions, :retry_if)
class Options < Faraday::Options.new(:max, :interval, :interval_randomness, :backoff_factor,
:exceptions, :methods, :retry_if)
DEFAULT_CHECK = lambda { |env,exception| false }

def self.from(value)
Expand Down Expand Up @@ -54,6 +55,10 @@ def exceptions
Error::TimeoutError])
end

def methods
Array(self[:methods] ||= IDEMPOTENT_METHODS)
end

def retry_if
self[:retry_if] ||= DEFAULT_CHECK
end
Expand All @@ -75,6 +80,9 @@ def retry_if
# given as Class, Module, or String. (default:
# [Errno::ETIMEDOUT, Timeout::Error,
# Error::TimeoutError])
# methods - A list of HTTP methods to retry without calling retry_if. Pass
# an empty Array to call retry_if for all exceptions.
# (defaults to the idempotent HTTP methods in IDEMPOTENT_METHODS)
# 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
Expand Down Expand Up @@ -133,7 +141,7 @@ def build_exception_matcher(exceptions)
private

def retry_request?(env, exception)
IDEMPOTENT_METHODS.include?(env[:method]) || @options.retry_if.call(env, exception)
@options.methods.include?(env[:method]) || @options.retry_if.call(env, exception)
end

end
Expand Down
22 changes: 20 additions & 2 deletions test/middleware/retry_test.rb
Expand Up @@ -126,7 +126,7 @@ def test_should_stop_retrying_if_block_returns_false_checking_exception
assert_equal 1, @times_called
end

def test_should_not_call_retry_if_for_idempotent_methods
def test_should_not_call_retry_if_for_idempotent_methods_if_methods_unspecified
@explode = lambda {|n| raise Errno::ETIMEDOUT }
check = lambda { |env,exception| raise "this should have never been called" }
assert_raises(Errno::ETIMEDOUT) {
Expand All @@ -135,13 +135,31 @@ def test_should_not_call_retry_if_for_idempotent_methods
assert_equal 3, @times_called
end

def test_should_not_retry_for_non_idempotent_method
def test_should_not_retry_for_non_idempotent_method_if_methods_unspecified
@explode = lambda {|n| raise Errno::ETIMEDOUT }
assert_raises(Errno::ETIMEDOUT) {
conn.post("/unstable")
}
assert_equal 1, @times_called
end

def test_should_not_call_retry_if_for_specified_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, :methods => [:post]).post("/unstable")
}
assert_equal 3, @times_called
end

def test_should_call_retry_if_for_empty_method_list
@explode = lambda {|n| raise Errno::ETIMEDOUT }
check = lambda { |env,exception| @times_called < 2 }
assert_raises(Errno::ETIMEDOUT) {
conn(:retry_if => check, :methods => []).get("/unstable")
}
assert_equal 2, @times_called
end

end
end