From 609f07b3454bde2f48ca5e1589cda169f7a77187 Mon Sep 17 00:00:00 2001
From: Joshua Flanagan <joshuaflanagan@gmail.com>
Date: Mon, 20 May 2019 18:45:25 -0500
Subject: [PATCH] Allow Features to handle connection errors

This addresses https://github.com/httprb/http/issues/547

It allows a Feature to implement an on_error handler so that it has a chance to react to Requests that never get a Response.

> Extracted the build_response method to stay under the rubocop enforced method length limit.
---
 lib/http/client.rb           | 36 ++++++++++++++---------
 lib/http/feature.rb          |  2 ++
 spec/lib/http/client_spec.rb | 55 ++++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+), 13 deletions(-)

diff --git a/lib/http/client.rb b/lib/http/client.rb
index daca1c23..ece1140a 100644
--- a/lib/http/client.rb
+++ b/lib/http/client.rb
@@ -70,20 +70,18 @@ def perform(req, options)
 
       @connection ||= HTTP::Connection.new(req, options)
 
-      unless @connection.failed_proxy_connect?
-        @connection.send_request(req)
-        @connection.read_headers!
+      begin
+        unless @connection.failed_proxy_connect?
+          @connection.send_request(req)
+          @connection.read_headers!
+        end
+      rescue Error => e
+        options.features.each_value do |feature|
+          feature.on_error(req, e)
+        end
+        raise
       end
-
-      res = Response.new(
-        :status        => @connection.status_code,
-        :version       => @connection.http_version,
-        :headers       => @connection.headers,
-        :proxy_headers => @connection.proxy_response_headers,
-        :connection    => @connection,
-        :encoding      => options.encoding,
-        :request       => req
-      )
+      res = build_response(req, options)
 
       res = options.features.inject(res) do |response, (_name, feature)|
         feature.wrap_response(response)
@@ -106,6 +104,18 @@ def close
 
     private
 
+    def build_response(req, options)
+      Response.new(
+        :status        => @connection.status_code,
+        :version       => @connection.http_version,
+        :headers       => @connection.headers,
+        :proxy_headers => @connection.proxy_response_headers,
+        :connection    => @connection,
+        :encoding      => options.encoding,
+        :request       => req
+      )
+    end
+
     # Verify our request isn't going to be made against another URI
     def verify_connection!(uri)
       if default_options.persistent? && uri.origin != default_options.persistent
diff --git a/lib/http/feature.rb b/lib/http/feature.rb
index 53d33811..adb60339 100644
--- a/lib/http/feature.rb
+++ b/lib/http/feature.rb
@@ -13,6 +13,8 @@ def wrap_request(request)
     def wrap_response(response)
       response
     end
+
+    def on_error(request, error); end
   end
 end
 
diff --git a/spec/lib/http/client_spec.rb b/spec/lib/http/client_spec.rb
index a3a144cc..3ce51175 100644
--- a/spec/lib/http/client_spec.rb
+++ b/spec/lib/http/client_spec.rb
@@ -266,6 +266,61 @@ def simple_response(body, status = 200)
         end
       end
     end
+
+    context "Feature" do
+      let(:feature_class) do
+        Class.new(HTTP::Feature) do
+          attr_reader :captured_request, :captured_response, :captured_error
+
+          def wrap_request(request)
+            @captured_request = request
+          end
+
+          def wrap_response(response)
+            @captured_response = response
+          end
+
+          def on_error(request, error)
+            @captured_request = request
+            @captured_error = error
+          end
+        end
+      end
+      it "is given a chance to wrap the Request" do
+        feature_instance = feature_class.new
+
+        response = client.use(:test_feature => feature_instance).
+                   request(:get, dummy.endpoint)
+
+        expect(response.code).to eq(200)
+        expect(feature_instance.captured_request.verb).to eq(:get)
+        expect(feature_instance.captured_request.uri.to_s).to eq(dummy.endpoint + "/")
+      end
+
+      it "is given a chance to wrap the Response" do
+        feature_instance = feature_class.new
+
+        response = client.use(:test_feature => feature_instance).
+                   request(:get, dummy.endpoint)
+
+        expect(feature_instance.captured_response).to eq(response)
+      end
+
+      it "is given a chance to handle an error" do
+        sleep_url = "#{dummy.endpoint}/sleep"
+        feature_instance = feature_class.new
+
+        expect do
+          client.use(:test_feature => feature_instance).
+            timeout(0.2).
+            request(:post, sleep_url)
+        end.to raise_error(HTTP::TimeoutError)
+
+        expect(feature_instance.captured_error).to be_a(HTTP::TimeoutError)
+        expect(feature_instance.captured_request.verb).to eq(:post)
+        expect(feature_instance.captured_request.uri.to_s).to eq(sleep_url)
+      end
+    end
   end
 
   include_context "HTTP handling" do