From af33564900e113c78e213a85385c467178ca0a94 Mon Sep 17 00:00:00 2001
From: Alexey Zapparov <alexey@zapparov.com>
Date: Wed, 26 Feb 2020 18:26:25 +0100
Subject: [PATCH 1/8] POC: Add retriable client

---
 lib/http/chainable.rb           | 22 ++++++++
 lib/http/retriable/client.rb    | 33 ++++++++++++
 lib/http/retriable/errors.rb    |  4 ++
 lib/http/retriable/performer.rb | 92 +++++++++++++++++++++++++++++++++
 4 files changed, 151 insertions(+)
 create mode 100644 lib/http/retriable/client.rb
 create mode 100644 lib/http/retriable/errors.rb
 create mode 100644 lib/http/retriable/performer.rb

diff --git a/lib/http/chainable.rb b/lib/http/chainable.rb
index 70a32bd4..06c0ec80 100644
--- a/lib/http/chainable.rb
+++ b/lib/http/chainable.rb
@@ -246,6 +246,28 @@ def use(*features)
       branch default_options.with_features(features)
     end
 
+    # Returns retriable client instance, which retries requests if they failed
+    # due to some socket errors or response status is `5xx`.
+    #
+    # @example Usage
+    #
+    #   # Retry max 5 times with randomly growing delay between retries
+    #   HTTP.retriable.get(url)
+    #
+    #   # Retry max 3 times with randomly growing delay between retries
+    #   HTTP.retriable(:times => 3).get(url)
+    #
+    #   # Retry max 3 times with 1 sec delay between retries
+    #   HTTP.retriable(:times => 3, :delay => proc { 1 }).get(url)
+    #
+    #   # Retry max 3 times with geometrically progressed delay between retries
+    #   HTTP.retriable(:times => 3, :delay => proc { |i| 1 + i*i }).get(url)
+    #
+    # @param (see Performer#initialize)
+    def retriable(options = {})
+      Retriable::Client.new(Retriable::Performer.new(options), default_options)
+    end
+
     private
 
     # :nodoc:
diff --git a/lib/http/retriable/client.rb b/lib/http/retriable/client.rb
new file mode 100644
index 00000000..b8cd7a4a
--- /dev/null
+++ b/lib/http/retriable/client.rb
@@ -0,0 +1,33 @@
+module HTTP
+  module Retriable
+    # Retriable version of HTTP::Client.
+    #
+    # @see http://www.rubydoc.info/gems/http/HTTP/Client
+    class Client < HTTP::Client
+      # @param [Performer] performer
+      # @param [HTTP::Options, Hash] options
+      def initialize(performer, options)
+        @performer = performer
+        super(options)
+      end
+
+      # Overriden version of `HTTP::Client#make_request`.
+      #
+      # Monitors request/response phase with performer.
+      #
+      # @see http://www.rubydoc.info/gems/http/HTTP/Client:perform
+      def perform(req, options)
+        @performer.perform(self, req) { super(req, options) }
+      end
+
+      private
+
+      # Overriden version of `HTTP::Chainable#branch`.
+      #
+      # @return [HTTP::Retriable::Client]
+      def branch(options)
+        Retriable::Client.new(@performer, options)
+      end
+    end
+  end
+end
diff --git a/lib/http/retriable/errors.rb b/lib/http/retriable/errors.rb
new file mode 100644
index 00000000..37bea2a4
--- /dev/null
+++ b/lib/http/retriable/errors.rb
@@ -0,0 +1,4 @@
+module HTTP
+  # Retriable performance ran out of attempts
+  class OutOfRetriesError < Error; end
+end
diff --git a/lib/http/retriable/performer.rb b/lib/http/retriable/performer.rb
new file mode 100644
index 00000000..696bac69
--- /dev/null
+++ b/lib/http/retriable/performer.rb
@@ -0,0 +1,92 @@
+# frozen_string_literal: true
+
+require "http"
+require "http/retriable/errors"
+require "http/retriable/client"
+require "openssl"
+
+module HTTP
+  module Retriable
+    # Request performing watchdog.
+    # @api private
+    class Performer
+      # Exceptions we should retry
+      RETRIABLE_ERRORS = [
+        HTTP::TimeoutError,
+        HTTP::ConnectionError,
+        IO::EAGAINWaitReadable,
+        Errno::ECONNRESET,
+        Errno::ECONNREFUSED,
+        Errno::EHOSTUNREACH,
+        OpenSSL::SSL::SSLError,
+        EOFError,
+        IOError
+      ].freeze
+
+      # Default retry delay proc
+      DELAY_PROC = proc { |i| 1 + i * rand }
+
+      # @param [Hash] opts
+      # @option opts [#to_i] :tries (5)
+      # @option opts [#call] :delay (DELAY_PROC)
+      def initialize(opts)
+        @tries = opts.fetch(:tries, 5).to_i
+        @delay = opts.fetch(:delay, DELAY_PROC)
+      end
+
+      # Watches request/response execution.
+      #
+      # If any of {RETRIABLE_ERRORS} occur or response status is `5xx`, retries
+      # up to `:tries` amount of times. Sleeps for amount of seconds calculated
+      # with `:delay` proc before each retry.
+      #
+      # @see #initialize
+      # @api private
+      def perform(client, req)
+        i = 0
+
+        while i < @tries
+          begin
+            res = yield
+            return res unless 500 <= res.status.to_i
+
+            # Some servers support Keep-Alive on any response. Thus we should
+            # flush response before retry, to avoid state error (when socket
+            # has pending response data and we try to write new request).
+            # Alternatively, as we don't need response body here at all, we
+            # are going to close client, effectivle closing underlying socket
+            # and resetting client's state.
+            client.close
+          rescue *RETRIABLE_ERRORS => e
+            client.close
+            err = e
+          rescue
+            client.close
+            raise
+          end
+
+          sleep @delay.call i
+          i += 1
+        end
+
+        raise OutOfRetriesError, error_message(req, res&.status, err)
+      end
+
+      private
+
+      # Builds out of retries error message.
+      #
+      # @param req [HTTP::Request]
+      # @param status [HTTP::Response::Status, nil]
+      # @param exception [Exception, nil]
+      def error_message(req, status, exception)
+        message = "#{req.verb.to_s.upcase} <#{req.uri}> failed"
+
+        message += " with #{status}" if status
+        message += ":#{exception}"   if exception
+
+        message
+      end
+    end
+  end
+end

From 2650cbc1c12363b468431fbd1d16fc89c724a314 Mon Sep 17 00:00:00 2001
From: Bert Goethals <bert@bertg.be>
Date: Tue, 3 Mar 2020 12:24:30 +0100
Subject: [PATCH 2/8] Fix require order

---
 lib/http.rb                     | 1 +
 lib/http/retriable/client.rb    | 2 ++
 lib/http/retriable/performer.rb | 1 -
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/http.rb b/lib/http.rb
index afc57fe6..108fc4ae 100644
--- a/lib/http.rb
+++ b/lib/http.rb
@@ -6,6 +6,7 @@
 require "http/timeout/global"
 require "http/chainable"
 require "http/client"
+require "http/retriable/client"
 require "http/connection"
 require "http/options"
 require "http/feature"
