Print out request and response body on non-2xx responsejk/http-error-log

5 files changed, 89 insertions, 4 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 471a00500f..50be057943 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -69,10 +69,18 @@
 
 ### Chef Contributions
 
+* Report the request and response when a non-200 error code happens
+* [FEATURE] Upgrade `knife upload` and `knife download` to download
+  **everything** in an organization, now including the organization definition
+  itself (`knife download /org.json`) and the invitations and member list
+  (`knife download /invitations.json` and `knife download /members.json`).
+  Should be compatible with knife-ec-backup.
+* Make default Windows paths more backslashy
 * `knife` now prefers to load `config.rb` in preference to `knife.rb`;
 `knife.rb` will be used if `config.rb` is not found.
 * Fixed Config[:cache_path] to use path_join()
-* Updated chef-zero to 3.0.
+* Updated chef-zero to 3.0, so that client tests can be run against Enterprise
+  Chef as well as Open Source.
 * knife cookbook site download/list/search/share/show/unshare now uses
   supermerket.getchef.com urls
 * added Chef::ResourceCollection#insert_at API to the ResourceCollection
diff --git a/lib/chef/http/basic_client.rb b/lib/chef/http/basic_client.rb
index b9a82499ed..f0f5151dbd 100644
--- a/lib/chef/http/basic_client.rb
+++ b/lib/chef/http/basic_client.rb
@@ -71,6 +71,20 @@ class Chef
           end
           Chef::Log.debug("---- End HTTP Status/Header Data ----")
 
+          # For non-400's, log the request and response bodies
+          if !response.code || !response.code.start_with?('2')
+            if response.body
+              Chef::Log.debug("---- HTTP Response Body ----")
+              Chef::Log.debug(response.body)
+              Chef::Log.debug("---- End HTTP Response Body -----")
+            end
+            if req_body
+              Chef::Log.debug("---- HTTP Request Body ----")
+              Chef::Log.debug(req_body)
+              Chef::Log.debug("---- End HTTP Request Body ----")
+            end
+          end
+
           yield response if block_given?
           # http_client.request may not have the return signature we want, so
           # force the issue:
diff --git a/spec/functional/http/simple_spec.rb b/spec/functional/http/simple_spec.rb
index 092d164342..fec71351df 100644
--- a/spec/functional/http/simple_spec.rb
+++ b/spec/functional/http/simple_spec.rb
@@ -80,5 +80,61 @@ describe Chef::HTTP::Simple do
   end
 
   it_behaves_like "downloading all the things"
-end
 
+  context "when Chef::Log.level = :debug" do
+    before do
+      Chef::Log.level = :debug
+      @debug_log = ''
+      Chef::Log.stub(:debug) { |str| @debug_log << str }
+    end
+
+    let(:source) { 'http://localhost:9000' }
+
+    it "Logs the request and response for 200's but not the body" do
+      http_client.get('http://localhost:9000/nyan_cat.png')
+      expect(@debug_log).to match(/200/)
+      expect(@debug_log).to match(/HTTP Request Header Data/)
+      expect(@debug_log).to match(/HTTP Status and Header Data/)
+      expect(@debug_log).not_to match(/HTTP Request Body/)
+      expect(@debug_log).not_to match(/HTTP Response Body/)
+      expect(@debug_log).not_to match(/Your request is just terrible./)
+    end
+
+    it "Logs the request and response for 200 POST, but not the body" do
+      http_client.post('http://localhost:9000/posty', 'hithere')
+      expect(@debug_log).to match(/200/)
+      expect(@debug_log).to match(/HTTP Request Header Data/)
+      expect(@debug_log).to match(/HTTP Status and Header Data/)
+      expect(@debug_log).not_to match(/HTTP Request Body/)
+      expect(@debug_log).not_to match(/hithere/)
+      expect(@debug_log).not_to match(/HTTP Response Body/)
+      expect(@debug_log).not_to match(/Your request is just terrible./)
+    end
+
+    it "Logs the request and response and bodies for 400 response" do
+      expect do
+        http_client.get('http://localhost:9000/bad_request')
+      end.to raise_error(Net::HTTPServerException)
+      expect(@debug_log).to match(/400/)
+      expect(@debug_log).to match(/HTTP Request Header Data/)
+      expect(@debug_log).to match(/HTTP Status and Header Data/)
+      expect(@debug_log).not_to match(/HTTP Request Body/)
+      expect(@debug_log).not_to match(/hithere/)
+      expect(@debug_log).to match(/HTTP Response Body/)
+      expect(@debug_log).to match(/Your request is just terrible./)
+    end
+
+    it "Logs the request and response and bodies for 400 POST response" do
+      expect do
+        http_client.post('http://localhost:9000/bad_request', 'hithere')
+      end.to raise_error(Net::HTTPServerException)
+      expect(@debug_log).to match(/400/)
+      expect(@debug_log).to match(/HTTP Request Header Data/)
+      expect(@debug_log).to match(/HTTP Status and Header Data/)
+      expect(@debug_log).to match(/HTTP Request Body/)
+      expect(@debug_log).to match(/hithere/)
+      expect(@debug_log).to match(/HTTP Response Body/)
+      expect(@debug_log).to match(/Your request is just terrible./)
+    end
+  end
+end
diff --git a/spec/support/shared/functional/http.rb b/spec/support/shared/functional/http.rb
index c362ecaa18..9a3389306d 100644
--- a/spec/support/shared/functional/http.rb
+++ b/spec/support/shared/functional/http.rb
@@ -155,6 +155,14 @@ module ChefHTTPShared
       }
     )
 
+    @api.post('/posty', 200, 'Hi!')
+
+    #
+    # 400 with an error
+    #
+    @api.get('/bad_request', 400, '{ "error": [ "Your request is just terrible." ] }')
+    @api.post('/bad_request', 400, '{ "error": [ "Your request is just terrible." ] }')
+
   end
 
   def stop_tiny_server
@@ -239,4 +247,3 @@ shared_examples_for "downloading all the things" do
     it_behaves_like "a 403 after a successful request when reusing the request object"
   end
 end
-
diff --git a/spec/unit/rest_spec.rb b/spec/unit/rest_spec.rb
index f20672ef25..3ad822193b 100644
--- a/spec/unit/rest_spec.rb
+++ b/spec/unit/rest_spec.rb
@@ -540,7 +540,7 @@ describe Chef::REST do
       let(:request_mock) { {} }
 
       let(:http_response) do
-        http_response = Net::HTTPSuccess.new("1.1",200, "it-works")
+        http_response = Net::HTTPSuccess.new("1.1",'200', "it-works")
 
         http_response.stub(:read_body)
         http_response.should_not_receive(:body)