feat(request): optionally retry all network failures

lib/stripe.rb

@@ -72,7 +72,7 @@ module Stripe
 
   class << self
     attr_accessor :api_key, :api_base, :verify_ssl_certs, :api_version, :connect_base, :uploads_base,
-                  :open_timeout, :read_timeout
+                  :open_timeout, :read_timeout, :on_successful_retry
   end
 
   def self.api_url(url='', api_base_url=nil)
@@ -130,33 +130,47 @@ def self.request(method, url, api_key, params={}, headers={}, api_base_url=nil)
                         :method => method, :open_timeout => open_timeout,
                         :payload => payload, :url => url, :timeout => read_timeout)
 
+    response = execute_request_with_rescues(request_opts, api_base_url)
+
+    [parse(response), api_key]
+  end
+
+  def self.max_retries_on_network_failure
+    @max_retries_on_network_failure || 0
+  end
+
+  def self.max_retries_on_network_failure=(val)
+    @max_retries_on_network_failure = val.to_i
+  end
+
+  private
+
+  def self.execute_request_with_rescues(request_opts, api_base_url, retry_count = 0)
     begin
       response = execute_request(request_opts)
     rescue SocketError => e
-      handle_restclient_error(e, api_base_url)
+      response = handle_restclient_error(e, request_opts, retry_count, api_base_url)
     rescue NoMethodError => e
       # Work around RestClient bug
       if e.message =~ /\WRequestFailed\W/
         e = APIConnectionError.new('Unexpected HTTP response code')
-        handle_restclient_error(e, api_base_url)
+        response = handle_restclient_error(e, request_opts, retry_count, api_base_url)
       else
         raise
       end
     rescue RestClient::ExceptionWithResponse => e
       if e.response
         handle_api_error(e.response)
       else
-        handle_restclient_error(e, api_base_url)
+        response = handle_restclient_error(e, request_opts, retry_count, api_base_url)
       end
     rescue RestClient::Exception, Errno::ECONNREFUSED => e
-      handle_restclient_error(e, api_base_url)
+      response = handle_restclient_error(e, request_opts, retry_count, api_base_url)
     end
 
-    [parse(response), api_key]
+    response
   end
 
-  private
-
   def self.user_agent
     @uname ||= get_uname
     lang_version = "#{RUBY_VERSION} p#{RUBY_PATCHLEVEL} (#{RUBY_RELEASE_DATE})"
@@ -214,6 +228,10 @@ def self.request_headers(api_key)
       :content_type => 'application/x-www-form-urlencoded'
     }
 
+    # It is only safe to retry network failures if we
+    # add an Idempotency-Key header
+    headers[:idempotency_key] ||= generate_random_idempotency_key if self.max_retries_on_network_failure > 0
+
     headers[:stripe_version] = api_version if api_version
 
     begin
@@ -224,6 +242,15 @@ def self.request_headers(api_key)
     end
   end
 
+  # the build machines run ruby 1.8.7, and so do not have SecureRandom
+  def self.generate_random_idempotency_key
+    if defined? SecureRandom && SecureRandom.respond_to?(:uuid)
+      SecureRandom.uuid
+    else
+      Time.now.to_f.to_s + rand.to_s
+    end
+  end
+
   def self.execute_request(opts)
     RestClient::Request.execute(opts)
   end
@@ -287,7 +314,16 @@ def self.api_error(error, resp, error_obj)
     APIError.new(error[:message], resp.code, resp.body, error_obj, resp.headers)
   end
 
-  def self.handle_restclient_error(e, api_base_url=nil)
+  def self.handle_restclient_error(e, request_opts, retry_count, api_base_url=nil)
+
+    if should_retry?(e, retry_count)
+      response = execute_request_with_rescues(request_opts, api_base_url, retry_count + 1)
+      if self.on_successful_retry
+        self.on_successful_retry.call(e, response)
+      end
+      return response
+    end
+
     api_base_url = @api_base unless api_base_url
     connection_message = "Please check your internet connection and try again. " \
         "If this problem persists, you should check Stripe's service status at " \
@@ -318,6 +354,16 @@ def self.handle_restclient_error(e, api_base_url=nil)
 
     end
 
+    if retry_count > 0
+      message += " Request was retried #{retry_count} times."
+    end
+
     raise APIConnectionError.new(message + "\n\n(Network error: #{e.message})")
   end
+
+  def self.should_retry?(e, retry_count)
+    return false unless self.max_retries_on_network_failure > retry_count
+    return false if e.is_a?(RestClient::SSLCertificateNotVerified)
+    return true
+  end
 end

lib/stripe/api_resource.rb

@@ -1,3 +1,4 @@
+
 module Stripe
   class APIResource < StripeObject
     include Stripe::APIOperations::Request

test/stripe/api_resource_test.rb

@@ -593,6 +593,49 @@ class ApiResourceTest < Test::Unit::TestCase
 
         acct.save
       end
+
+    end
+
+    should 'retry failed network requests if specified and raise if error persists' do
+      Stripe.expects(:max_retries_on_network_failure).returns(2).at_least_once
+      @mock.expects(:post).times(3).with('https://api.stripe.com/v1/charges', nil, 'amount=50&currency=usd').raises(Errno::ECONNREFUSED.new)
+
+      err = assert_raises Stripe::APIConnectionError do
+        Stripe::Charge.create(:amount => 50, :currency => 'usd', :card => { :number => nil })
+      end   
+      assert_match(/Request was retried 2 times/, err.message)   
+    end
+
+    should 'retry failed network requests if specified and return successful response' do
+      Stripe.expects(:max_retries_on_network_failure).returns(2).at_least_once
+      callback = Proc.new {}
+      Stripe.expects(:on_successful_retry).returns(callback).at_least_once
+      response = make_response({"id" => "myid"})
+      err = Errno::ECONNREFUSED.new
+      callback.expects(:call).with(err, response)
+      @mock.expects(:post).times(2).with('https://api.stripe.com/v1/charges', nil, 'amount=50&currency=usd').raises(err).then.returns(response)
+
+      result = Stripe::Charge.create(:amount => 50, :currency => 'usd', :card => { :number => nil })
+      assert_equal "myid", result.id
+    end
+
+    should 'ensure there is always an idempotency_key' do
+      Stripe.expects(:generate_random_idempotency_key).at_least_once.returns("random_key")
+      Stripe.expects(:max_retries_on_network_failure).returns(2).at_least_once
+      Stripe.expects(:execute_request).with do |opts|
+        opts[:headers][:idempotency_key] == "random_key"
+      end.returns(make_response({"id" => "myid"}))
+
+      Stripe::Charge.create(:amount => 50, :currency => 'usd', :card => { :number => nil })
+    end
+
+    should 'ensure not override a provided idempotency_key' do
+      Stripe.expects(:max_retries_on_network_failure).returns(2).at_least_once
+      Stripe.expects(:execute_request).with do |opts|
+        opts[:headers][:idempotency_key] == "provided_key"
+      end.returns(make_response({"id" => "myid"}))
+
+      Stripe::Charge.create({:amount => 50, :currency => 'usd', :card => { :number => nil }}, {:idempotency_key => 'provided_key'})
     end
   end
 end