diff --git a/lib/http/retriable/client.rb b/lib/http/retriable/client.rb
index b8cd7a4a..cef41b5b 100644
--- a/lib/http/retriable/client.rb
+++ b/lib/http/retriable/client.rb
@@ -1,3 +1,5 @@
+require "http/retriable/performer"
+
 module HTTP
   module Retriable
     # Retriable version of HTTP::Client.
diff --git a/lib/http/retriable/performer.rb b/lib/http/retriable/performer.rb
index 696bac69..e4f430f9 100644
--- a/lib/http/retriable/performer.rb
+++ b/lib/http/retriable/performer.rb
@@ -2,7 +2,6 @@
 
 require "http"
 require "http/retriable/errors"
-require "http/retriable/client"
 require "openssl"
 
 module HTTP

From 50a62e1dec8bbe8160e34cbc795927eefe10cb0e Mon Sep 17 00:00:00 2001
From: Bert Goethals <bert@bertg.be>
Date: Tue, 3 Mar 2020 12:24:42 +0100
Subject: [PATCH 3/8] Complete Retryable POC, with basic retry features

* Delay by default backs backs off over time
* Maximum delay time
* Exceptions to retry from
* Status codes to retry from
* Custom retry logic
* Respect Retry-After header if present
* on_retry callback
---
 lib/http/chainable.rb                     |   2 +-
 lib/http/retriable/client.rb              |   2 +
 lib/http/retriable/errors.rb              |  14 +-
 lib/http/retriable/performer.rb           | 141 +++++++--
 spec/lib/http/retriable/performer_spec.rb | 331 ++++++++++++++++++++++
 spec/lib/http_spec.rb                     |  31 ++
 spec/support/dummy_server.rb              |   3 +-
 spec/support/dummy_server/servlet.rb      |  13 +
 8 files changed, 507 insertions(+), 30 deletions(-)
 create mode 100644 spec/lib/http/retriable/performer_spec.rb

diff --git a/lib/http/chainable.rb b/lib/http/chainable.rb
index 06c0ec80..38e31e4f 100644
--- a/lib/http/chainable.rb
+++ b/lib/http/chainable.rb
@@ -264,7 +264,7 @@ def use(*features)
     #   HTTP.retriable(:times => 3, :delay => proc { |i| 1 + i*i }).get(url)
     #
     # @param (see Performer#initialize)
-    def retriable(options = {})
+    def retriable(**options)
       Retriable::Client.new(Retriable::Performer.new(options), default_options)
     end
 
diff --git a/lib/http/retriable/client.rb b/lib/http/retriable/client.rb
index cef41b5b..7edfd3a5 100644
--- a/lib/http/retriable/client.rb
+++ b/lib/http/retriable/client.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require "http/retriable/performer"
 
 module HTTP
diff --git a/lib/http/retriable/errors.rb b/lib/http/retriable/errors.rb
index 37bea2a4..9da048a2 100644
--- a/lib/http/retriable/errors.rb
+++ b/lib/http/retriable/errors.rb
@@ -1,4 +1,16 @@
+# frozen_string_literal: true
+
 module HTTP
   # Retriable performance ran out of attempts
-  class OutOfRetriesError < Error; end
+  class OutOfRetriesError < Error
+    attr_accessor :response
+
+    def cause=(exception)
+      @cause = exception
+    end
+
+    def cause
+      @cause || super
+    end
+  end
 end
diff --git a/lib/http/retriable/performer.rb b/lib/http/retriable/performer.rb
index e4f430f9..60c46bda 100644
--- a/lib/http/retriable/performer.rb
+++ b/lib/http/retriable/performer.rb
@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require "date"
 require "http"
 require "http/retriable/errors"
 require "openssl"
@@ -22,15 +23,48 @@ class Performer
         IOError
       ].freeze
 
+      RETRIABLE_STATUSES = [500].freeze
+
       # Default retry delay proc
-      DELAY_PROC = proc { |i| 1 + i * rand }
+      DELAY_PROC = ->(i) {
+        delay = 2**(i - 1) - 1
+        delay_noise = rand
+        delay + delay_noise
+      }
 
       # @param [Hash] opts
       # @option opts [#to_i] :tries (5)
-      # @option opts [#call] :delay (DELAY_PROC)
+      # @option opts [#call, #to_i] :delay (DELAY_PROC)
+      # @option opts [Array(Exception)] :exceptions (RETRIABLE_ERRORS)
+      # @option opts [Array(#to_i)] :retry_statuses ([500])
+      # @option opts [#call] :on_retry
+      # @option opts [#to_f] :max_delay (Float::MAX)
+      # @option opts [#call] :should_retry
       def initialize(opts)
+        if opts.key?(:should_retry)
+          @should_retry_proc = opts.fetch(:should_retry)
+        else
+          @exception_classes = opts.fetch(:exceptions, RETRIABLE_ERRORS)
+          @retry_statuses = opts.fetch(:retry_statuses, RETRIABLE_STATUSES)
+          @should_retry_proc = ->(_req, err, res, _i) {
+            if err
+              @exception_classes.any? { |e| err.is_a?(e) }
+            else
+              @retry_statuses.include?(res.status.to_i)
+            end
+          }
+        end
         @tries = opts.fetch(:tries, 5).to_i
-        @delay = opts.fetch(:delay, DELAY_PROC)
+        @on_retry = opts.fetch(:on_retry, ->(*) {})
+        @maximum_delay = opts.fetch(:max_delay, Float::MAX).to_f
+        @delay = begin
+          case delay = opts.fetch(:delay, DELAY_PROC)
+          when Numeric
+            ->(*) { delay }
+          else
+            delay
+          end
+        end
       end
 
       # Watches request/response execution.
@@ -42,49 +76,102 @@ def initialize(opts)
       # @see #initialize
       # @api private
       def perform(client, req)
-        i = 0
+        1.upto(Float::INFINITY) do |i| # infinite loop with index
+          err, res = nil
 
-        while i < @tries
+          # rubocop:disable Lint/RescueException
           begin
             res = yield
-            return res unless 500 <= res.status.to_i
-
-            # Some servers support Keep-Alive on any response. Thus we should
-            # flush response before retry, to avoid state error (when socket
-            # has pending response data and we try to write new request).
-            # Alternatively, as we don't need response body here at all, we
-            # are going to close client, effectivle closing underlying socket
-            # and resetting client's state.
-            client.close
-          rescue *RETRIABLE_ERRORS => e
-            client.close
+          rescue Exception => e
             err = e
-          rescue
+          end
+          # rubocop:enable Lint/RescueException
+
+          if @should_retry_proc.call(req, err, res, i)
+            if i < @tries
+              @on_retry.call(req, err, res)
+
+              # Some servers support Keep-Alive on any response. Thus we should
+              # flush response before retry, to avoid state error (when socket
+              # has pending response data and we try to write new request).
+              # Alternatively, as we don't need response body here at all, we
+              # are going to close client, effectivle closing underlying socket
+              # and resetting client's state.
+              client.close
+
+              sleep calculate_delay(i, res)
+            else
+              res&.flush
+              client.close
+              raise out_of_retries_error(req, res, err)
+            end
+          elsif err
             client.close
-            raise
+            raise err
+          elsif res
+            return res
           end
+        end
+      end
 
-          sleep @delay.call i
-          i += 1
+      def calculate_delay(iteration, response)
+        if response && (retry_header = response.headers["Retry-After"])
+          delay_from_retry_header(retry_header)
+        else
+          calculate_delay_from_iteration(iteration)
         end
