add test to display retries

Author: silesky

.solargraph.yml

@@ -1,15 +1,15 @@
 ---
 include:
-- "**/*.rb"
+  - "**/*.rb"
 exclude:
-- vendor/**/*
-- ".bundle/**/*"
-- "bin/**/*"
+  - vendor/**/*
+  - ".bundle/**/*"
+  - "bin/**/*"
 require: []
 domains: []
 reporters:
-- rubocop
-- require_not_found
+  - rubocop
+  - require_not_found
 formatter:
   rubocop:
     cops: safe

Gemfile.lock

@@ -18,22 +18,26 @@ GEM
     coderay (1.1.3)
     crack (0.4.5)
       rexml
-    docile (1.3.5)
+    docile (1.4.0)
     e2mmap (0.1.0)
-    faraday (1.4.1)
+    faraday (1.4.2)
+      faraday-em_http (~> 1.0)
+      faraday-em_synchrony (~> 1.0)
       faraday-excon (~> 1.1)
       faraday-net_http (~> 1.0)
       faraday-net_http_persistent (~> 1.1)
       multipart-post (>= 1.2, < 3)
       ruby2_keywords (>= 0.0.4)
+    faraday-em_http (1.0.0)
+    faraday-em_synchrony (1.0.0)
     faraday-excon (1.1.0)
     faraday-net_http (1.0.1)
     faraday-net_http_persistent (1.1.0)
     faraday_middleware (1.0.0)
       faraday (~> 1.0)
-    ffi (1.15.0)
+    ffi (1.15.1)
     formatador (0.2.5)
-    guard (2.16.2)
+    guard (2.17.0)
       formatador (>= 0.2.4)
       listen (>= 2.7, < 4.0)
       lumberjack (>= 1.0.12, < 2.0)
@@ -57,11 +61,11 @@ GEM
       rb-inotify (~> 0.9, >= 0.9.10)
     lumberjack (1.2.8)
     method_source (1.0.0)
-    mini_portile2 (2.5.0)
+    mini_portile2 (2.5.1)
     minitest (5.14.4)
     minitest-fail-fast (0.1.0)
       minitest (~> 5)
-    minitest-focus (1.2.1)
+    minitest-focus (1.3.1)
       minitest (>= 4, < 6)
     minitest-hooks (1.5.0)
       minitest (> 5.3)
@@ -74,14 +78,14 @@ GEM
       ruby-progressbar
     multipart-post (2.1.1)
     nenv (0.3.0)
-    nokogiri (1.11.3)
+    nokogiri (1.11.6)
       mini_portile2 (~> 2.5.0)
       racc (~> 1.4)
     notiffany (0.1.3)
       nenv (~> 0.1)
       shellany (~> 0.0)
     parallel (1.20.1)
-    parser (3.0.1.0)
+    parser (3.0.1.1)
       ast (~> 2.4.1)
     pry (0.14.1)
       coderay (~> 1.1)
@@ -90,24 +94,24 @@ GEM
     racc (1.5.2)
     rainbow (3.0.0)
     rake (13.0.3)
-    rb-fsevent (0.10.4)
+    rb-fsevent (0.11.0)
     rb-inotify (0.10.1)
       ffi (~> 1.0)
     regexp_parser (2.1.1)
     reverse_markdown (2.0.0)
       nokogiri
     rexml (3.2.5)
-    rubocop (1.13.0)
+    rubocop (1.15.0)
       parallel (~> 1.10)
       parser (>= 3.0.0.0)
       rainbow (>= 2.2.2, < 4.0)
       regexp_parser (>= 1.8, < 3.0)
       rexml
-      rubocop-ast (>= 1.2.0, < 2.0)
+      rubocop-ast (>= 1.5.0, < 2.0)
       ruby-progressbar (~> 1.7)
       unicode-display_width (>= 1.4.0, < 3.0)
-    rubocop-ast (1.4.1)
-      parser (>= 2.7.1.5)
+    rubocop-ast (1.6.0)
+      parser (>= 3.0.1.1)
     ruby-progressbar (1.11.0)
     ruby2_keywords (0.0.4)
     shellany (0.0.1)
@@ -116,7 +120,7 @@ GEM
       simplecov-html (~> 0.11)
       simplecov_json_formatter (~> 0.1)
     simplecov-html (0.12.3)
-    simplecov_json_formatter (0.1.2)
+    simplecov_json_formatter (0.1.3)
     solargraph (0.40.4)
       backport (~> 1.1)
       benchmark
@@ -134,7 +138,7 @@ GEM
     thor (1.1.0)
     tilt (2.0.10)
     unicode-display_width (2.0.0)
-    webmock (3.12.2)
+    webmock (3.13.0)
       addressable (>= 2.3.6)
       crack (>= 0.3.2)
       hashdiff (>= 0.4.0, < 2.0.0)

