Merge pull request #353 from nbrustein/retry-network-failures

feat(request): optionally retry network failures

README.rdoc

@@ -59,3 +59,16 @@ Run a single test suite:
 Run a single test:
 
     bundle exec ruby -Ilib/ test/stripe/util_test.rb -n /should.convert.names.to.symbols/
+
+== Configuration
+
+=== max_network_retries
+
+When `max_network_retries` is set to a positive integer, stripe will retry requests that 
+fail on a network error.  Idempotency keys will be added to post and get requests to ensure the 
+safety of retrying.  There will be a short delay between each retry, with an exponential backoff
+algorithm used to determine the length of the delay.  Default value is 0.
+
+Example:
+
+    Stripe.max_network_retries = 2

lib/stripe.rb

@@ -69,6 +69,9 @@ module Stripe
   @connect_base = 'https://connect.stripe.com'
   @uploads_base = 'https://uploads.stripe.com'
 
+  @max_network_retry_delay = 2
+  @initial_network_retry_delay = 0.5
+
   @ssl_bundle_path  = DEFAULT_CA_BUNDLE_PATH
   @verify_ssl_certs = true
 
@@ -78,6 +81,8 @@ module Stripe
   class << self
     attr_accessor :api_key, :api_base, :verify_ssl_certs, :api_version, :connect_base, :uploads_base,
                   :open_timeout, :read_timeout
+
+    attr_reader :max_network_retry_delay, :initial_network_retry_delay
   end
 
   def self.api_url(url='', api_base_url=nil)
@@ -131,37 +136,51 @@ def self.request(method, url, api_key, params={}, headers={}, api_base_url=nil)
       end
     end
 
-    request_opts.update(:headers => request_headers(api_key).update(headers),
+    request_opts.update(:headers => request_headers(api_key, method).update(headers),
                         :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_network_retries
+    @max_network_retries || 0
+  end
+
+  def self.max_network_retries=(val)
+    @max_network_retries = 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})"
@@ -215,13 +234,19 @@ class << self
     deprecate :uri_encode, "Stripe::Util#encode_parameters", 2016, 01
   end
 
-  def self.request_headers(api_key)
+  def self.request_headers(api_key, method)
     headers = {
       :user_agent => "Stripe/v1 RubyBindings/#{Stripe::VERSION}",
       :authorization => "Bearer #{api_key}",
       :content_type => 'application/x-www-form-urlencoded'
     }
 
+    # It is only safe to retry network failures on post and delete
+    # requests if we add an Idempotency-Key header
+    if [:post, :delete].include?(method) && self.max_network_retries > 0
+      headers[:idempotency_key] ||= SecureRandom.uuid 
+    end
+
     headers[:stripe_version] = api_version if api_version
 
     begin
@@ -303,7 +328,15 @@ 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)
+      retry_count = retry_count + 1
+      sleep sleep_time(retry_count)
+      response = execute_request_with_rescues(request_opts, api_base_url, retry_count)
+      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 " \
@@ -334,6 +367,31 @@ 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 if retry_count >= self.max_network_retries
+    return false if e.is_a?(RestClient::SSLCertificateNotVerified)
+    return true
+  end
+
+  def self.sleep_time(retry_count)
+    # This method was adapted from https://github.com/ooyala/retries/blob/master/lib/retries.rb
+
+    # The sleep time is an exponentially-increasing function of base_sleep_seconds. But, it never exceeds
+    # max_sleep_seconds.
+    sleep_seconds = [initial_network_retry_delay * (2 ** (retry_count - 1)), max_network_retry_delay].min
+    # Randomize to a random value in the range sleep_seconds/2 .. sleep_seconds
+
+    sleep_seconds = sleep_seconds * (0.5 * (1 + rand()))
+    # But never sleep less than base_sleep_seconds
+    sleep_seconds = [initial_network_retry_delay, sleep_seconds].max
+
+    sleep_seconds
+  end
 end

test/stripe/api_resource_test.rb