+      end
+
+      RFC2822_DATE_REGEX = /^
+        (?:Sun|Mon|Tue|Wed|Thu|Fri|Sat),\s+
+        (?:0[1-9]|[1-2]?[0-9]|3[01])\s+
+        (?:Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec)\s+
+        (?:19[0-9]{2}|[2-9][0-9]{3})\s+
+        (?:2[0-3]|[0-1][0-9]):(?:[0-5][0-9]):(?:60|[0-5][0-9])\s+
+        GMT
+      $/x.freeze
+
+      # Spec for Retry-After header
+      # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After
+      def delay_from_retry_header(value)
+        value = value.to_s.strip
+
+        delay = case value
+                when RFC2822_DATE_REGEX then DateTime.rfc2822(value).to_time - Time.now.utc
+                when /^\d+$/            then value.to_i
+                else 0
+                end
 
-        raise OutOfRetriesError, error_message(req, res&.status, err)
+        ensure_dealy_in_bounds(delay)
+      end
+
+      def calculate_delay_from_iteration(iteration)
+        ensure_dealy_in_bounds(
+          @delay.call(iteration)
+        )
+      end
+
+      def ensure_dealy_in_bounds(delay)
+        [0, [delay, @maximum_delay].min].max
       end
 
       private
 
-      # Builds out of retries error message.
+      # Builds OutOfRetriesError
       #
       # @param req [HTTP::Request]
-      # @param status [HTTP::Response::Status, nil]
+      # @param status [HTTP::Response, nil]
       # @param exception [Exception, nil]
-      def error_message(req, status, exception)
+      def out_of_retries_error(req, response, exception)
         message = "#{req.verb.to_s.upcase} <#{req.uri}> failed"
 
-        message += " with #{status}" if status
-        message += ":#{exception}"   if exception
+        message += " with #{response.status}" if response
+        message += ":#{exception}" if exception
 
-        message
+        HTTP::OutOfRetriesError.new(message).tap do |ex|
+          ex.cause = exception
+          ex.response = response
+        end
       end
     end
   end
diff --git a/spec/lib/http/retriable/performer_spec.rb b/spec/lib/http/retriable/performer_spec.rb
new file mode 100644
index 00000000..05f85845
--- /dev/null
+++ b/spec/lib/http/retriable/performer_spec.rb
@@ -0,0 +1,331 @@
+# frozen_string_literal: true
+
+# rubocop:disable Lint/HandleExceptions
+RSpec.describe HTTP::Retriable::Performer do
+  let(:client) do
+    HTTP::Client.new
+  end
+  let(:response) do
+    HTTP::Response.new(
+      :status  => 200,
+      :version => "1.1",
+      :headers => {},
+      :body    => "Hello world!",
+      :uri     => "http://example.com/",
+      :request => request
+    )
+  end
+  let(:request) do
+    HTTP::Request.new(
+      :verb => :get,
+      :uri  => "http://example.com"
+    )
+  end
+
+  CustomException = Class.new(StandardError)
+
+  let(:perform_spy) { {:counter => 0} }
+  let(:counter_spy) { perform_spy[:counter] }
+
+  def perform(options = {}, client_arg = client, request_arg = request, &block)
+    # by explicitly overwriting the default delay, we make a much faster test suite
+    default_options = {:delay => 0}
+    options = default_options.merge(options)
+
+    HTTP::Retriable::Performer.
+      new(options).
+      perform(client_arg, request_arg) do
+        perform_spy[:counter] += 1
+        block ? yield : response
+      end
+  end
+
+  def measure_wait
+    t1 = Process.clock_gettime(Process::CLOCK_MONOTONIC)
+    result = yield
+    t2 = Process.clock_gettime(Process::CLOCK_MONOTONIC)
+    [t2 - t1, result]
+  end
+
+  describe "#perform" do
+    describe "expected exception" do
+      it "retries the request" do
+        expect do
+          perform(:exceptions => [CustomException], :tries => 2) do
+            raise CustomException
+          end
+        end.to raise_error HTTP::OutOfRetriesError
+
+        expect(counter_spy).to eq 2
+      end
+    end
+
+    describe "unexpected exception" do
+      it "does not retry the request" do
+        expect do
+          perform(:exceptions => [], :tries => 2) do
+            raise CustomException
+          end
+        end.to raise_error CustomException
+
+        expect(counter_spy).to eq 1
+      end
+    end
+
+    describe "expected status codes" do
+      it "retries the request" do
+        expect do
+          perform(:retry_statuses => [200], :tries => 2)
+        end.to raise_error HTTP::OutOfRetriesError
+
+        expect(counter_spy).to eq 2
+      end
+    end
+
+    describe "unexpected status code" do
+      it "does not retry the request" do
+        expect(
+          perform(:retry_statuses => [], :tries => 2)
+        ).to eq response
+
+        expect(counter_spy).to eq 1
+      end
+    end
+
+    describe "on_retry callback" do
+      it "calls the on_retry callback on each retry with exception" do
+        callback_call_spy = 0
+
+        callback_spy = ->(request, error, response) do
+          expect(request).to eq request
+          expect(error).to be_a HTTP::TimeoutError
+          expect(response).to be_nil
+          callback_call_spy += 1
+        end
+
+        expect do
+          perform(:tries => 3, :on_retry => callback_spy) do
+            raise HTTP::TimeoutError
+          end
+        end.to raise_error HTTP::OutOfRetriesError
+
+        expect(callback_call_spy).to eq 2
+      end
+
+      it "calls the on_retry callback on each retry with response" do
+        callback_call_spy = 0
+
+        callback_spy = ->(request, error, response) do
+          expect(request).to eq request
+          expect(error).to be_nil
+          expect(response).to be response
+          callback_call_spy += 1
+        end
+
+        expect do
+          perform(:retry_statuses => [200], :tries => 3, :on_retry => callback_spy)
+        end.to raise_error HTTP::OutOfRetriesError
+
+        expect(callback_call_spy).to eq 2
+      end
+    end
+
+    describe "delay option" do
+      let(:timing_slack) { 0.05 }
+
+      it "can be a positive number" do
+        time, = measure_wait do
+          begin
+            perform(:delay => 0.1, :tries => 3, :should_retry => ->(*) { true })
+          rescue HTTP::OutOfRetriesError
+          end
+        end
+        expect(time).to be_within(timing_slack).of(0.2)
+      end
+
+      it "can be a proc number" do
+        time, = measure_wait do
+          begin
+            perform(:delay => ->(i) { i / 10.0 }, :tries => 3, :should_retry => ->(*) { true })
+          rescue HTTP::OutOfRetriesError
+          end
+        end
+        expect(time).to be_within(timing_slack).of(0.3)
+      end
+
+      it "receives correct retry number when a proc" do
+        retry_count = 0
+        retry_proc = ->(i) {
+          expect(i).to eq(retry_count).and(be > 0)
+          0
+        }
+        begin
+          perform(:delay => retry_proc, :should_retry => ->(*) { true }) do
+            retry_count += 1
+            response
+          end
+        rescue HTTP::OutOfRetriesError
+        end
+      end
+    end
+
+    describe "should_retry option" do
+      it "decides if the request should be retried" do
+        retry_proc = ->(req, err, res, i) do
+          expect(req).to eq request
+          if res
+            expect(err).to be_nil
+            expect(res).to be response
+          else
+            expect(err).to be_a CustomException
+            expect(res).to be_nil
+          end
+
+          i < 5
+        end
+
+        begin
+          perform(:should_retry => retry_proc) do
+            rand < 0.5 ? response : raise(CustomException)
+          end
+        rescue CustomException
+        end
+
+        expect(counter_spy).to eq 5
+      end
+
+      it "raises the original error if not retryable" do
+        retry_proc = ->(*) { false }
+
+        expect do
+          perform(:should_retry => retry_proc) do
+            raise CustomException
+          end
+        end.to raise_error CustomException
+
+        expect(counter_spy).to eq 1
+      end
+
+      it "raises HTTP::OutOfRetriesError if retryable" do
+        retry_proc = ->(*) { true }
+
+        expect do
+          perform(:should_retry => retry_proc) do
+            raise CustomException
+          end
+        end.to raise_error HTTP::OutOfRetriesError
+
+        expect(counter_spy).to eq 5
+      end
+    end
+  end
+
+  describe "connection closing" do
+    let(:client) { double(:client) }
+
+    it "does not close the connection if we get a propper response" do
+      expect(client).to_not receive(:close)
+      perform
+    end
+
+    it "closes the connection after each raiseed attempt" do
+      expect(client).to receive(:close).exactly(3).times
+      begin
+        perform(:should_retry => ->(*) { true }, :tries => 3)
+      rescue HTTP::OutOfRetriesError
+      end
+    end
+
+    it "closes the connection on an unexpected exception" do
+      expect(client).to receive(:close)
+      begin
+        perform do
+          raise CustomException
+        end
+      rescue CustomException
+      end
+    end
+  end
+
+  describe HTTP::OutOfRetriesError do
+    it "has the original exception as a cause if available" do
+      err = nil
+      begin
+        perform(:exceptions => [CustomException]) do
+          raise CustomException
+        end
+      rescue HTTP::OutOfRetriesError => e
+        err = e
+      end
+      expect(err.cause).to be_a CustomException
+    end
+
+    it "has the last raiseed response as an attribute" do
+      err = nil
+      begin
+        perform(:should_retry => ->(*) { true })
+      rescue HTTP::OutOfRetriesError => e
+        err = e
+      end
+      expect(err.response).to be response
+    end
+  end
+
+  describe "delay calculation" do
+    def call_delay(iterations, **options)
+      HTTP::Retriable::Performer.new(options).calculate_delay_from_iteration(iterations)
+    end
+
+    def call_retry_header(value, **options)
+      HTTP::Retriable::Performer.new(options).delay_from_retry_header(value)
+    end
+
+    it "prevents negative sleep time" do
+      expect(call_delay(20, :delay => -20)).to eq 0
+    end
+
+    it "backs off exponentially" do
+      expect(call_delay(1)).to be_between 0, 1
+      expect(call_delay(2)).to be_between 1, 2
+      expect(call_delay(3)).to be_between 3, 4
+      expect(call_delay(4)).to be_between 7, 8
+      expect(call_delay(5)).to be_between 15, 16
+    end
+
+    it "can have a maximum wait time" do
+      expect(call_delay(1, :max_delay => 5)).to be_between 0, 1
+      expect(call_delay(5, :max_delay => 5)).to eq 5
+    end
+
+    it "respects Retry-After headers as integer" do
+      delay_time = rand(6...2500)
+      header_value = delay_time.to_s
+      expect(call_retry_header(header_value)).to eq delay_time
+      expect(call_retry_header(header_value, :max_delay => 5)).to eq 5
+    end
+
+    it "respects Retry-After headers as rfc2822 timestamp" do
+      delay_time = rand(6...2500)
+      header_value = (Time.now.gmtime + delay_time).to_datetime.rfc2822.sub("+0000", "GMT")
+      expect(call_retry_header(header_value)).to be_within(1).of(delay_time)
+      expect(call_retry_header(header_value, :max_delay => 5)).to eq 5
+    end
+
+    it "respects Retry-After headers as rfc2822 timestamp in the past" do
+      delay_time = rand(6...2500)
+      header_value = (Time.now.gmtime - delay_time).to_datetime.rfc2822.sub("+0000", "GMT")
+      expect(call_retry_header(header_value)).to eq 0
+    end
+
+    it "does not error on invalid Retry-After header" do
+      [ # invalid strings
+        "This is a string with a number 5 in it",
+        "8 Eight is the first digit in this string",
+        "This is a string with a #{Time.now.gmtime.to_datetime.rfc2822} timestamp in it"
+      ].each do |header_value|
+        expect(call_retry_header(header_value)).to eq 0
+      end
+    end
+  end
+end
+# rubocop:enable Lint/HandleExceptions
diff --git a/spec/lib/http_spec.rb b/spec/lib/http_spec.rb
index 56b228d6..8d2f6be8 100644
--- a/spec/lib/http_spec.rb
+++ b/spec/lib/http_spec.rb
@@ -150,6 +150,37 @@
     end
   end
 