lib/shipengine/exceptions.rb

@@ -68,20 +68,26 @@ def initialize(message:, code:, request_id: nil, source: nil)
     end
 
     class RateLimitError < SystemError
-      attr_reader :retry_attempt
-
-      def initialize(message: 'You have exceeded the rate limit.', source: nil, request_id: nil)
-        super(message: message, code: ErrorCode.get(:RATE_LIMIT_EXCEEDED), request_id: request_id, source: source)
+      attr_reader :retries
+
+      def initialize(retries: nil, message: 'You have exceeded the rate limit.', source: nil, request_id: nil)
+        super(
+          message: message,
+          code: ErrorCode.get(:RATE_LIMIT_EXCEEDED),
+          request_id: request_id,
+          source: source,
+        )
+        @retries = retries
       end
     end
 
     class TimeoutError < SystemError
-      def initialize(message:, source: nil, request_id: nil)
+      def initialize(message:, source: ::Exceptions::DEFAULT_SOURCE, request_id: nil)
         super(message: message, code: ErrorCode.get(:TIMEOUT), request_id: request_id, source: source)
       end
     end
 
-    def self.create_error_instance_by_type(type:, message:, code:, request_id: nil, source: nil)
+    def self.create_error_instance(type:, message:, code:, request_id: nil, source: nil, config: nil)
       case type
       when Exceptions::ErrorType.get(:BUSINESS_RULES)
         BusinessRulesError.new(message: message, code: code, request_id: request_id, source: source)
