Skip to content

Commit

Permalink
Allow Features to handle connection errors
Browse files Browse the repository at this point in the history
This addresses #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.
  • Loading branch information
joshuaflanagan authored and tarcieri committed Oct 21, 2019
1 parent 4430932 commit 609f07b
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 13 deletions.
36 changes: 23 additions & 13 deletions lib/http/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/http/feature.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ def wrap_request(request)
def wrap_response(response)
response
end

def on_error(request, error); end
end
end

Expand Down
55 changes: 55 additions & 0 deletions spec/lib/http/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 609f07b

Please sign in to comment.