+  describe ".retry" do
+    it "ensure endpoint counts retries" do
+      expect(HTTP.get("#{dummy.endpoint}/retry-2").to_s).to eq "retried 1x"
+      expect(HTTP.get("#{dummy.endpoint}/retry-2").to_s).to eq "retried 2x"
+    end
+
+    it "retries the request" do
+      response = HTTP.retriable(:delay => 0).get "#{dummy.endpoint}/retry-2"
+      expect(response.to_s).to eq "retried 2x"
+    end
+
+    it "retries the request and gives us access to the failed requests" do
+      err = nil
+      retry_callback = ->(_, _, res) {
+        expect(res.to_s).to match(/^retried \dx$/)
+      }
+      begin
+        HTTP.retriable(
+          :should_retry => ->(*) { true },
+          :tries        => 3,
+          :delay        => 0,
+          :on_retry     => retry_callback
+        ).get "#{dummy.endpoint}/retry-2"
+      rescue HTTP::Error => e
+        err = e
+      end
+
+      expect(err.response.to_s).to eq "retried 3x"
+    end
+  end
+
   context "posting forms to resources" do
     it "is easy" do
       response = HTTP.post "#{dummy.endpoint}/form", :form => {:example => "testing-form"}
diff --git a/spec/support/dummy_server.rb b/spec/support/dummy_server.rb
index baea6c0a..03863a9c 100644
--- a/spec/support/dummy_server.rb
+++ b/spec/support/dummy_server.rb
@@ -26,7 +26,8 @@ class DummyServer < WEBrick::HTTPServer
 
   def initialize(options = {}) # rubocop:disable Style/OptionHash
     super(options[:ssl] ? SSL_CONFIG : CONFIG)
-    mount("/", Servlet)
+    @memo = {}
+    mount("/", Servlet, @memo)
   end
 
   def endpoint
diff --git a/spec/support/dummy_server/servlet.rb b/spec/support/dummy_server/servlet.rb
index 39eccc82..6e84c54a 100644
--- a/spec/support/dummy_server/servlet.rb
+++ b/spec/support/dummy_server/servlet.rb
@@ -16,6 +16,11 @@ def self.handlers
       @handlers ||= {}
     end
 
