remove retry logic

History.txt

@@ -1,3 +1,9 @@
+=== Next
+
+Breaking changes:
+
+* Requests are no longer retried by net-http-persistent.
+
 === 3.0
 
 Breaking changes:

lib/net/http/persistent.rb

@@ -136,45 +136,6 @@
 # Socket options may be set on newly-created connections.  See #socket_options
 # for details.
 #
-# === Non-Idempotent Requests
-#
-# By default non-idempotent requests will not be retried per RFC 2616.  By
-# setting retry_change_requests to true requests will automatically be retried
-# once.
-#
-# Only do this when you know that retrying a POST or other non-idempotent
-# request is safe for your application and will not create duplicate
-# resources.
-#
-# The recommended way to handle non-idempotent requests is the following:
-#
-#   require 'net/http/persistent'
-#
-#   uri = URI 'http://example.com/awesome/web/service'
-#   post_uri = uri + 'create'
-#
-#   http = Net::HTTP::Persistent.new name: 'my_app_name'
-#
-#   post = Net::HTTP::Post.new post_uri.path
-#   # ... fill in POST request
-#
-#   begin
-#     response = http.request post_uri, post
-#   rescue Net::HTTP::Persistent::Error
-#
-#     # POST failed, make a new request to verify the server did not process
-#     # the request
-#     exists_uri = uri + '...'
-#     response = http.get exists_uri
-#
-#     # Retry if it failed
-#     retry if response.code == '404'
-#   end
-#
-# The method of determining if the resource was created or not is unique to
-# the particular service you are using.  Of course, you will want to add
-# protection from infinite looping.
-#
 # === Connection Termination
 #
 # If you are done using the Net::HTTP::Persistent instance you may shut down
@@ -209,21 +170,6 @@ class Net::HTTP::Persistent
 
   VERSION = '3.0.0'
 
-  ##
-  # 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 if HAVE_OPENSSL),
-    Timeout::Error,
-  ].compact
-
   ##
   # Error class for errors raised by Net::HTTP::Persistent.  Various
   # SystemCallErrors are re-raised with a human-readable message under this
@@ -491,17 +437,6 @@ def self.detect_idle_timeout uri, max = 10
 
   attr_reader :verify_mode
 
-  ##
-  # Enable retries of non-idempotent requests that change data (e.g. POST
-  # requests) when the server has disconnected.
-  #
-  # This will in the worst case lead to multiple requests with the same data,
-  # but it may be useful for some applications.  Take care when enabling
-  # this option to ensure it is safe to POST or perform other non-idempotent
-  # requests to the server.
-
-  attr_accessor :retry_change_requests
-
   ##
   # Creates a new Net::HTTP::Persistent.
   #
@@ -569,8 +504,6 @@ def initialize name: nil, proxy: nil, pool_size: DEFAULT_POOL_SIZE
       @reuse_ssl_sessions = OpenSSL::SSL.const_defined? :Session
     end
 
-    @retry_change_requests = false
-
     self.proxy = proxy if proxy
   end
 
@@ -667,19 +600,6 @@ def connection_for uri
     @pool.checkin net_http_args
   end
 
-  ##
-  # Returns an error message containing the number of requests performed on
-  # this connection
-
-  def error_message connection
-    connection.requests -= 1 # fixup
-
-    age = Time.now - connection.last_use
-
-    "after #{connection.requests} requests on #{connection.http.object_id}, " \
-      "last used #{age} seconds ago"
-  end
-
   ##
   # URI::escape wrapper
 
@@ -742,24 +662,6 @@ def http_version uri
     @http_versions["#{uri.host}:#{uri.port}"]
   end
 
-  ##
-  # Is +req+ idempotent according to RFC 2616?
-
-  def idempotent? req
-    case req
-    when Net::HTTP::Delete, Net::HTTP::Get, Net::HTTP::Head,
-         Net::HTTP::Options, Net::HTTP::Put, Net::HTTP::Trace then
-      true
-    end
-  end
-
-  ##
-  # Is the request +req+ idempotent or is retry_change_requests allowed.
-
-  def can_retry? req
-    @retry_change_requests && !idempotent?(req)
-  end
-
   ##
   # Adds "http://" to the String +uri+ if it is missing.
 
@@ -937,14 +839,8 @@ def reset connection
   # the response will not have been read).
   #
   # +req+ must be a Net::HTTPRequest subclass (see Net::HTTP for a list).
-  #
-  # If there is an error and the request is idempotent according to RFC 2616
-  # it will be retried automatically.
 
   def request uri, req = nil, &block
-    retried      = false
-    bad_response = false
-
     uri      = URI uri
     req      = request_setup req || uri
     response = nil
@@ -957,38 +853,10 @@ def request uri, req = nil, &block
 
         response = http.request req, &block
 
-        if req.connection_close? or
-           (response.http_version <= '1.0' and
-            not response.connection_keep_alive?) or
-           response.connection_close? then
+        if req.connection_close? or !keepalive_supported?(response)
           finish connection
         end
-      rescue Net::HTTPBadResponse => e
-        message = error_message connection
-
-        finish connection
-
-        raise Error, "too many bad responses #{message}" if
-        bad_response or not can_retry? req
-
-        bad_response = true
-        retry
-      rescue *RETRIED_EXCEPTIONS => e
-        request_failed e, req, connection if
-          retried or not can_retry? req
-
-        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
-
-        retried = true
-        retry
-      rescue Exception => e
+      rescue Exception # make sure to close the connection when it was interrupted
         finish connection
 
         raise
@@ -1002,21 +870,6 @@ def request uri, req = nil, &block
     response
   end
 
-  ##
-  # Raises an Error for +exception+ which resulted from attempting the request
-  # +req+ on the +connection+.
-  #
-  # Finishes the +connection+.
-
-  def request_failed exception, req, connection # :nodoc:
-    due_to = "(due to #{exception.message} - #{exception.class})"
-    message = "too many connection resets #{due_to} #{error_message connection}"
-
-    finish connection
-
-    raise Error, message, exception.backtrace
-  end
-
   ##
   # Creates a GET request if +req_or_uri+ is a URI and adds headers to the
   # request.
@@ -1189,6 +1042,12 @@ def verify_callback= callback
     reconnect_ssl
   end
 
+  private
+
+  def keepalive_supported?(response)
+    (response.http_version > '1.0' and response.connection_keep_alive?) or
+      response.connection_close?
+  end
 end
 
 require 'net/http/persistent/connection'