Skip to content

Commit

Permalink
add data about the request to http errors
Browse files Browse the repository at this point in the history
not super pumped about the monkey patching, but this will be useful for
adding context to API errors
  • Loading branch information
danielsdeleo committed Jun 22, 2012
1 parent 8db8a8f commit 97282c2
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 38 deletions.
22 changes: 22 additions & 0 deletions chef/lib/chef/monkey_patches/net_http.rb
@@ -0,0 +1,22 @@

# Module gets mixed in to Net::HTTP exception classes so we can attach our
# RESTRequest object to them and get the request parameters back out later.
module ChefNetHTTPExceptionExtensions
attr_accessor :chef_rest_request
end

require 'net/http'
module Net
class HTTPError
include ChefNetHTTPExceptionExtensions
end
class HTTPRetriableError
include ChefNetHTTPExceptionExtensions
end
class HTTPServerException
include ChefNetHTTPExceptionExtensions
end
class HTTPFatalError
include ChefNetHTTPExceptionExtensions
end
end
93 changes: 55 additions & 38 deletions chef/lib/chef/rest.rb
Expand Up @@ -28,8 +28,11 @@
require 'chef/rest/auth_credentials'
require 'chef/rest/rest_request'
require 'chef/monkey_patches/string'
require 'chef/monkey_patches/net_http'
require 'chef/config'



class Chef
# == Chef::REST
# Chef's custom REST client with built-in JSON support and RSA signed header
Expand Down Expand Up @@ -240,30 +243,37 @@ def api_request(method, url, headers={}, data=false)
headers = build_headers(method, url, headers, json_body)

retriable_rest_request(method, url, json_body, headers) do |rest_request|
response = rest_request.call {|r| r.read_body}
begin
response = rest_request.call {|r| r.read_body}

response_body = decompress_body(response)
response_body = decompress_body(response)

if response.kind_of?(Net::HTTPSuccess)
if response['content-type'] =~ /json/
Chef::JSONCompat.from_json(response_body.chomp)
if response.kind_of?(Net::HTTPSuccess)
if response['content-type'] =~ /json/
Chef::JSONCompat.from_json(response_body.chomp)
else
Chef::Log.warn("Expected JSON response, but got content-type '#{response['content-type']}'")
response_body
end
elsif redirect_location = redirected_to(response)
follow_redirect {api_request(:GET, create_url(redirect_location))}
else
Chef::Log.warn("Expected JSON response, but got content-type '#{response['content-type']}'")
response_body
# have to decompress the body before making an exception for it. But the body could be nil.
response.body.replace(decompress_body(response)) if response.body.respond_to?(:replace)

if response['content-type'] =~ /json/
exception = Chef::JSONCompat.from_json(response_body)
msg = "HTTP Request Returned #{response.code} #{response.message}: "
msg << (exception["error"].respond_to?(:join) ? exception["error"].join(", ") : exception["error"].to_s)
Chef::Log.info(msg)
end
response.error!
end
elsif redirect_location = redirected_to(response)
follow_redirect {api_request(:GET, create_url(redirect_location))}
else
# have to decompress the body before making an exception for it. But the body could be nil.
response.body.replace(decompress_body(response)) if response.body.respond_to?(:replace)

if response['content-type'] =~ /json/
exception = Chef::JSONCompat.from_json(response_body)
msg = "HTTP Request Returned #{response.code} #{response.message}: "
msg << (exception["error"].respond_to?(:join) ? exception["error"].join(", ") : exception["error"].to_s)
Chef::Log.info(msg)
rescue Exception => e
if e.respond_to?(:chef_rest_request=)
e.chef_rest_request = rest_request
end
response.error!
raise
end
end
end
Expand Down Expand Up @@ -295,28 +305,35 @@ def decompress_body(response)
def streaming_request(url, headers, &block)
headers = build_headers(:GET, url, headers, nil, true)
retriable_rest_request(:GET, url, nil, headers) do |rest_request|
tempfile = nil
response = rest_request.call do |r|
if block_given? && r.kind_of?(Net::HTTPSuccess)
begin
tempfile = stream_to_tempfile(url, r, &block)
yield tempfile
ensure
tempfile.close!
begin
tempfile = nil
response = rest_request.call do |r|
if block_given? && r.kind_of?(Net::HTTPSuccess)
begin
tempfile = stream_to_tempfile(url, r, &block)
yield tempfile
ensure
tempfile.close!
end
else
tempfile = stream_to_tempfile(url, r)
end
end
if response.kind_of?(Net::HTTPSuccess)
tempfile
elsif redirect_location = redirected_to(response)
# TODO: test tempfile unlinked when following redirects.
tempfile && tempfile.close!
follow_redirect {streaming_request(create_url(redirect_location), {}, &block)}
else
tempfile = stream_to_tempfile(url, r)
tempfile && tempfile.close!
response.error!
end
end
if response.kind_of?(Net::HTTPSuccess)
tempfile
elsif redirect_location = redirected_to(response)
# TODO: test tempfile unlinked when following redirects.
tempfile && tempfile.close!
follow_redirect {streaming_request(create_url(redirect_location), {}, &block)}
else
tempfile && tempfile.close!
response.error!
rescue Exception => e
if e.respond_to?(:chef_rest_request=)
e.chef_rest_request = rest_request
end
raise
end
end
end
Expand Down
16 changes: 16 additions & 0 deletions chef/spec/unit/rest_spec.rb
Expand Up @@ -260,6 +260,22 @@
lambda {@rest.run_request(:GET, @url)}.should raise_error(Net::HTTPFatalError)
end

it "adds the rest_request object to any http exception raised" do
@http_response = Net::HTTPServerError.new("1.1", "500", "drooling from inside of mouth")
http_response = Net::HTTPServerError.new("1.1", "500", "drooling from inside of mouth")
http_response.stub!(:read_body)
@rest.stub!(:sleep)
@http_client.stub!(:request).and_yield(http_response).and_return(http_response)
exception = begin
@rest.api_request(:GET, @url, {})
rescue => e
e
end

e.chef_rest_request.url.should == @url
e.chef_rest_request.method.should == :GET
end

describe "streaming downloads to a tempfile" do
before do
@tempfile = Tempfile.open("chef-rspec-rest_spec-line-#{__LINE__}--")
Expand Down

0 comments on commit 97282c2

Please sign in to comment.