Make retries for Net::HTTP configurable [Feature #10674]

by stereobooster
fix https://github.com/ruby/ruby/pull/1654

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@60035 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

lib/net/http.rb

@@ -670,6 +670,7 @@ module Net   #:nodoc:
       @open_timeout = 60
       @read_timeout = 60
       @continue_timeout = nil
+      @max_retries = 1
       @debug_output = nil
 
       @proxy_from_env = false
@@ -736,6 +737,21 @@ module Net   #:nodoc:
     # it raises a Net::ReadTimeout exception. The default value is 60 seconds.
     attr_reader :read_timeout
 
+    # Maximum number of times to retry an idempotent request in case of
+    # Net::ReadTimeout, IOError, EOFError, Errno::ECONNRESET,
+    # Errno::ECONNABORTED, Errno::EPIPE, OpenSSL::SSL, Timeout::Error
+    # Should be non-negative integer number. Zero means no retries.
+    # The default value is 1.
+    def max_retries=(retries)
+      retries = retries.to_int
+      if retries < 0
+        raise ArgumentError, 'max_retries should be non-negative integer number'
+      end
+      @max_retries = retries
+    end
+
+    attr_reader :max_retries
+
     # Setter for the read_timeout attribute.
     def read_timeout=(sec)
       @socket.read_timeout = sec if @socket
@@ -1477,7 +1493,7 @@ module Net   #:nodoc:
              # avoid a dependency on OpenSSL
              defined?(OpenSSL::SSL) ? OpenSSL::SSL::SSLError : IOError,
              Timeout::Error => exception
-        if count == 0 && IDEMPOTENT_METHODS_.include?(req.method)
+        if count < max_retries && IDEMPOTENT_METHODS_.include?(req.method)
           count += 1
           @socket.close if @socket
           D "Conn close because of error #{exception}, and retry"

test/net/http/test_http.rb

@@ -1045,6 +1045,56 @@ class TestNetHTTPKeepAlive < Test::Unit::TestCase
     }
   end
 
+  class MockSocket
+    attr_reader :count
+    def initialize(success_after: nil)
+      @success_after = success_after
+      @count = 0
+    end
+    def close
+    end
+    def closed?
+    end
+    def write(_)
+    end
+    def readline
+      @count += 1
+      if @success_after && @success_after <= @count
+        "HTTP/1.1 200 OK"
+      else
+        raise Errno::ECONNRESET
+      end
+    end
+    def readuntil(*_)
+      ""
+    end
+    def read_all(_)
+    end
+  end
+
+  def test_http_retry_success
+    start {|http|
+      socket = MockSocket.new(success_after: 10)
+      http.instance_variable_set(:@socket, socket)
+      assert_equal 0, socket.count
+      http.max_retries = 10
+      res = http.get('/')
+      assert_equal 10, socket.count
+      assert_kind_of Net::HTTPResponse, res
+      assert_kind_of String, res.body
+    }
+  end
+
+  def test_http_retry_failed
+    start {|http|
+      socket = MockSocket.new
+      http.instance_variable_set(:@socket, socket)
+      http.max_retries = 10
+      assert_raise(Errno::ECONNRESET){ http.get('/') }
+      assert_equal 11, socket.count
+    }
+  end
+
   def test_keep_alive_server_close
     def @server.run(sock)
       sock.close