@@ -658,5 +658,119 @@ class ApiResourceTest < Test::Unit::TestCase
         account.save(:display_name => 'stripe')
       end
     end
+
+    context "with retries" do
+
+      setup do
+        Stripe.stubs(:max_network_retries).returns(2)
+      end
+
+      should 'retry failed network requests if specified and raise if error persists' do        
+        Stripe.expects(:sleep_time).at_least_once.returns(0)
+        @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(:sleep_time).at_least_once.returns(0)
+        response = make_response({"id" => "myid"})
+        err = Errno::ECONNREFUSED.new
+        @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 'not retry a SSLCertificateNotVerified error' do
+        @mock.expects(:post).times(1).with('https://api.stripe.com/v1/charges', nil, 'amount=50&currency=usd').raises(RestClient::SSLCertificateNotVerified.new('message'))
+
+        err = assert_raises Stripe::APIConnectionError do
+          Stripe::Charge.create(:amount => 50, :currency => 'usd', :card => { :number => nil })
+        end   
+        assert_no_match(/retried/, err.message)   
+      end
+
+      should 'not add an idempotency key to GET requests' do
+        SecureRandom.expects(:uuid).times(0)
+        Stripe.expects(:execute_request).with do |opts|
+          opts[:headers][:idempotency_key].nil?
+        end.returns(make_response({"id" => "myid"}))
+
+        Stripe::Charge.list
+      end
+
+      should 'ensure there is always an idempotency_key on POST requests' do
+        SecureRandom.expects(:uuid).at_least_once.returns("random_key")
+        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 there is always an idempotency_key on DELETE requests' do
+        SecureRandom.expects(:uuid).at_least_once.returns("random_key")
+        Stripe.expects(:execute_request).with do |opts|
+          opts[:headers][:idempotency_key] == "random_key"
+        end.returns(make_response({"id" => "myid"}))
+
+        c = Stripe::Customer.construct_from(make_customer)
+        c.delete
+      end
+
+      should 'not override a provided idempotency_key' do
+        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
+
+    context "sleep_time" do
+
+      should "should grow exponentially" do
+        Stripe.stubs(:rand).returns(1)
+        Stripe.stubs(:max_network_retry_delay).returns(999)
+        assert_equal(Stripe.initial_network_retry_delay, Stripe.sleep_time(1))
+        assert_equal(Stripe.initial_network_retry_delay * 2, Stripe.sleep_time(2))
+        assert_equal(Stripe.initial_network_retry_delay * 4, Stripe.sleep_time(3))
+        assert_equal(Stripe.initial_network_retry_delay * 8, Stripe.sleep_time(4))
+      end
+
+      should "enforce the max_network_retry_delay" do
+        Stripe.stubs(:rand).returns(1)
+        Stripe.stubs(:initial_network_retry_delay).returns(1)
+        Stripe.stubs(:max_network_retry_delay).returns(2)
+        assert_equal(1, Stripe.sleep_time(1))
+        assert_equal(2, Stripe.sleep_time(2))
+        assert_equal(2, Stripe.sleep_time(3))
+        assert_equal(2, Stripe.sleep_time(4))
+      end
+
+      should "add some randomness" do
+        random_value = 0.8
+        Stripe.stubs(:rand).returns(random_value)
+        Stripe.stubs(:initial_network_retry_delay).returns(1)
+        Stripe.stubs(:max_network_retry_delay).returns(8)
+
+        base_value = Stripe.initial_network_retry_delay * (0.5 * (1 + random_value))
+
+        # the initial value cannot be smaller than the base,
+        # so the randomness is ignored
+        assert_equal(Stripe.initial_network_retry_delay, Stripe.sleep_time(1))
+
+        # after the first one, the randomness is applied
+        assert_equal(base_value * 2, Stripe.sleep_time(2))
+        assert_equal(base_value * 4, Stripe.sleep_time(3))
+        assert_equal(base_value * 8, Stripe.sleep_time(4))
+      end
+
+    end
   end
 end