Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Only retry exceptions ruby 2 does not handle

Ruby 2 retries requests for some exceptions automatically per RFC 2616.

net-http-persistent must not retry requests that were already retried.

This change rescues exceptions retried by Net::HTTP and does not retry
those exceptions to keep net-http-persistent compliant with RFC 2616.

Related to #34
  • Loading branch information...
commit dc202b22798b5a24259c6e4b55bb32db5cd567ee 1 parent d760af7
@drbrain authored
Showing with 128 additions and 43 deletions.
  1. +34 −6 lib/net/http/persistent.rb
  2. +94 −37 test/test_net_http_persistent.rb
View
40 lib/net/http/persistent.rb
@@ -188,6 +188,21 @@ class Net::HTTP::Persistent
VERSION = '2.8.1'
##
+ # Exceptions rescued for automatic retry on ruby 2.0.0. This overlaps with
+ # the exception list for ruby 1.x.
+
+ RETRIED_EXCEPTIONS = [ # :nodoc:
+ (Net::ReadTimeout if Net.const_defined? :ReadTimeout),
+ IOError,
+ EOFError,
+ Errno::ECONNRESET,
+ Errno::ECONNABORTED,
+ Errno::EPIPE,
+ OpenSSL::SSL::SSLError,
+ Timeout::Error,
+ ].compact
+
+ ##
# Error class for errors raised by Net::HTTP::Persistent. Various
# SystemCallErrors are re-raised with a human-readable message under this
# class.
@@ -476,6 +491,9 @@ def initialize name = nil, proxy = nil
@retry_change_requests = false
+ @ruby_1 = RUBY_VERSION < '2'
+ @retried_on_ruby_2 = !@ruby_1
+
self.proxy = proxy if proxy
end
@@ -688,10 +706,15 @@ def idempotent? req
end
##
- # Is the request idempotent or is retry_change_requests allowed
+ # Is the request +req+ idempotent or is retry_change_requests allowed.
+ #
+ # If +retried_on_ruby_2+ is true, true will be returned if we are on ruby,
+ # retry_change_requests is allowed and the request is not idempotent.
- def can_retry? req
- @retry_change_requests or idempotent?(req)
+ def can_retry? req, retried_on_ruby_2 = false
+ return @retry_change_requests && !idempotent?(req) if retried_on_ruby_2
+
+ @retry_change_requests || idempotent?(req)
end
if RUBY_VERSION > '1.9' then
@@ -942,10 +965,15 @@ def request uri, req = nil, &block
bad_response = true
retry
- rescue IOError, EOFError, Timeout::Error,
- Errno::ECONNABORTED, Errno::ECONNRESET, Errno::EPIPE,
- Errno::EINVAL, Errno::ETIMEDOUT, OpenSSL::SSL::SSLError => e
+ rescue *RETRIED_EXCEPTIONS => e # retried on ruby 2
+ request_failed e, req, connection if
+ retried or not can_retry? req, @retried_on_ruby_2
+
+ reset connection
+ retried = true
+ retry
+ rescue Errno::EINVAL, Errno::ETIMEDOUT => e # not retried on ruby 2
request_failed e, req, connection if retried or not can_retry? req
reset connection
View
131 test/test_net_http_persistent.rb
@@ -228,17 +228,30 @@ def test_can_retry_eh_change_requests
assert @http.can_retry? post
end
-
- def test_can_retry_eh_idempotent
- head = Net::HTTP::Head.new '/'
- assert @http.can_retry? head
+ if RUBY_1 then
+ def test_can_retry_eh_idempotent
+ head = Net::HTTP::Head.new '/'
- post = Net::HTTP::Post.new '/'
+ assert @http.can_retry? head
- refute @http.can_retry? post
+ post = Net::HTTP::Post.new '/'
+
+ refute @http.can_retry? post
+ end
+ else
+ def test_can_retry_eh_idempotent
+ head = Net::HTTP::Head.new '/'
+
+ assert @http.can_retry? head
+ refute @http.can_retry? head, true
+
+ post = Net::HTTP::Post.new '/'
+
+ refute @http.can_retry? post
+ end
end
-
+
def test_cert_store_equals
@http.cert_store = :cert_store
@@ -912,23 +925,38 @@ def c.request(*a) raise Net::HTTPBadResponse end
assert_match %r%too many bad responses%, e.message
end
- def test_request_bad_response_retry
- c = basic_connection
- def c.request(*a)
- if defined? @called then
- r = Net::HTTPResponse.allocate
- r.instance_variable_set :@header, {}
- def r.http_version() '1.1' end
- r
- else
- @called = true
- raise Net::HTTPBadResponse
+ if RUBY_1 then
+ def test_request_bad_response_retry
+ c = basic_connection
+ def c.request(*a)
+ if defined? @called then
+ r = Net::HTTPResponse.allocate
+ r.instance_variable_set :@header, {}
+ def r.http_version() '1.1' end
+ r
+ else
+ @called = true
+ raise Net::HTTPBadResponse
+ end
end
+
+ @http.request @uri
+
+ assert c.finished?
end
+ else
+ def test_request_bad_response_retry
+ c = basic_connection
+ def c.request(*a)
+ raise Net::HTTPBadResponse
+ end
- @http.request @uri
+ assert_raises Net::HTTP::Persistent::Error do
+ @http.request @uri
+ end
- assert c.finished?
+ assert c.finished?
+ end
end
def test_request_bad_response_unsafe
@@ -1114,25 +1142,43 @@ def c.request(*a) raise Errno::ECONNRESET end
assert_match %r%too many connection resets%, e.message
end
- def test_request_reset_retry
- c = basic_connection
- touts[c.object_id] = Time.now
- def c.request(*a)
- if defined? @called then
- r = Net::HTTPResponse.allocate
- r.instance_variable_set :@header, {}
- def r.http_version() '1.1' end
- r
- else
- @called = true
- raise Errno::ECONNRESET
+ if RUBY_1 then
+ def test_request_reset_retry
+ c = basic_connection
+ touts[c.object_id] = Time.now
+ def c.request(*a)
+ if defined? @called then
+ r = Net::HTTPResponse.allocate
+ r.instance_variable_set :@header, {}
+ def r.http_version() '1.1' end
+ r
+ else
+ @called = true
+ raise Errno::ECONNRESET
+ end
end
+
+ @http.request @uri
+
+ assert c.reset?
+ assert c.finished?
end
+ else
+ def test_request_reset_retry
+ c = basic_connection
+ touts[c.object_id] = Time.now
- @http.request @uri
+ def c.request(*a)
+ raise Errno::ECONNRESET
+ end
- assert c.reset?
- assert c.finished?
+ assert_raises Net::HTTP::Persistent::Error do
+ @http.request @uri
+ end
+
+ refute c.reset?
+ assert c.finished?
+ end
end
def test_request_reset_unsafe
@@ -1252,14 +1298,25 @@ def test_retry_change_requests_equals
refute @http.retry_change_requests
- assert @http.can_retry?(get)
+ if RUBY_1 then
+ assert @http.can_retry?(get)
+ else
+ assert @http.can_retry?(get)
+ end
+ refute @http.can_retry?(get, true)
refute @http.can_retry?(post)
@http.retry_change_requests = true
assert @http.retry_change_requests
- assert @http.can_retry?(get)
+ if RUBY_1 then
+ assert @http.can_retry?(get)
+ else
+ assert @http.can_retry?(get)
+ refute @http.can_retry?(get, true)
+ end
+
assert @http.can_retry?(post)
end
Please sign in to comment.
Something went wrong with that request. Please try again.