@@ -94,7 +100,7 @@ def self.create_error_instance_by_type(type:, message:, code:, request_id: nil,
       when Exceptions::ErrorType.get(:SYSTEM)
         case code
         when ErrorCode.get(:RATE_LIMIT_EXCEEDED)
-          RateLimitError.new(message: message, request_id: request_id, source: source)
+          RateLimitError.new(message: message, request_id: request_id, source: source, retries: config.retries)
         when ErrorCode.get(:TIMEOUT)
           TimeoutError.new(message: message, request_id: request_id, source: source)
         else

lib/shipengine/internal_client.rb

@@ -20,6 +20,8 @@ module CustomMiddleware
     # This transforms the `retryAfter` field from our JSON-RPC server to an HTTP header, so this client
     # can use the standard retry middleware from faraday-middleware.
     class RetryAfter < Faraday::Middleware
+      attr_reader :retry_attempt
+
       def initialize(app)
         super(app)
         @retry_attempt = 0
@@ -32,15 +34,11 @@ def on_complete(env)
         return env unless (status == 429) && body.is_a?(Hash) && body['error']
 
         # ShipEngineErrorLogger.log('Retrying...attempt: #{ @retry_attempt}')
-        env[:response_headers]['Retry-After'] = body.dig('error', 'data', 'retryAfter').to_s unless env[:response_headers]['Retry-After']
+        env[:response_headers]['Retry-After'] ||= body.dig('error', 'data', 'retryAfter').to_s
         @retry_attempt += 1
         env
       end
     end
-
-    def self.register_request_middleware
-      Faraday::Request.register_middleware(retry_after: RetryAfter)
-    end
   end
 
   class InternalClientResponseSuccess
@@ -59,7 +57,7 @@ class InternalClient
 
     # @param [::ShipEngine::Configuration] configuration
     def initialize(configuration)
-      CustomMiddleware.register_request_middleware
+      Faraday::Request.register_middleware(retry_after: CustomMiddleware::RetryAfter)
       @configuration = configuration
     end
 
@@ -83,20 +81,21 @@ def make_request(method, params, config = { api_key: nil, base_url: nil, retries
         req.body = build_jsonrpc_request_body(method, params)
       end
 
-      assert_shipengine_rpc_success(response)
+      assert_shipengine_rpc_success(response, config_with_overrides)
+
       result, id = response.body.values_at('result', 'id')
       InternalClientResponseSuccess.new(result: result, request_id: id)
     end
 
     private
 
-    # @param [::ShipEngine::Configuration] configuration
+    # @param config [::ShipEngine::Configuration]
     # @return [::Faraday::Connection]
-    def create_connection(configuration)
-      retries = configuration.retries
-      base_url = configuration.base_url
-      api_key = configuration.api_key
-      timeout = configuration.timeout
+    def create_connection(config)
+      retries = config.retries
+      base_url = config.base_url
+      api_key = config.api_key
+      timeout = config.timeout
       Faraday.new(url: base_url, request: { timeout: timeout }) do |f|
         f.request :json
         f.request :retry, {
@@ -111,13 +110,12 @@ def create_connection(configuration)
           'Accept' => 'application/json',
           'User-Agent' => "shipengine-ruby/#{VERSION} (#{RUBY_PLATFORM})"
         }
-
         f.response :json
       end
     end
 
-    # @param [String] method - e.g. "address.validate.v1"
-    # @param [Hash] params
+    # @param method [String] e.g. "address.validate.v1"
+    # @param params [Hash]
     # @return [Hash] - JSON:RPC response
     def build_jsonrpc_request_body(method, params)
       {
@@ -128,9 +126,10 @@ def build_jsonrpc_request_body(method, params)
       }
     end
 
-    def assert_shipengine_rpc_success(response)
+    # @param response [::Faraday::Response]
+    # @param config [::ShipEngine::Configuration]
+    def assert_shipengine_rpc_success(response, config)
       body = response.body
-
       unless body.is_a?(Hash)
         # this should not happen
         ShipEngineErrorLogger.log('response body is NOT a hash', [status: response.status, body: response.body])
@@ -142,7 +141,7 @@ def assert_shipengine_rpc_success(response)
 
       message, data = error.values_at('message', 'data')
       source, type, code = data.values_at('source', 'type', 'code')
-      raise Exceptions.create_error_instance_by_type(type: type, message: message, code: code, request_id: request_id, source: source)
+      raise Exceptions.create_error_instance(type: type, message: message, code: code, request_id: request_id, source: source, config: config)
     end
   end
 end

test/internal_client_test.rb

@@ -224,14 +224,24 @@ def self.rate_limit_error(data: {})
         assert_raises_rate_limit_error { client.validate_address(valid_address_params) }
       end
 
+      it 'should throw an error with the number of tries' do
+        client = ShipEngine::Client.new(api_key: 'abc123', retries: 2)
+        stub_request(:post, base_url)
+          .to_return(status: 429, body: Factory.rate_limit_error).then
+          .to_return(status: 429, body: Factory.rate_limit_error).then
+          .to_return(status: 429, body: Factory.rate_limit_error)
+        assert_raises_rate_limit_error(retries: 2) { client.validate_address(valid_address_params) }
+      end
+
       it 'respects the Retry-After header, which can override error.retryAfter' do
         client = ShipEngine::Client.new(api_key: 'abc123', retries: 1)
         stub = stub_request(:post, base_url)
-               .to_return(status: 429, body: Factory.rate_limit_error, headers: { "Retry-After": 1 }).then
-               .to_return(status: 200, body: valid_address_res)
+               .to_return(status: 429, body: Factory.rate_limit_error, headers: { "Retry-After": 1 })
+               .then.to_return(status: 200, body: valid_address_res)
         start = Time.now
         client.validate_address(valid_address_params)
         diff = Time.now - start
+
         assert_operator(diff, :>, 1, 'should take more than than 1 second')
         assert_requested(stub, times: 2)
       end
@@ -263,17 +273,17 @@ def self.rate_limit_error(data: {})
 
       # https://auctane.atlassian.net/browse/DX-1500
       it 'SLOW: should use the retryAfter field in errors to dictate how long it should wait to retry' do
-        retries = 2
+        retries = 3
         client = ShipEngine::Client.new(api_key: 'abc123', retries: retries)
         stub = stub_request(:post, base_url)
                .to_return(status: 429, body: Factory.rate_limit_error(data: { retryAfter: 1 })).then
-               .to_return(status: 429, body: Factory.rate_limit_error(data: { retryAfter: 2 })).then
+               .to_return(status: 429, body: Factory.rate_limit_error(data: { retryAfter: 1 })).then
+               .to_return(status: 429, body: Factory.rate_limit_error(data: { retryAfter: 1 })).then
                .to_return(status: 200, body: valid_address_res)
         start = Time.now
         client.validate_address(valid_address_params)
         diff = Time.now - start
-        assert_operator(diff, :>, 3, 'should take between 3 and 4 seconds')
-        assert_operator(diff, :<, 4, 'should take between 3 and 4 seconds')
+        assert(diff > 3 && diff < 4, 'should take between 3 and 4 seconds')
         assert_requested(stub, times: retries + 1)
       end
     end

test/test_utility/custom_assertions.rb

@@ -70,12 +70,13 @@ def assert_address_validation_result(expected_result, response_result)
     # rubocop:enable Layout/LineLength
   end
 
-  def assert_raises_rate_limit_error(&block)
-    assert_raises_shipengine(ShipEngine::Exceptions::RateLimitError, {
-                               code: 'rate_limit_exceeded',
-                               message: 'You have exceeded the rate limit.',
-                               source: 'shipengine'
-                             }, &block)
+  def assert_raises_rate_limit_error(retries: nil, &block)
+    err = assert_raises_shipengine(ShipEngine::Exceptions::RateLimitError, {
+                                     code: 'rate_limit_exceeded',
+                                     message: 'You have exceeded the rate limit.',
+                                     source: 'shipengine'
+                                   }, &block)
+    assert_equal(retries, err.retries) unless retries.nil?
   end
 
   # @param expected_messages [Array<Hash>]