From a93317be3210bac13040f6fb45d2a70be2bcfdef Mon Sep 17 00:00:00 2001 From: Alexey Zapparov Date: Wed, 26 Feb 2020 18:26:25 +0100 Subject: [PATCH 01/11] 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 acd534f6..f71be18e 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..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 91c216c7651e4c359bdce94bad003b70448ac699 Mon Sep 17 00:00:00 2001 From: Bert Goethals Date: Tue, 3 Mar 2020 12:24:30 +0100 Subject: [PATCH 02/11] 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 47e373e962d75386fd4d11a3aa29e41439499dc3 Mon Sep 17 00:00:00 2001 From: Bert Goethals Date: Tue, 3 Mar 2020 12:24:42 +0100 Subject: [PATCH 03/11] 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 f71be18e..22b0a3ed 100644 --- a/lib/http/chainable.rb +++ b/lib/http/chainable.rb @@ -265,7 +265,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 05fd6efd..2a961cf0 100644 --- a/spec/lib/http_spec.rb +++ b/spec/lib/http_spec.rb @@ -152,6 +152,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 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 From ab0df12976d6c52f04726d416fd96d945e0508a6 Mon Sep 17 00:00:00 2001 From: Bert Goethals Date: Thu, 5 Mar 2020 12:06:52 +0100 Subject: [PATCH 04/11] 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 09ab09266b29e5bc8f40b56488b43e463e5aa04b Mon Sep 17 00:00:00 2001 From: Bert Goethals Date: Mon, 9 Mar 2020 09:34:53 +0100 Subject: [PATCH 05/11] 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 2eee97b0cae4713efaaa31ef85095a865a2c8caa Mon Sep 17 00:00:00 2001 From: Bert Goethals Date: Mon, 9 Mar 2020 09:35:29 +0100 Subject: [PATCH 06/11] 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 7c100396309d62fddab5eadb1aa9c634a497bc20 Mon Sep 17 00:00:00 2001 From: Bert Goethals Date: Fri, 13 Mar 2020 10:01:29 +0100 Subject: [PATCH 07/11] 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 824ed49f658c8f7aefb32244c4c2697deee351d5 Mon Sep 17 00:00:00 2001 From: Bert Goethals Date: Fri, 13 Mar 2020 10:03:58 +0100 Subject: [PATCH 08/11] 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 2a961cf0..a6d69f5f 100644 --- a/spec/lib/http_spec.rb +++ b/spec/lib/http_spec.rb @@ -159,7 +159,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 From b9a029a6b5d938d58401c760d6c8060b3d9f69ee Mon Sep 17 00:00:00 2001 From: Tomify Date: Fri, 5 Jan 2024 12:52:44 -0500 Subject: [PATCH 09/11] Removed hashrockets --- lib/http/chainable.rb | 6 +- .../http/retriable/delay_calculator_spec.rb | 26 ++++---- spec/lib/http/retriable/performer_spec.rb | 66 +++++++++---------- spec/lib/http_spec.rb | 10 +-- 4 files changed, 54 insertions(+), 54 deletions(-) diff --git a/lib/http/chainable.rb b/lib/http/chainable.rb index 22b0a3ed..0fd144fd 100644 --- a/lib/http/chainable.rb +++ b/lib/http/chainable.rb @@ -256,13 +256,13 @@ def use(*features) # HTTP.retriable.get(url) # # # Retry max 3 times with randomly growing delay between retries - # HTTP.retriable(:times => 3).get(url) + # 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) + # 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) + # HTTP.retriable(times: 3, delay: proc { |i| 1 + i*i }).get(url) # # @param (see Performer#initialize) def retriable(**options) diff --git a/spec/lib/http/retriable/delay_calculator_spec.rb b/spec/lib/http/retriable/delay_calculator_spec.rb index f0f703cb..ef256b09 100644 --- a/spec/lib/http/retriable/delay_calculator_spec.rb +++ b/spec/lib/http/retriable/delay_calculator_spec.rb @@ -3,19 +3,19 @@ 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 + 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" + verb: :get, + uri: "http://example.com" ) end @@ -29,7 +29,7 @@ def call_retry_header(value, **options) end it "prevents negative sleep time" do - expect(call_delay(20, :delay => -20)).to eq 0 + expect(call_delay(20, delay: -20)).to eq 0 end it "backs off exponentially" do @@ -41,22 +41,22 @@ def call_retry_header(value, **options) 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 + 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 + 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 + 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 diff --git a/spec/lib/http/retriable/performer_spec.rb b/spec/lib/http/retriable/performer_spec.rb index 5642f332..cfa42881 100644 --- a/spec/lib/http/retriable/performer_spec.rb +++ b/spec/lib/http/retriable/performer_spec.rb @@ -8,30 +8,30 @@ let(:response) do HTTP::Response.new( - :status => 200, - :version => "1.1", - :headers => {}, - :body => "Hello world!", - :uri => "http://example.com/", - :request => request + 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" + verb: :get, + uri: "http://example.com" ) end CustomException = Class.new(StandardError) - let(:perform_spy) { {:counter => 0} } + 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} + default_options = {delay: 0} options = default_options.merge(options) HTTP::Retriable::Performer. @@ -53,7 +53,7 @@ def measure_wait describe "expected exception" do it "retries the request" do expect do - perform(:exceptions => [CustomException], :tries => 2) do + perform(exceptions: [CustomException], tries: 2) do raise CustomException end end.to raise_error HTTP::OutOfRetriesError @@ -65,7 +65,7 @@ def measure_wait describe "unexpected exception" do it "does not retry the request" do expect do - perform(:exceptions => [], :tries => 2) do + perform(exceptions: [], tries: 2) do raise CustomException end end.to raise_error CustomException @@ -78,19 +78,19 @@ def measure_wait def response(**options) HTTP::Response.new( { - :status => 200, - :version => "1.1", - :headers => {}, - :body => "Hello world!", - :uri => "http://example.com/", - :request => request + 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) + perform(retry_statuses: [200], tries: 2) end.to raise_error HTTP::OutOfRetriesError expect(counter_spy).to eq 2 @@ -107,8 +107,8 @@ def response(**options) ].each do |retry_statuses| it retry_statuses.to_s do expect do - perform(:retry_statuses => retry_statuses, :tries => 2) do - response(:status => 301) + perform(retry_statuses: retry_statuses, tries: 2) do + response(status: 301) end end.to raise_error HTTP::OutOfRetriesError end @@ -119,7 +119,7 @@ def response(**options) describe "unexpected status code" do it "does not retry the request" do expect( - perform(:retry_statuses => [], :tries => 2) + perform(retry_statuses: [], tries: 2) ).to eq response expect(counter_spy).to eq 1 @@ -138,7 +138,7 @@ def response(**options) end expect do - perform(:tries => 3, :on_retry => callback_spy) do + perform(tries: 3, on_retry: callback_spy) do raise HTTP::TimeoutError end end.to raise_error HTTP::OutOfRetriesError @@ -157,7 +157,7 @@ def response(**options) end expect do - perform(:retry_statuses => [200], :tries => 3, :on_retry => callback_spy) + perform(retry_statuses: [200], tries: 3, on_retry: callback_spy) end.to raise_error HTTP::OutOfRetriesError expect(callback_call_spy).to eq 2 @@ -170,7 +170,7 @@ def response(**options) it "can be a positive number" do time, = measure_wait do begin - perform(:delay => 0.1, :tries => 3, :should_retry => ->(*) { true }) + perform(delay: 0.1, tries: 3, should_retry: ->(*) { true }) rescue HTTP::OutOfRetriesError end end @@ -180,7 +180,7 @@ def response(**options) it "can be a proc number" do time, = measure_wait do begin - perform(:delay => ->(attempt) { attempt / 10.0 }, :tries => 3, :should_retry => ->(*) { true }) + perform(delay: ->(attempt) { attempt / 10.0 }, tries: 3, should_retry: ->(*) { true }) rescue HTTP::OutOfRetriesError end end @@ -194,7 +194,7 @@ def response(**options) 0 } begin - perform(:delay => retry_proc, :should_retry => ->(*) { true }) do + perform(delay: retry_proc, should_retry: ->(*) { true }) do retry_count += 1 response end @@ -219,7 +219,7 @@ def response(**options) end begin - perform(:should_retry => retry_proc) do + perform(should_retry: retry_proc) do rand < 0.5 ? response : raise(CustomException) end rescue CustomException @@ -232,7 +232,7 @@ def response(**options) retry_proc = ->(*) { false } expect do - perform(:should_retry => retry_proc) do + perform(should_retry: retry_proc) do raise CustomException end end.to raise_error CustomException @@ -244,7 +244,7 @@ def response(**options) retry_proc = ->(*) { true } expect do - perform(:should_retry => retry_proc) do + perform(should_retry: retry_proc) do raise CustomException end end.to raise_error HTTP::OutOfRetriesError @@ -265,7 +265,7 @@ def response(**options) it "closes the connection after each raiseed attempt" do expect(client).to receive(:close).exactly(3).times begin - perform(:should_retry => ->(*) { true }, :tries => 3) + perform(should_retry: ->(*) { true }, tries: 3) rescue HTTP::OutOfRetriesError end end @@ -285,7 +285,7 @@ def response(**options) it "has the original exception as a cause if available" do err = nil begin - perform(:exceptions => [CustomException]) do + perform(exceptions: [CustomException]) do raise CustomException end rescue HTTP::OutOfRetriesError => e @@ -297,7 +297,7 @@ def response(**options) it "has the last raiseed response as an attribute" do err = nil begin - perform(:should_retry => ->(*) { true }) + perform(should_retry: ->(*) { true }) rescue HTTP::OutOfRetriesError => e err = e end diff --git a/spec/lib/http_spec.rb b/spec/lib/http_spec.rb index a6d69f5f..58e93102 100644 --- a/spec/lib/http_spec.rb +++ b/spec/lib/http_spec.rb @@ -159,7 +159,7 @@ end it "retries the request" do - response = HTTP.retriable(:delay => 0, :retry_statuses => 500...600).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 @@ -170,10 +170,10 @@ } begin HTTP.retriable( - :should_retry => ->(*) { true }, - :tries => 3, - :delay => 0, - :on_retry => retry_callback + should_retry: ->(*) { true }, + tries: 3, + delay: 0, + on_retry: retry_callback ).get "#{dummy.endpoint}/retry-2" rescue HTTP::Error => e err = e From 1f4a53a76d4adc0a60607ac9819794de824164d5 Mon Sep 17 00:00:00 2001 From: Tomify Date: Fri, 5 Jan 2024 16:04:18 -0500 Subject: [PATCH 10/11] Fixed formatting --- .rubocop_todo.yml | 39 +++++++++------- lib/http/retriable/delay_calculator.rb | 6 +-- lib/http/retriable/errors.rb | 4 +- lib/http/retriable/performer.rb | 4 +- spec/lib/http/retriable/performer_spec.rb | 54 +++++++++++------------ spec/lib/http_spec.rb | 4 +- 6 files changed, 56 insertions(+), 55 deletions(-) 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/retriable/delay_calculator.rb b/lib/http/retriable/delay_calculator.rb index b082293b..922f5421 100644 --- a/lib/http/retriable/delay_calculator.rb +++ b/lib/http/retriable/delay_calculator.rb @@ -30,7 +30,7 @@ def call(iteration, response) (?: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 + $/x # Spec for Retry-After header # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After @@ -50,14 +50,14 @@ def calculate_delay_from_iteration(iteration) elsif @delay @delay else - delay = 2**(iteration - 1) - 1 + 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 + delay.clamp(0, @max_delay) end end end diff --git a/lib/http/retriable/errors.rb b/lib/http/retriable/errors.rb index 9da048a2..f96b364b 100644 --- a/lib/http/retriable/errors.rb +++ b/lib/http/retriable/errors.rb @@ -5,9 +5,7 @@ module HTTP class OutOfRetriesError < Error attr_accessor :response - def cause=(exception) - @cause = exception - end + attr_writer :cause def cause @cause || super diff --git a/lib/http/retriable/performer.rb b/lib/http/retriable/performer.rb index de630dd9..b25c81d1 100644 --- a/lib/http/retriable/performer.rb +++ b/lib/http/retriable/performer.rb @@ -49,9 +49,9 @@ def initialize(opts) # # @see #initialize # @api private - def perform(client, req) + def perform(client, req, &block) 1.upto(Float::INFINITY) do |attempt| # infinite loop with index - err, res = try_request { yield } + err, res = try_request(&block) if retry_request?(req, err, res, attempt) begin diff --git a/spec/lib/http/retriable/performer_spec.rb b/spec/lib/http/retriable/performer_spec.rb index cfa42881..f729e0e2 100644 --- a/spec/lib/http/retriable/performer_spec.rb +++ b/spec/lib/http/retriable/performer_spec.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# rubocop:disable Lint/HandleExceptions RSpec.describe HTTP::Retriable::Performer do let(:client) do HTTP::Client.new @@ -24,19 +23,21 @@ ) end - CustomException = Class.new(StandardError) - - let(:perform_spy) { {counter: 0} } + 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} + default_options = { delay: 0 } options = default_options.merge(options) - HTTP::Retriable::Performer. - new(options). - perform(client_arg, request_arg) do + HTTP::Retriable::Performer + .new(options) + .perform(client_arg, request_arg) do perform_spy[:counter] += 1 block ? yield : response end @@ -130,10 +131,10 @@ def response(**options) 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 + callback_spy = proc do |callback_request, error, callback_response| + expect(callback_request).to eq request expect(error).to be_a HTTP::TimeoutError - expect(response).to be_nil + expect(callback_response).to be_nil callback_call_spy += 1 end @@ -149,10 +150,10 @@ def response(**options) 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 + callback_spy = proc do |callback_request, error, callback_response| + expect(callback_request).to eq request expect(error).to be_nil - expect(response).to be response + expect(callback_response).to be response callback_call_spy += 1 end @@ -169,30 +170,26 @@ def response(**options) 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 + 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 - begin - perform(delay: ->(attempt) { attempt / 10.0 }, tries: 3, should_retry: ->(*) { true }) - rescue HTTP::OutOfRetriesError - end + 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 = ->(attempt) { + 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 @@ -205,7 +202,7 @@ def response(**options) describe "should_retry option" do it "decides if the request should be retried" do - retry_proc = ->(req, err, res, attempt) do + retry_proc = proc do |req, err, res, attempt| expect(req).to eq request if res expect(err).to be_nil @@ -258,7 +255,7 @@ def response(**options) let(:client) { double(:client) } it "does not close the connection if we get a propper response" do - expect(client).to_not receive(:close) + expect(client).not_to receive(:close) perform end @@ -288,7 +285,7 @@ def response(**options) perform(exceptions: [CustomException]) do raise CustomException end - rescue HTTP::OutOfRetriesError => e + rescue described_class => e err = e end expect(err.cause).to be_a CustomException @@ -298,11 +295,10 @@ def response(**options) err = nil begin perform(should_retry: ->(*) { true }) - rescue HTTP::OutOfRetriesError => e + rescue described_class => e err = e end expect(err.response).to be response end end end -# rubocop:enable Lint/HandleExceptions diff --git a/spec/lib/http_spec.rb b/spec/lib/http_spec.rb index 58e93102..614ea9a7 100644 --- a/spec/lib/http_spec.rb +++ b/spec/lib/http_spec.rb @@ -165,9 +165,7 @@ 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$/) - } + retry_callback = ->(_, _, res) { expect(res.to_s).to match(/^retried \dx$/) } begin HTTP.retriable( should_retry: ->(*) { true }, From c42dd147ae8c218b85676f76ffebe3a815a4e73b Mon Sep 17 00:00:00 2001 From: Tomify Date: Fri, 5 Jan 2024 16:09:45 -0500 Subject: [PATCH 11/11] Fixed uri warning --- spec/lib/http/retriable/delay_calculator_spec.rb | 10 +--------- spec/lib/http/retriable/performer_spec.rb | 2 -- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/spec/lib/http/retriable/delay_calculator_spec.rb b/spec/lib/http/retriable/delay_calculator_spec.rb index ef256b09..555f8d46 100644 --- a/spec/lib/http/retriable/delay_calculator_spec.rb +++ b/spec/lib/http/retriable/delay_calculator_spec.rb @@ -7,15 +7,7 @@ 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" + request: HTTP::Request.new(verb: :get, uri: "http://example.com") ) end diff --git a/spec/lib/http/retriable/performer_spec.rb b/spec/lib/http/retriable/performer_spec.rb index f729e0e2..60c0e12c 100644 --- a/spec/lib/http/retriable/performer_spec.rb +++ b/spec/lib/http/retriable/performer_spec.rb @@ -11,7 +11,6 @@ version: "1.1", headers: {}, body: "Hello world!", - uri: "http://example.com/", request: request ) end @@ -83,7 +82,6 @@ def response(**options) version: "1.1", headers: {}, body: "Hello world!", - uri: "http://example.com/", request: request }.merge(options) )