User-friendly messages and retries for EOFError and a few more networ…

…k errors (#859)

lib/stripe/stripe_client.rb

@@ -81,17 +81,17 @@ def self.default_connection_manager
     def self.should_retry?(error, method:, num_retries:)
       return false if num_retries >= Stripe.max_network_retries
 
-      # Retry on timeout-related problems (either on open or read).
-      return true if error.is_a?(Net::OpenTimeout)
-      return true if error.is_a?(Net::ReadTimeout)
-
-      # Destination refused the connection, the connection was reset, or a
-      # variety of other connection failures. This could occur from a single
-      # saturated server, so retry in case it's intermittent.
-      return true if error.is_a?(Errno::ECONNREFUSED)
-      return true if error.is_a?(SocketError)
-
-      if error.is_a?(Stripe::StripeError)
+      case error
+      when Net::OpenTimeout, Net::ReadTimeout
+        # Retry on timeout-related problems (either on open or read).
+        true
+      when EOFError, Errno::ECONNREFUSED, Errno::ECONNRESET,
+            Errno::EHOSTUNREACH, Errno::ETIMEDOUT, SocketError
+        # Destination refused the connection, the connection was reset, or a
+        # variety of other connection failures. This could occur from a single
+        # saturated server, so retry in case it's intermittent.
+        true
+      when Stripe::StripeError
         # The API may ask us not to retry (e.g. if doing so would be a no-op),
         # or advise us to retry (e.g. in cases of lock timeouts). Defer to
         # those instructions if given.
@@ -119,10 +119,10 @@ def self.should_retry?(error, method:, num_retries:)
         return true if error.http_status == 500 && method != :post
 
         # 503 Service Unavailable
-        return true if error.http_status == 503
+        error.http_status == 503
+      else
+        false
       end
-
-      false
     end
 
     def self.sleep_time(num_retries)
@@ -296,7 +296,11 @@ def execute_request(method, path,
     # The original error message is also appended onto the final exception for
     # full transparency.
     NETWORK_ERROR_MESSAGES_MAP = {
+      EOFError => ERROR_MESSAGE_CONNECTION,
       Errno::ECONNREFUSED => ERROR_MESSAGE_CONNECTION,
+      Errno::ECONNRESET => ERROR_MESSAGE_CONNECTION,
+      Errno::EHOSTUNREACH => ERROR_MESSAGE_CONNECTION,
+      Errno::ETIMEDOUT => ERROR_MESSAGE_TIMEOUT_CONNECT,
       SocketError => ERROR_MESSAGE_CONNECTION,
 
       Net::OpenTimeout => ERROR_MESSAGE_TIMEOUT_CONNECT,

test/stripe/stripe_client_test.rb

@@ -172,6 +172,26 @@ class StripeClientTest < Test::Unit::TestCase
                                           method: :post, num_retries: 0)
       end
 
+      should "retry on EOFError" do
+        assert StripeClient.should_retry?(EOFError.new,
+                                          method: :post, num_retries: 0)
+      end
+
+      should "retry on Errno::ECONNRESET" do
+        assert StripeClient.should_retry?(Errno::ECONNRESET.new,
+                                          method: :post, num_retries: 0)
+      end
+
+      should "retry on Errno::ETIMEDOUT" do
+        assert StripeClient.should_retry?(Errno::ETIMEDOUT.new,
+                                          method: :post, num_retries: 0)
+      end
+
+      should "retry on Errno::EHOSTUNREACH" do
+        assert StripeClient.should_retry?(Errno::EHOSTUNREACH.new,
+                                          method: :post, num_retries: 0)
+      end
+
       should "retry on Net::OpenTimeout" do
         assert StripeClient.should_retry?(Net::OpenTimeout.new,
                                           method: :post, num_retries: 0)