From d6313d771f5a81dbeac49bf7196ebf85f0a5afdb Mon Sep 17 00:00:00 2001 From: Tom Prats Date: Mon, 22 Jul 2024 15:16:43 -0400 Subject: [PATCH] Add .retriable feature to Http - Rebased (#775) * 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 --------- Co-authored-by: Alexey Zapparov Co-authored-by: Bert Goethals --- .rubocop_todo.yml | 39 ++- lib/http.rb | 1 + lib/http/chainable.rb | 22 ++ lib/http/retriable/client.rb | 37 +++ lib/http/retriable/delay_calculator.rb | 64 ++++ lib/http/retriable/errors.rb | 14 + lib/http/retriable/performer.rb | 154 +++++++++ .../http/retriable/delay_calculator_spec.rb | 69 ++++ spec/lib/http/retriable/performer_spec.rb | 302 ++++++++++++++++++ spec/lib/http_spec.rb | 29 ++ spec/support/dummy_server.rb | 3 +- spec/support/dummy_server/servlet.rb | 13 + 12 files changed, 731 insertions(+), 16 deletions(-) create mode 100644 lib/http/retriable/client.rb create mode 100644 lib/http/retriable/delay_calculator.rb create mode 100644 lib/http/retriable/errors.rb create mode 100644 lib/http/retriable/performer.rb create mode 100644 spec/lib/http/retriable/delay_calculator_spec.rb create mode 100644 spec/lib/http/retriable/performer_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index d6f36a5d..79be39a4 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config --auto-gen-only-exclude --exclude-limit 100` -# on 2023-10-18 14:33:19 UTC using RuboCop version 1.57.1. +# on 2024-01-05 21:03:44 UTC using RuboCop version 1.57.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -54,17 +54,20 @@ Layout/SpaceInsideHashLiteralBraces: - 'spec/support/http_handling_shared.rb' - 'spec/support/ssl_helper.rb' -# Offense count: 6 +# Offense count: 3 # Configuration parameters: AllowedParentClasses. Lint/MissingSuper: Exclude: - 'lib/http/features/auto_deflate.rb' - - 'lib/http/features/instrumentation.rb' - - 'lib/http/features/logging.rb' - - 'lib/http/features/normalize_uri.rb' - 'spec/lib/http/client_spec.rb' - 'spec/lib/http/features/instrumentation_spec.rb' +# Offense count: 6 +# Configuration parameters: AllowComments, AllowNil. +Lint/SuppressedException: + Exclude: + - 'spec/lib/http/retriable/performer_spec.rb' + # Offense count: 8 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes, Max. Metrics/AbcSize: @@ -93,7 +96,7 @@ Metrics/CyclomaticComplexity: - 'lib/http/chainable.rb' - 'lib/http/client.rb' -# Offense count: 15 +# Offense count: 16 # Configuration parameters: CountComments, Max, CountAsOne, AllowedMethods, AllowedPatterns. Metrics/MethodLength: Exclude: @@ -106,6 +109,7 @@ Metrics/MethodLength: - 'lib/http/redirector.rb' - 'lib/http/response.rb' - 'lib/http/response/body.rb' + - 'lib/http/retriable/performer.rb' - 'lib/http/timeout/global.rb' # Offense count: 1 @@ -183,7 +187,7 @@ RSpec/DescribeMethod: - 'spec/lib/http/options/new_spec.rb' - 'spec/lib/http/options/proxy_spec.rb' -# Offense count: 132 +# Offense count: 136 # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: SkipBlocks, EnforcedStyle. # SupportedStyles: described_class, explicit @@ -208,7 +212,7 @@ RSpec/DescribedClass: - 'spec/lib/http/uri/normalizer_spec.rb' - 'spec/lib/http_spec.rb' -# Offense count: 41 +# Offense count: 49 # Configuration parameters: Max, CountAsOne. RSpec/ExampleLength: Exclude: @@ -219,6 +223,7 @@ RSpec/ExampleLength: - 'spec/lib/http/redirector_spec.rb' - 'spec/lib/http/request/body_spec.rb' - 'spec/lib/http/response/body_spec.rb' + - 'spec/lib/http/retriable/performer_spec.rb' - 'spec/lib/http/uri_spec.rb' - 'spec/lib/http_spec.rb' - 'spec/support/http_handling_shared.rb' @@ -256,13 +261,13 @@ RSpec/InstanceVariable: Exclude: - 'spec/lib/http/redirector_spec.rb' -# Offense count: 40 +# Offense count: 43 # Configuration parameters: . # SupportedStyles: have_received, receive RSpec/MessageSpies: EnforcedStyle: receive -# Offense count: 59 +# Offense count: 74 # Configuration parameters: Max. RSpec/MultipleExpectations: Exclude: @@ -280,15 +285,18 @@ RSpec/MultipleExpectations: - 'spec/lib/http/redirector_spec.rb' - 'spec/lib/http/response/body_spec.rb' - 'spec/lib/http/response/parser_spec.rb' + - 'spec/lib/http/retriable/delay_calculator_spec.rb' + - 'spec/lib/http/retriable/performer_spec.rb' - 'spec/lib/http/uri_spec.rb' - 'spec/lib/http_spec.rb' - 'spec/support/http_handling_shared.rb' -# Offense count: 8 +# Offense count: 9 # Configuration parameters: AllowSubject, Max. RSpec/MultipleMemoizedHelpers: Exclude: - 'spec/lib/http/response_spec.rb' + - 'spec/lib/http/retriable/performer_spec.rb' - 'spec/lib/http/uri_spec.rb' # Offense count: 58 @@ -305,13 +313,14 @@ RSpec/NamedSubject: - 'spec/lib/http/response/status_spec.rb' - 'spec/lib/http/response_spec.rb' -# Offense count: 15 +# Offense count: 16 # Configuration parameters: Max, AllowedGroups. RSpec/NestedGroups: Exclude: - 'spec/lib/http/client_spec.rb' - 'spec/lib/http/redirector_spec.rb' - 'spec/lib/http/request_spec.rb' + - 'spec/lib/http/retriable/performer_spec.rb' - 'spec/lib/http_spec.rb' # Offense count: 2 @@ -355,7 +364,7 @@ RSpec/VariableName: Exclude: - 'spec/lib/http/headers_spec.rb' -# Offense count: 11 +# Offense count: 12 # Configuration parameters: IgnoreNameless, IgnoreSymbolicNames. RSpec/VerifiedDoubles: Exclude: @@ -364,6 +373,7 @@ RSpec/VerifiedDoubles: - 'spec/lib/http/response/body_spec.rb' - 'spec/lib/http/response/status_spec.rb' - 'spec/lib/http/response_spec.rb' + - 'spec/lib/http/retriable/performer_spec.rb' - 'spec/lib/http_spec.rb' # Offense count: 1 @@ -404,14 +414,13 @@ Style/Encoding: - 'spec/lib/http_spec.rb' - 'spec/support/dummy_server/servlet.rb' -# Offense count: 17 +# Offense count: 16 # Configuration parameters: SuspiciousParamNames, Allowlist. # SuspiciousParamNames: options, opts, args, params, parameters Style/OptionHash: Exclude: - 'lib/http/chainable.rb' - 'lib/http/client.rb' - - 'lib/http/feature.rb' - 'lib/http/options.rb' - 'lib/http/redirector.rb' - 'lib/http/timeout/null.rb' 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/chainable.rb b/lib/http/chainable.rb index acd534f6..0fd144fd 100644 --- a/lib/http/chainable.rb +++ b/lib/http/chainable.rb @@ -247,6 +247,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..7edfd3a5 --- /dev/null +++ b/lib/http/retriable/client.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require "http/retriable/performer" + +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/delay_calculator.rb b/lib/http/retriable/delay_calculator.rb new file mode 100644 index 00000000..922f5421 --- /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 + + # 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) + delay.clamp(0, @max_delay) + end + end + end +end diff --git a/lib/http/retriable/errors.rb b/lib/http/retriable/errors.rb new file mode 100644 index 00000000..f96b364b --- /dev/null +++ b/lib/http/retriable/errors.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module HTTP + # Retriable performance ran out of attempts + class OutOfRetriesError < Error + attr_accessor :response + + attr_writer :cause + + def cause + @cause || super + end + end +end diff --git a/lib/http/retriable/performer.rb b/lib/http/retriable/performer.rb new file mode 100644 index 00000000..b25c81d1 --- /dev/null +++ b/lib/http/retriable/performer.rb @@ -0,0 +1,154 @@ +# frozen_string_literal: true + +require "date" +require "http" +require "http/retriable/errors" +require "http/retriable/delay_calculator" +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 + + # @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 + # @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[:retry_statuses] + @tries = opts.fetch(:tries, 5).to_i + @on_retry = opts.fetch(:on_retry, ->(*) {}) + @should_retry_proc = opts[:should_retry] + @delay_calculator = DelayCalculator.new(opts) + 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, &block) + 1.upto(Float::INFINITY) do |attempt| # infinite loop with index + err, res = try_request(&block) + + if retry_request?(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). + # 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 + end + elsif err + client.close + raise err + elsif res + return res + end + end + end + + def calculate_delay(iteration, response) + @delay_calculator.call(iteration, response) + end + + 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 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) + return false unless @retry_statuses + + 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) + sleep calculate_delay(attempt, res) + else + res&.flush + raise out_of_retries_error(req, res, err) + end + end + + # Builds OutOfRetriesError + # + # @param request [HTTP::Request] + # @param status [HTTP::Response, nil] + # @param exception [Exception, nil] + 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 + + HTTP::OutOfRetriesError.new(message).tap do |ex| + ex.cause = exception + ex.response = response + 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..555f8d46 --- /dev/null +++ b/spec/lib/http/retriable/delay_calculator_spec.rb @@ -0,0 +1,69 @@ +# 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!", + request: 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 new file mode 100644 index 00000000..60c0e12c --- /dev/null +++ b/spec/lib/http/retriable/performer_spec.rb @@ -0,0 +1,302 @@ +# frozen_string_literal: true + +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!", + request: request + ) + end + + let(:request) do + HTTP::Request.new( + verb: :get, + uri: "http://example.com" + ) + end + + let(:perform_spy) { { counter: 0 } } + let(:counter_spy) { perform_spy[:counter] } + + before do + stub_const("CustomException", Class.new(StandardError)) + end + + 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 + def response(**options) + HTTP::Response.new( + { + status: 200, + version: "1.1", + headers: {}, + body: "Hello world!", + request: request + }.merge(options) + ) + end + + 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 + + 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 + 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 = proc do |callback_request, error, callback_response| + expect(callback_request).to eq request + expect(error).to be_a HTTP::TimeoutError + expect(callback_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 = proc do |callback_request, error, callback_response| + expect(callback_request).to eq request + expect(error).to be_nil + expect(callback_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 + perform(delay: 0.1, tries: 3, should_retry: ->(*) { true }) + rescue HTTP::OutOfRetriesError + end + expect(time).to be_within(timing_slack).of(0.2) + end + + it "can be a proc number" do + time, = measure_wait do + perform(delay: ->(attempt) { attempt / 10.0 }, tries: 3, should_retry: ->(*) { true }) + rescue HTTP::OutOfRetriesError + 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 = proc do |attempt| + expect(attempt).to eq(retry_count).and(be > 0) + 0 + end + 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 = proc do |req, err, res, attempt| + 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 + + attempt < 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).not_to 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 described_class => 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 described_class => e + err = e + end + expect(err.response).to be response + end + end +end diff --git a/spec/lib/http_spec.rb b/spec/lib/http_spec.rb index 05fd6efd..614ea9a7 100644 --- a/spec/lib/http_spec.rb +++ b/spec/lib/http_spec.rb @@ -152,6 +152,35 @@ 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, retry_statuses: 500...600).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 58ed8648..fd4c7775 100644 --- a/spec/support/dummy_server.rb +++ b/spec/support/dummy_server.rb @@ -26,7 +26,8 @@ class DummyServer < WEBrick::HTTPServer def initialize(options = {}) 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 81e32ab4..075f8ad3 100644 --- a/spec/support/dummy_server/servlet.rb +++ b/spec/support/dummy_server/servlet.rb @@ -18,6 +18,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) @@ -186,5 +191,13 @@ def do_#{method.upcase}(req, res) res["Content-Encoding"] = "deflate" 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