+    def initialize(server, memo)
+      super(server)
+      @memo = memo
+    end
+
     %w[get post head].each do |method|
       class_eval <<-RUBY, __FILE__, __LINE__ + 1
         def self.#{method}(path, &block)
@@ -172,5 +177,13 @@ def do_#{method.upcase}(req, res)
                    "#{req.body}-raw"
                  end
     end
+
+    get "/retry-2" do |_req, res|
+      @memo[:attempts] ||= 0
+      @memo[:attempts] += 1
+
+      res.body = "retried #{@memo[:attempts]}x"
+      res.status = @memo[:attempts] == 2 ? 200 : 500
+    end
   end
 end

From 9b9b77dddf882141484888db9fa81e5c3130848e Mon Sep 17 00:00:00 2001
From: Bert Goethals <bert@bertg.be>
Date: Thu, 5 Mar 2020 12:06:52 +0100
Subject: [PATCH 4/8] Reduce cognitive complexity as requested by CodeClimate

---
 lib/http/retriable/performer.rb           | 102 ++++++++++++----------
 spec/lib/http/retriable/performer_spec.rb |  10 +--
 2 files changed, 61 insertions(+), 51 deletions(-)

diff --git a/lib/http/retriable/performer.rb b/lib/http/retriable/performer.rb
index 60c46bda..2f98d83c 100644
--- a/lib/http/retriable/performer.rb
+++ b/lib/http/retriable/performer.rb
@@ -26,8 +26,8 @@ class Performer
       RETRIABLE_STATUSES = [500].freeze
 
       # Default retry delay proc
