Merge pull request #1551 from hltbra/fix-resource-reporter-post-resporting-data

Fix ResourceReporter#post_reporting_data http error handling. Fixes #1550

2 files changed, 55 insertions, 7 deletions

diff --git a/lib/chef/resource_reporter.rb b/lib/chef/resource_reporter.rb
index 47bbd13741..046e4e82c6 100644
--- a/lib/chef/resource_reporter.rb
+++ b/lib/chef/resource_reporter.rb
@@ -231,17 +231,17 @@ class Chef
         Chef::Log.info("Sending resource update report (run-id: #{run_id})")
         Chef::Log.debug run_data.inspect
         compressed_data = encode_gzip(run_data.to_json)
+        Chef::Log.debug("Sending compressed run data...")
+        # Since we're posting compressed data we can not directly call post_rest which expects JSON
+        reporting_url = @rest_client.create_url(resource_history_url)
         begin
-          Chef::Log.debug("Sending compressed run data...")
-          # Since we're posting compressed data we can not directly call post_rest which expects JSON
-          reporting_url = @rest_client.create_url(resource_history_url)
           @rest_client.raw_http_request(:POST, reporting_url, headers({'Content-Encoding' => 'gzip'}), compressed_data)
-        rescue Net::HTTPServerException => e
-          if e.response.code.to_s == "400"
+        rescue StandardError => e
+          if e.respond_to? :response
             Chef::FileCache.store("failed-reporting-data.json", Chef::JSONCompat.to_json_pretty(run_data), 0640)
-            Chef::Log.error("Failed to post reporting data to server (HTTP 400), saving to #{Chef::FileCache.load("failed-reporting-data.json", false)}")
+            Chef::Log.error("Failed to post reporting data to server (HTTP #{e.response.code}), saving to #{Chef::FileCache.load("failed-reporting-data.json", false)}")
           else
-            Chef::Log.error("Failed to post reporting data to server (HTTP #{e.response.code.to_s})")
+            Chef::Log.error("Failed to post reporting data to server (#{e})")
           end
         end
       else
diff --git a/spec/unit/resource_reporter_spec.rb b/spec/unit/resource_reporter_spec.rb
index a7ec665495..fe6a895b5a 100644
--- a/spec/unit/resource_reporter_spec.rb
+++ b/spec/unit/resource_reporter_spec.rb
@@ -21,6 +21,7 @@
 
 require File.expand_path("../../spec_helper", __FILE__)
 require 'chef/resource_reporter'
+require 'socket'
 
 describe Chef::ResourceReporter do
   before(:all) do
@@ -707,5 +708,52 @@ describe Chef::ResourceReporter do
         @resource_reporter.run_completed(@node)
       end
     end
+
+    context "when data report post is enabled and the server response fails" do
+      before do
+        @enable_reporting_url_fatals = Chef::Config[:enable_reporting_url_fatals]
+        Chef::Config[:enable_reporting_url_fatals] = true
+        # this call doesn't matter for this context
+        @rest_client.stub(:create_url)
+      end
+
+      after do
+        Chef::Config[:enable_reporting_url_fatals] = @enable_reporting_url_fatals
+      end
+
+      it "should log 4xx errors" do
+        response = Net::HTTPClientError.new("forbidden", "403", "Forbidden")
+        error = Net::HTTPServerException.new("403 message", response)
+        @rest_client.stub(:raw_http_request).and_raise(error)
+        Chef::Log.should_receive(:error).with(/403/)
+
+        @resource_reporter.post_reporting_data
+      end
+
+      it "should log error 5xx errors" do
+        response = Net::HTTPServerError.new("internal error", "500", "Internal Server Error")
+        error = Net::HTTPFatalError.new("500 message", response)
+        @rest_client.stub(:raw_http_request).and_raise(error)
+        Chef::Log.should_receive(:error).with(/500/)
+
+        @resource_reporter.post_reporting_data
+      end
+
+      it "should log if a socket error happens" do
+        @rest_client.stub(:raw_http_request).and_raise(SocketError.new("test socket error"))
+        Chef::Log.should_receive(:error).with(/test socket error/)
+
+        @resource_reporter.post_reporting_data
+
+      end
+
+      it "should raise if an unkwown error happens" do
+        @rest_client.stub(:raw_http_request).and_raise(Exception.new)
+
+        lambda {
+          @resource_reporter.post_reporting_data
+        }.should raise_error(Exception)
+      end
+    end
   end
 end