From a7343f86cc1723aa61a7e9721f4020617caea78d Mon Sep 17 00:00:00 2001
From: Beth Skurrie <beth@bethesque.com>
Date: Fri, 26 Mar 2021 09:33:19 +1100
Subject: [PATCH] feat: automatically retry at the http client level for 50x
 responses

---
 lib/pact_broker/client/hal/http_client.rb     | 34 ++++++++-
 .../client/hal/http_client_spec.rb            | 71 +++++++++++++++++--
 2 files changed, 97 insertions(+), 8 deletions(-)

diff --git a/lib/pact_broker/client/hal/http_client.rb b/lib/pact_broker/client/hal/http_client.rb
index 566670ac..8d8922b0 100644
--- a/lib/pact_broker/client/hal/http_client.rb
+++ b/lib/pact_broker/client/hal/http_client.rb
@@ -7,6 +7,7 @@ module PactBroker
   module Client
     module Hal
       class HttpClient
+        RETRYABLE_ERRORS = [Errno::ECONNREFUSED, Errno::ECONNRESET, Errno::ETIMEDOUT, Errno::EHOSTUNREACH, Net::ReadTimeout]
         attr_accessor :username, :password, :verbose, :token
 
         def initialize options
@@ -53,7 +54,7 @@ def create_request uri, http_method, body = nil, headers = {}
         end
 
         def perform_request request, uri
-          response = Retry.until_truthy_or_max_times do
+          response = until_truthy_or_max_times(times: 5, sleep: 5, condition: ->(resp) { resp.code.to_i < 500 }) do
             http = Net::HTTP.new(uri.host, uri.port, :ENV)
             http.set_debug_output(output_stream) if verbose
             http.use_ssl = (uri.scheme == 'https')
@@ -69,6 +70,37 @@ def perform_request request, uri
           Response.new(response)
         end
 
+        def until_truthy_or_max_times options = {}
+          max_tries = options.fetch(:times, 3)
+          tries = 0
+          sleep_interval = options.fetch(:sleep, 5)
+          sleep(sleep_interval) if options[:sleep_first]
+          while true
+            begin
+              result = yield
+              return result if max_tries < 2
+              if options[:condition]
+                condition_result = options[:condition].call(result)
+                return result if condition_result
+              else
+                return result if result
+              end
+              tries += 1
+              return result if max_tries == tries
+              sleep sleep_interval
+            rescue *RETRYABLE_ERRORS => e
+              tries += 1
+              $stderr.puts "ERROR: Error making request - #{e.class} #{e.message} #{e.backtrace.find{|l| l.include?('pact_broker-client')}}, attempt #{tries} of #{max_tries}"
+              raise e if max_tries == tries
+              sleep sleep_interval
+            end
+          end
+        end
+
+        def sleep seconds
+          Kernel.sleep seconds
+        end
+
         def output_stream
           AuthorizationHeaderRedactor.new($stdout)
         end
diff --git a/spec/lib/pact_broker/client/hal/http_client_spec.rb b/spec/lib/pact_broker/client/hal/http_client_spec.rb
index 790261c7..cd219497 100644
--- a/spec/lib/pact_broker/client/hal/http_client_spec.rb
+++ b/spec/lib/pact_broker/client/hal/http_client_spec.rb
@@ -3,13 +3,13 @@
 module PactBroker::Client
   module Hal
     describe HttpClient do
-      before do
-        allow(Retry).to receive(:until_truthy_or_max_times) { |&block| block.call }
-      end
-
       subject { HttpClient.new(username: 'foo', password: 'bar') }
 
       describe "get" do
+        before do
+          allow(subject).to receive(:until_truthy_or_max_times) { |&block| block.call }
+        end
+
         let!(:request) do
           stub_request(:get, "http://example.org/").
             with(  headers: {
@@ -41,18 +41,22 @@ module Hal
           end
         end
 
-
         it "retries on failure" do
-          expect(Retry).to receive(:until_truthy_or_max_times)
+          expect(subject).to receive(:until_truthy_or_max_times)
           do_get
         end
 
         it "returns a response" do
           expect(do_get.body).to eq({"some" => "json"})
         end
+
       end
 
       describe "post" do
+        before do
+          allow(subject).to receive(:until_truthy_or_max_times) { |&block| block.call }
+        end
+
         let!(:request) do
           stub_request(:post, "http://example.org/").
             with(  headers: {
@@ -75,7 +79,7 @@ module Hal
         end
 
         it "calls Retry.until_truthy_or_max_times" do
-          expect(Retry).to receive(:until_truthy_or_max_times)
+          expect(subject).to receive(:until_truthy_or_max_times)
           do_post
         end
 
@@ -100,6 +104,59 @@ module Hal
           end
         end
       end
+
+      describe "integration test" do
+        before do
+          allow(subject).to receive(:sleep)
+        end
+
+        let(:do_get) { subject.get('http://example.org') }
+
+        context "with a 50x error is returned less than the max number of tries" do
+          let!(:request) do
+            stub_request(:get, "http://example.org").
+              to_return({ status: 500 }, { status: 502 }, { status: 503 }, { status: 200 })
+          end
+
+          it "retries" do
+            expect(do_get.status).to eq 200
+          end
+        end
+
+        context "with a 50x error is returned more than the max number of tries" do
+          let!(:request) do
+            stub_request(:get, "http://example.org").
+              to_return({ status: 500 }, { status: 501 }, { status: 502 }, { status: 503 }, { status: 504 })
+          end
+
+          it "retries and returns the last 50x response" do
+            expect(do_get.status).to eq 504
+          end
+        end
+
+        context "when exceptions are raised" do
+          before do
+            allow($stderr).to receive(:puts)
+          end
+
+          let!(:request) do
+            stub_request(:get, "http://example.org")
+              .to_raise(Errno::ECONNREFUSED)
+          end
+
+          it "logs the error" do
+            expect($stderr).to receive(:puts).with(/Errno::ECONNREFUSED/)
+            begin
+              do_get
+            rescue Errno::ECONNREFUSED
+            end
+          end
+
+          it "retries and raises the last exception" do
+            expect { do_get }.to raise_error(Errno::ECONNREFUSED)
+          end
+        end
+      end
     end
   end
 end