-      DELAY_PROC = ->(i) {
-        delay = 2**(i - 1) - 1
+      DELAY_PROC = ->(attempt) {
+        delay = 2**(attempt - 1) - 1
         delay_noise = rand
         delay + delay_noise
       }
@@ -41,30 +41,13 @@ class Performer
       # @option opts [#to_f] :max_delay (Float::MAX)
       # @option opts [#call] :should_retry
       def initialize(opts)
-        if opts.key?(:should_retry)
-          @should_retry_proc = opts.fetch(:should_retry)
-        else
-          @exception_classes = opts.fetch(:exceptions, RETRIABLE_ERRORS)
-          @retry_statuses = opts.fetch(:retry_statuses, RETRIABLE_STATUSES)
-          @should_retry_proc = ->(_req, err, res, _i) {
-            if err
-              @exception_classes.any? { |e| err.is_a?(e) }
-            else
-              @retry_statuses.include?(res.status.to_i)
-            end
-          }
-        end
+        @exception_classes = opts.fetch(:exceptions, RETRIABLE_ERRORS)
+        @retry_statuses = opts.fetch(:retry_statuses, RETRIABLE_STATUSES)
         @tries = opts.fetch(:tries, 5).to_i
         @on_retry = opts.fetch(:on_retry, ->(*) {})
         @maximum_delay = opts.fetch(:max_delay, Float::MAX).to_f
-        @delay = begin
-          case delay = opts.fetch(:delay, DELAY_PROC)
-          when Numeric
-            ->(*) { delay }
-          else
-            delay
-          end
-        end
+        @should_retry_proc = opts.fetch(:should_retry, build_retry_proc(@exception_classes, @retry_statuses))
+        @delay = build_delay_proc(opts.fetch(:delay, DELAY_PROC))
       end
 
       # Watches request/response execution.
@@ -76,21 +59,13 @@ def initialize(opts)
       # @see #initialize
       # @api private
       def perform(client, req)
-        1.upto(Float::INFINITY) do |i| # infinite loop with index
-          err, res = nil
-
-          # rubocop:disable Lint/RescueException
-          begin
-            res = yield
-          rescue Exception => e
-            err = e
-          end
-          # rubocop:enable Lint/RescueException
-
-          if @should_retry_proc.call(req, err, res, i)
-            if i < @tries
-              @on_retry.call(req, err, res)
+        1.upto(Float::INFINITY) do |attempt| # infinite loop with index
+          err, res = try_request { yield }
 
+          if @should_retry_proc.call(req, err, res, attempt)
+            begin
+              wait_for_retry_or_raise(req, err, res, attempt)
+            ensure
               # Some servers support Keep-Alive on any response. Thus we should
               # flush response before retry, to avoid state error (when socket
               # has pending response data and we try to write new request).
@@ -98,12 +73,6 @@ def perform(client, req)
               # are going to close client, effectivle closing underlying socket
               # and resetting client's state.
               client.close
-
-              sleep calculate_delay(i, res)
-            else
-              res&.flush
-              client.close
-              raise out_of_retries_error(req, res, err)
             end
           elsif err
             client.close
@@ -157,13 +126,37 @@ def ensure_dealy_in_bounds(delay)
 
       private
 
+      # rubocop:disable Lint/RescueException
+      def try_request
+        err, res = nil
+
+        begin
+          res = yield
+        rescue Exception => e
+          err = e
+        end
+
+        [err, res]
+      end
+      # rubocop:enable Lint/RescueException
+
+      def wait_for_retry_or_raise(req, err, res, attempt)
+        if attempt < @tries
+          @on_retry.call(req, err, res)
+          sleep calculate_delay(attempt, res)
+        else
+          res&.flush
+          raise out_of_retries_error(req, res, err)
+        end
+      end
+
       # Builds OutOfRetriesError
       #
-      # @param req [HTTP::Request]
+      # @param request [HTTP::Request]
       # @param status [HTTP::Response, nil]
       # @param exception [Exception, nil]
-      def out_of_retries_error(req, response, exception)
-        message = "#{req.verb.to_s.upcase} <#{req.uri}> failed"
+      def out_of_retries_error(request, response, exception)
+        message = "#{request.verb.to_s.upcase} <#{request.uri}> failed"
 
         message += " with #{response.status}" if response
         message += ":#{exception}" if exception
@@ -173,6 +166,23 @@ def out_of_retries_error(req, response, exception)
           ex.response = response
         end
       end
+
+      def build_retry_proc(exception_classes, retry_statuses)
+        ->(_req, err, res, _i) {
+          if err
+            exception_classes.any? { |e| err.is_a?(e) }
+          else
+            retry_statuses.include?(res.status.to_i)
+          end
+        }
+      end
+
+      def build_delay_proc(delay)
+        case delay
+        when Numeric then ->(*) { delay }
+        else delay
+        end
+      end
     end
   end
 end
diff --git a/spec/lib/http/retriable/performer_spec.rb b/spec/lib/http/retriable/performer_spec.rb
index 05f85845..4a0a4f3c 100644
--- a/spec/lib/http/retriable/performer_spec.rb
+++ b/spec/lib/http/retriable/performer_spec.rb
@@ -146,7 +146,7 @@ def measure_wait
       it "can be a proc number" do
         time, = measure_wait do
           begin
-            perform(:delay => ->(i) { i / 10.0 }, :tries => 3, :should_retry => ->(*) { true })
+            perform(:delay => ->(attempt) { attempt / 10.0 }, :tries => 3, :should_retry => ->(*) { true })
           rescue HTTP::OutOfRetriesError
           end
         end
@@ -155,8 +155,8 @@ def measure_wait
 
       it "receives correct retry number when a proc" do
         retry_count = 0
-        retry_proc = ->(i) {
-          expect(i).to eq(retry_count).and(be > 0)
+        retry_proc = ->(attempt) {
+          expect(attempt).to eq(retry_count).and(be > 0)
           0
         }
         begin
@@ -171,7 +171,7 @@ def measure_wait
 
     describe "should_retry option" do
       it "decides if the request should be retried" do
-        retry_proc = ->(req, err, res, i) do
+        retry_proc = ->(req, err, res, attempt) do
           expect(req).to eq request
           if res
             expect(err).to be_nil
@@ -181,7 +181,7 @@ def measure_wait
             expect(res).to be_nil
           end
 
-          i < 5
+          attempt < 5
         end
 
         begin

From b527a41b2651818891414c34cda5bcde8a2b9680 Mon Sep 17 00:00:00 2001
From: Bert Goethals <bert@bertg.be>
Date: Mon, 9 Mar 2020 09:34:53 +0100
Subject: [PATCH 5/8] Allow to express retryable status_codes in different ways

---
 lib/http/retriable/performer.rb           | 11 +++++++-
 spec/lib/http/retriable/performer_spec.rb | 34 +++++++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/lib/http/retriable/performer.rb b/lib/http/retriable/performer.rb
index 2f98d83c..b8836f33 100644
--- a/lib/http/retriable/performer.rb
+++ b/lib/http/retriable/performer.rb
@@ -168,11 +168,20 @@ def out_of_retries_error(request, response, exception)
       end
 
       def build_retry_proc(exception_classes, retry_statuses)
+        retry_statuses = [retry_statuses].flatten
+
         ->(_req, err, res, _i) {
           if err
             exception_classes.any? { |e| err.is_a?(e) }
           else
-            retry_statuses.include?(res.status.to_i)
+            response_status = res.status.to_i
+            retry_statuses.any? do |matcher|
+              case matcher
+              when Range then matcher.include?(response_status)
+              when Numeric then matcher == response_status
+              else matcher.call(response_status)
+              end
+            end
           end
         }
       end
diff --git a/spec/lib/http/retriable/performer_spec.rb b/spec/lib/http/retriable/performer_spec.rb
index 4a0a4f3c..169f2d04 100644
--- a/spec/lib/http/retriable/performer_spec.rb
+++ b/spec/lib/http/retriable/performer_spec.rb
@@ -73,6 +73,20 @@ def measure_wait
     end
 
     describe "expected status codes" do
+
+      def response(options = {})
+        HTTP::Response.new(
+          {
+            :status  => 200,
+            :version => "1.1",
+            :headers => {},
+            :body    => "Hello world!",
+            :uri     => "http://example.com/",
+            :request => request
+          }.merge(options)
+        )
+      end
+
       it "retries the request" do
         expect do
           perform(:retry_statuses => [200], :tries => 2)
@@ -80,6 +94,26 @@ def measure_wait
 
         expect(counter_spy).to eq 2
       end
+
+      describe "status codes can be expressed in many ways" do
+
+        [
+          301,
+          [200, 301, 485],
+          250...400,
+          [250...Float::INFINITY],
+          ->(status_code) { status_code == 301  },
+          [->(status_code) { status_code == 301  }]
+        ].each do |retry_statuses|
+          it retry_statuses.to_s do
+            expect do
+              perform(:retry_statuses => retry_statuses, :tries => 2) do
+                response(:status => 301)
+              end
+            end.to raise_error HTTP::OutOfRetriesError
+          end
+        end
+      end
     end
 
     describe "unexpected status code" do

From 4b944d46ad8b8b86be5baa5b9c6e24bc8fd36a34 Mon Sep 17 00:00:00 2001
From: Bert Goethals <bert@bertg.be>
Date: Mon, 9 Mar 2020 09:35:29 +0100
Subject: [PATCH 6/8] By default retry all 500+ status codes

---
 lib/http/retriable/performer.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/http/retriable/performer.rb b/lib/http/retriable/performer.rb
index b8836f33..4f9722de 100644
--- a/lib/http/retriable/performer.rb
+++ b/lib/http/retriable/performer.rb
@@ -23,7 +23,7 @@ class Performer
         IOError
       ].freeze
 
-      RETRIABLE_STATUSES = [500].freeze
+      RETRIABLE_STATUSES = (500...Float::INFINITY).freeze
 
       # Default retry delay proc
       DELAY_PROC = ->(attempt) {

From 080b363f3b0c7cb25833c0269ca9c0e25d8ea72b Mon Sep 17 00:00:00 2001
From: Bert Goethals <bert@bertg.be>
Date: Fri, 13 Mar 2020 10:01:29 +0100
Subject: [PATCH 7/8] Refactor retry logic

* Less procs & proc building overall
* seperate delay calculation into its own class
---
 lib/http/retriable/delay_calculator.rb        |  64 +++++++++++
 lib/http/retriable/performer.rb               | 107 ++++++------------
 .../http/retriable/delay_calculator_spec.rb   |  77 +++++++++++++
 spec/lib/http/retriable/performer_spec.rb     |  67 +----------
 4 files changed, 178 insertions(+), 137 deletions(-)
 create mode 100644 lib/http/retriable/delay_calculator.rb
 create mode 100644 spec/lib/http/retriable/delay_calculator_spec.rb

diff --git a/lib/http/retriable/delay_calculator.rb b/lib/http/retriable/delay_calculator.rb
new file mode 100644
index 00000000..b082293b
--- /dev/null
+++ b/lib/http/retriable/delay_calculator.rb
@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+module HTTP
+  module Retriable
+    # @api private
+    class DelayCalculator
+      def initialize(opts)
+        @max_delay = opts.fetch(:max_delay, Float::MAX).to_f
+        if (delay = opts[:delay]).respond_to?(:call)
+          @delay_proc = opts.fetch(:delay)
+        else
+          @delay = delay
+        end
+      end
+
+      def call(iteration, response)
+        delay = if response && (retry_header = response.headers["Retry-After"])
+                  delay_from_retry_header(retry_header)
+                else
+                  calculate_delay_from_iteration(iteration)
+                end
+
+        ensure_dealy_in_bounds(delay)
+      end
+
+      RFC2822_DATE_REGEX = /^
+        (?:Sun|Mon|Tue|Wed|Thu|Fri|Sat),\s+
+        (?:0[1-9]|[1-2]?[0-9]|3[01])\s+
+        (?:Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec)\s+
+        (?:19[0-9]{2}|[2-9][0-9]{3})\s+
+        (?:2[0-3]|[0-1][0-9]):(?:[0-5][0-9]):(?:60|[0-5][0-9])\s+
+        GMT
+      $/x.freeze
+
+      # Spec for Retry-After header
+      # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After
+      def delay_from_retry_header(value)
+        value = value.to_s.strip
+
+        case value
+        when RFC2822_DATE_REGEX then DateTime.rfc2822(value).to_time - Time.now.utc
+        when /^\d+$/            then value.to_i
+        else 0
+        end
+      end
+
+      def calculate_delay_from_iteration(iteration)
+        if @delay_proc
+          @delay_proc.call(iteration)
+        elsif @delay
+          @delay
+        else
+          delay = 2**(iteration - 1) - 1
+          delay_noise = rand
+          delay + delay_noise
+        end
+      end
+
+      def ensure_dealy_in_bounds(delay)
+        [0, [delay, @max_delay].min].max
+      end
+    end
+  end
+end
diff --git a/lib/http/retriable/performer.rb b/lib/http/retriable/performer.rb
index 4f9722de..685435e5 100644
--- a/lib/http/retriable/performer.rb
+++ b/lib/http/retriable/performer.rb
@@ -3,6 +3,7 @@
 require "date"
 require "http"
 require "http/retriable/errors"
+require "http/retriable/delay_calculator"
 require "openssl"
 
 module HTTP
@@ -25,13 +26,6 @@ class Performer
 
       RETRIABLE_STATUSES = (500...Float::INFINITY).freeze
 
-      # Default retry delay proc
-      DELAY_PROC = ->(attempt) {
-        delay = 2**(attempt - 1) - 1
-        delay_noise = rand
-        delay + delay_noise
-      }
-
       # @param [Hash] opts
       # @option opts [#to_i] :tries (5)
       # @option opts [#call, #to_i] :delay (DELAY_PROC)
@@ -45,9 +39,8 @@ def initialize(opts)
         @retry_statuses = opts.fetch(:retry_statuses, RETRIABLE_STATUSES)
         @tries = opts.fetch(:tries, 5).to_i
         @on_retry = opts.fetch(:on_retry, ->(*) {})
-        @maximum_delay = opts.fetch(:max_delay, Float::MAX).to_f
-        @should_retry_proc = opts.fetch(:should_retry, build_retry_proc(@exception_classes, @retry_statuses))
-        @delay = build_delay_proc(opts.fetch(:delay, DELAY_PROC))
+        @should_retry_proc = opts[:should_retry]
+        @delay_calculator = DelayCalculator.new(opts)
       end
 
       # Watches request/response execution.
@@ -62,7 +55,7 @@ def perform(client, req)
         1.upto(Float::INFINITY) do |attempt| # infinite loop with index
           err, res = try_request { yield }
 
-          if @should_retry_proc.call(req, err, res, attempt)
+          if retry_request?(req, err, res, attempt)
             begin
               wait_for_retry_or_raise(req, err, res, attempt)
             ensure
@@ -84,44 +77,7 @@ def perform(client, req)
       end
 
       def calculate_delay(iteration, response)
-        if response && (retry_header = response.headers["Retry-After"])
-          delay_from_retry_header(retry_header)
-        else
-          calculate_delay_from_iteration(iteration)
-        end
-      end
-
-      RFC2822_DATE_REGEX = /^
-        (?:Sun|Mon|Tue|Wed|Thu|Fri|Sat),\s+
-        (?:0[1-9]|[1-2]?[0-9]|3[01])\s+
-        (?:Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec)\s+
-        (?:19[0-9]{2}|[2-9][0-9]{3})\s+
-        (?:2[0-3]|[0-1][0-9]):(?:[0-5][0-9]):(?:60|[0-5][0-9])\s+
-        GMT
-      $/x.freeze
-
-      # Spec for Retry-After header
-      # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After
-      def delay_from_retry_header(value)
-        value = value.to_s.strip
-
-        delay = case value
-                when RFC2822_DATE_REGEX then DateTime.rfc2822(value).to_time - Time.now.utc
-                when /^\d+$/            then value.to_i
-                else 0
-                end
-
-        ensure_dealy_in_bounds(delay)
-      end
-
-      def calculate_delay_from_iteration(iteration)
-        ensure_dealy_in_bounds(
-          @delay.call(iteration)
-        )
-      end
-
-      def ensure_dealy_in_bounds(delay)
-        [0, [delay, @maximum_delay].min].max
+        @delay_calculator.call(iteration, response)
       end
 
       private
@@ -140,6 +96,33 @@ def try_request
       end
       # rubocop:enable Lint/RescueException
 
+      def retry_request?(req, err, res, attempt)
+        if @should_retry_proc
+          @should_retry_proc.call(req, err, res, attempt)
+        elsif err
+          retry_exception?(err)
+        else
+          retry_response?(res)
+        end
+      end
+
+      def retry_exception?(err)
+        @exception_classes.any? { |e| err.is_a?(e) }
+      end
+
+      def retry_response?(res)
+        response_status = res.status.to_i
+        retry_matchers = [@retry_statuses].flatten
+
+        retry_matchers.any? do |matcher|
+          case matcher
+          when Range then matcher.cover?(response_status)
+          when Numeric then matcher == response_status
+          else matcher.call(response_status)
+          end
+        end
+      end
+
       def wait_for_retry_or_raise(req, err, res, attempt)
         if attempt < @tries
           @on_retry.call(req, err, res)
@@ -166,32 +149,6 @@ def out_of_retries_error(request, response, exception)
           ex.response = response
         end
       end
-
-      def build_retry_proc(exception_classes, retry_statuses)
-        retry_statuses = [retry_statuses].flatten
-
-        ->(_req, err, res, _i) {
-          if err
-            exception_classes.any? { |e| err.is_a?(e) }
-          else
-            response_status = res.status.to_i
-            retry_statuses.any? do |matcher|
-              case matcher
-              when Range then matcher.include?(response_status)
-              when Numeric then matcher == response_status
-              else matcher.call(response_status)
-              end
-            end
-          end
-        }
-      end
-
-      def build_delay_proc(delay)
-        case delay
-        when Numeric then ->(*) { delay }
-        else delay
-        end
-      end
     end
   end
 end
diff --git a/spec/lib/http/retriable/delay_calculator_spec.rb b/spec/lib/http/retriable/delay_calculator_spec.rb
new file mode 100644
index 00000000..f0f703cb
--- /dev/null
+++ b/spec/lib/http/retriable/delay_calculator_spec.rb
@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+RSpec.describe HTTP::Retriable::DelayCalculator do
+  let(:response) do
+    HTTP::Response.new(
+      :status  => 200,
+      :version => "1.1",
+      :headers => {},
+      :body    => "Hello world!",
+      :uri     => "http://example.com/",
+      :request => request
+    )
+  end
+
+  let(:request) do
+    HTTP::Request.new(
+      :verb => :get,
+      :uri  => "http://example.com"
+    )
+  end
+
+  def call_delay(iterations, **options)
+    described_class.new(options).call(iterations, response)
+  end
+
+  def call_retry_header(value, **options)
+    response.headers["Retry-After"] = value
+    described_class.new(options).call(rand(1...100), response)
+  end
+
+  it "prevents negative sleep time" do
+    expect(call_delay(20, :delay => -20)).to eq 0
+  end
+
+  it "backs off exponentially" do
+    expect(call_delay(1)).to be_between 0, 1
+    expect(call_delay(2)).to be_between 1, 2
+    expect(call_delay(3)).to be_between 3, 4
+    expect(call_delay(4)).to be_between 7, 8
+    expect(call_delay(5)).to be_between 15, 16
+  end
+
+  it "can have a maximum wait time" do
+    expect(call_delay(1, :max_delay => 5)).to be_between 0, 1
+    expect(call_delay(5, :max_delay => 5)).to eq 5
+  end
+
+  it "respects Retry-After headers as integer" do
+    delay_time = rand(6...2500)
+    header_value = delay_time.to_s
+    expect(call_retry_header(header_value)).to eq delay_time
+    expect(call_retry_header(header_value, :max_delay => 5)).to eq 5
+  end
+
+  it "respects Retry-After headers as rfc2822 timestamp" do
+    delay_time = rand(6...2500)
+    header_value = (Time.now.gmtime + delay_time).to_datetime.rfc2822.sub("+0000", "GMT")
+    expect(call_retry_header(header_value)).to be_within(1).of(delay_time)
+    expect(call_retry_header(header_value, :max_delay => 5)).to eq 5
+  end
+
+  it "respects Retry-After headers as rfc2822 timestamp in the past" do
+    delay_time = rand(6...2500)
+    header_value = (Time.now.gmtime - delay_time).to_datetime.rfc2822.sub("+0000", "GMT")
+    expect(call_retry_header(header_value)).to eq 0
+  end
+
+  it "does not error on invalid Retry-After header" do
+    [ # invalid strings
+      "This is a string with a number 5 in it",
+      "8 Eight is the first digit in this string",
+      "This is a string with a #{Time.now.gmtime.to_datetime.rfc2822} timestamp in it"
+    ].each do |header_value|
+      expect(call_retry_header(header_value)).to eq 0
+    end
+  end
+end
diff --git a/spec/lib/http/retriable/performer_spec.rb b/spec/lib/http/retriable/performer_spec.rb
index 169f2d04..5642f332 100644
--- a/spec/lib/http/retriable/performer_spec.rb
+++ b/spec/lib/http/retriable/performer_spec.rb
@@ -5,6 +5,7 @@
   let(:client) do
     HTTP::Client.new
   end
+
   let(:response) do
     HTTP::Response.new(
       :status  => 200,
@@ -15,6 +16,7 @@
       :request => request
     )
   end
+
   let(:request) do
     HTTP::Request.new(
       :verb => :get,
@@ -73,8 +75,7 @@ def measure_wait
     end
 
     describe "expected status codes" do
-
-      def response(options = {})
+      def response(**options)
         HTTP::Response.new(
           {
             :status  => 200,
@@ -96,14 +97,13 @@ def response(options = {})
       end
 
       describe "status codes can be expressed in many ways" do
-
         [
           301,
           [200, 301, 485],
           250...400,
           [250...Float::INFINITY],
-          ->(status_code) { status_code == 301  },
-          [->(status_code) { status_code == 301  }]
+          ->(status_code) { status_code == 301 },
+          [->(status_code) { status_code == 301 }]
         ].each do |retry_statuses|
           it retry_statuses.to_s do
             expect do
@@ -304,62 +304,5 @@ def response(options = {})
       expect(err.response).to be response
     end
   end
-
-  describe "delay calculation" do
-    def call_delay(iterations, **options)
-      HTTP::Retriable::Performer.new(options).calculate_delay_from_iteration(iterations)
-    end
-
-    def call_retry_header(value, **options)
-      HTTP::Retriable::Performer.new(options).delay_from_retry_header(value)
-    end
-
-    it "prevents negative sleep time" do
-      expect(call_delay(20, :delay => -20)).to eq 0
-    end
-
-    it "backs off exponentially" do
-      expect(call_delay(1)).to be_between 0, 1
-      expect(call_delay(2)).to be_between 1, 2
-      expect(call_delay(3)).to be_between 3, 4
-      expect(call_delay(4)).to be_between 7, 8
-      expect(call_delay(5)).to be_between 15, 16
-    end
-
-    it "can have a maximum wait time" do
-      expect(call_delay(1, :max_delay => 5)).to be_between 0, 1
-      expect(call_delay(5, :max_delay => 5)).to eq 5
-    end
-
-    it "respects Retry-After headers as integer" do
-      delay_time = rand(6...2500)
-      header_value = delay_time.to_s
-      expect(call_retry_header(header_value)).to eq delay_time
-      expect(call_retry_header(header_value, :max_delay => 5)).to eq 5
-    end
-
-    it "respects Retry-After headers as rfc2822 timestamp" do
-      delay_time = rand(6...2500)
-      header_value = (Time.now.gmtime + delay_time).to_datetime.rfc2822.sub("+0000", "GMT")
-      expect(call_retry_header(header_value)).to be_within(1).of(delay_time)
-      expect(call_retry_header(header_value, :max_delay => 5)).to eq 5
-    end
-
-    it "respects Retry-After headers as rfc2822 timestamp in the past" do
-      delay_time = rand(6...2500)
-      header_value = (Time.now.gmtime - delay_time).to_datetime.rfc2822.sub("+0000", "GMT")
-      expect(call_retry_header(header_value)).to eq 0
-    end
-
-    it "does not error on invalid Retry-After header" do
-      [ # invalid strings
-        "This is a string with a number 5 in it",
-        "8 Eight is the first digit in this string",
-        "This is a string with a #{Time.now.gmtime.to_datetime.rfc2822} timestamp in it"
-      ].each do |header_value|
-        expect(call_retry_header(header_value)).to eq 0
-      end
-    end
-  end
 end
 # rubocop:enable Lint/HandleExceptions

From aefebf6c72a9ae5c84b6535e9c944cf5d28ef091 Mon Sep 17 00:00:00 2001
From: Bert Goethals <bert@bertg.be>
Date: Fri, 13 Mar 2020 10:03:58 +0100
Subject: [PATCH 8/8] Don't resque any response by default

---
 lib/http/retriable/performer.rb | 8 ++++----
 spec/lib/http_spec.rb           | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/http/retriable/performer.rb b/lib/http/retriable/performer.rb
index 685435e5..de630dd9 100644
--- a/lib/http/retriable/performer.rb
+++ b/lib/http/retriable/performer.rb
@@ -24,19 +24,17 @@ class Performer
         IOError
       ].freeze
 
-      RETRIABLE_STATUSES = (500...Float::INFINITY).freeze
-
       # @param [Hash] opts
       # @option opts [#to_i] :tries (5)
       # @option opts [#call, #to_i] :delay (DELAY_PROC)
       # @option opts [Array(Exception)] :exceptions (RETRIABLE_ERRORS)
-      # @option opts [Array(#to_i)] :retry_statuses ([500])
+      # @option opts [Array(#to_i)] :retry_statuses
       # @option opts [#call] :on_retry
       # @option opts [#to_f] :max_delay (Float::MAX)
       # @option opts [#call] :should_retry
       def initialize(opts)
         @exception_classes = opts.fetch(:exceptions, RETRIABLE_ERRORS)
-        @retry_statuses = opts.fetch(:retry_statuses, RETRIABLE_STATUSES)
+        @retry_statuses = opts[:retry_statuses]
         @tries = opts.fetch(:tries, 5).to_i
         @on_retry = opts.fetch(:on_retry, ->(*) {})
         @should_retry_proc = opts[:should_retry]
@@ -111,6 +109,8 @@ def retry_exception?(err)
       end
 
       def retry_response?(res)
+        return false unless @retry_statuses
+
         response_status = res.status.to_i
         retry_matchers = [@retry_statuses].flatten
 
diff --git a/spec/lib/http_spec.rb b/spec/lib/http_spec.rb
index 8d2f6be8..fe9cc9ea 100644
--- a/spec/lib/http_spec.rb
+++ b/spec/lib/http_spec.rb
@@ -157,7 +157,7 @@
     end
 
     it "retries the request" do
-      response = HTTP.retriable(:delay => 0).get "#{dummy.endpoint}/retry-2"
+      response = HTTP.retriable(:delay => 0, :retry_statuses => 500...600).get "#{dummy.endpoint}/retry-2"
       expect(response.to_s).to eq "retried 2x"
     end