Raise errors at the end of the client run, for proper exiting.

3 files changed, 56 insertions, 28 deletions

diff --git a/lib/chef/client.rb b/lib/chef/client.rb
index 8203392b39..c9a07405e6 100644
--- a/lib/chef/client.rb
+++ b/lib/chef/client.rb
@@ -425,6 +425,8 @@ class Chef
     # === Returns
     # true:: Always returns true.
     def run
+      run_error = nil
+
       runlock = RunLock.new(Chef::Config.lockfile)
       runlock.acquire
       # don't add code that may fail before entering this section to be sure to release lock
@@ -466,10 +468,8 @@ class Chef
           audit_error = run_audits(run_context)
         end
 
-        if err = wrap_exceptions(converge_error, audit_error)
-          err.fill_backtrace
-          raise err
-        end
+        # Raise converge_error so run_failed reporters/events are processed.
+        raise converge_error if converge_error
 
         run_status.stop_clock
         Chef::Log.info("Chef Run complete in #{run_status.elapsed_time} seconds")
@@ -478,21 +478,16 @@ class Chef
 
         # rebooting has to be the last thing we do, no exceptions.
         Chef::Platform::Rebooter.reboot_if_needed!(node)
-
-        true
-
-      rescue Exception => e
+      rescue Exception => run_error
         # CHEF-3336: Send the error first in case something goes wrong below and we don't know why
-        Chef::Log.debug("Re-raising exception: #{e.class} - #{e.message}\n#{e.backtrace.join("\n  ")}")
+        Chef::Log.debug("Re-raising exception: #{run_error.class} - #{run_error.message}\n#{run_error.backtrace.join("\n  ")}")
         # If we failed really early, we may not have a run_status yet. Too early for these to be of much use.
         if run_status
           run_status.stop_clock
-          run_status.exception = e
+          run_status.exception = run_error
           run_failed
         end
-        Chef::Application.debug_stacktrace(e)
-        @events.run_failed(e)
-        raise
+        @events.run_failed(run_error)
       ensure
         Chef::RequestID.instance.reset_request_id
         request_id = nil
@@ -501,6 +496,23 @@ class Chef
         runlock.release
         GC.start
       end
+
+      # Raise audit, converge, and other errors here so that we exit
+      # with the proper exit status code and everything gets raised
+      # as a RunFailedWrappingError
+      if run_error || converge_error || audit_error
+        error = if run_error == converge_error
+          Chef::Exceptions::RunFailedWrappingError.new(converge_error, audit_error)
+        else
+          Chef::Exceptions::RunFailedWrappingError.new(run_error, converge_error, audit_error)
+        end
+        error.fill_backtrace
+        Chef::Application.debug_stacktrace(error)
+        raise error
+      end
+
+      run_error = nil
+
       true
     end
 
@@ -537,17 +549,6 @@ class Chef
 
       Chef::ReservedNames::Win32::Security.has_admin_privileges?
     end
-
-    def wrap_exceptions(converge_error, audit_error)
-      err = if audit_error && !audit_error.is_a?(Chef::Exceptions::AuditsFailed)
-        Chef::Exceptions::RunFailedWrappingError.new(converge_error, audit_error)
-      elsif converge_error
-        Chef::Exceptions::RunFailedWrappingError.new(converge_error)
-      end
-      err.fill_backtrace if err
-      err
-    end
-
   end
 end
 
diff --git a/spec/support/shared/examples/client.rb b/spec/support/shared/examples/client.rb
index 0a57abc044..330cb40ac6 100644
--- a/spec/support/shared/examples/client.rb
+++ b/spec/support/shared/examples/client.rb
@@ -16,6 +16,28 @@ shared_examples "a completed run" do
   end
 end
 
+shared_examples "a completed run with audit failure" do
+  include_context "run completed"
+
+  before do
+    expect(Chef::Application).to receive(:debug_stacktrace).with an_instance_of(Chef::Exceptions::RunFailedWrappingError)
+  end
+
+  it "converges, runs audits, saves the node and raises the error in a wrapping error" do
+    expect{ client.run }.to raise_error(Chef::Exceptions::RunFailedWrappingError) do |error|
+      expect(error.wrapped_errors.size).to eq(run_errors.size)
+      run_errors.each do |run_error|
+        expect(error.wrapped_errors).to include(run_error)
+        expect(error.backtrace).to include(*run_error.backtrace)
+      end
+    end
+
+    # fork is stubbed, so we can see the outcome of the run
+    expect(node.automatic_attrs[:platform]).to eq(platform)
+    expect(node.automatic_attrs[:platform_version]).to eq(platform_version)
+  end
+end
+
 shared_examples "a failed run" do
   include_context "run failed"
 
diff --git a/spec/unit/client_spec.rb b/spec/unit/client_spec.rb
index 1704bcc461..1e4bbb5c56 100644
--- a/spec/unit/client_spec.rb
+++ b/spec/unit/client_spec.rb
@@ -241,7 +241,7 @@ describe Chef::Client do
 
       describe "when audit phase errors" do
         include_context "audit phase failed with error"
-        include_examples "a failed run" do
+        include_examples "a completed run with audit failure" do
           let(:run_errors) { [audit_error] }
         end
       end
@@ -253,7 +253,9 @@ describe Chef::Client do
 
       describe "when audit phase completed with failed controls" do
         include_context "audit phase completed with failed controls"
-        include_examples "a completed run"
+        include_examples "a completed run with audit failure" do
+          let(:run_errors) { [audit_error] }
+        end
       end
     end
 
@@ -278,7 +280,7 @@ describe Chef::Client do
       describe "when audit phase completed with failed controls" do
         include_context "audit phase completed with failed controls"
         include_examples "a failed run" do
-          let(:run_errors) { [converge_error] }
+          let(:run_errors) { [converge_error, audit_error] }
         end
       end
     end
@@ -512,7 +514,10 @@ describe Chef::Client do
 
     it "should run exception handlers on early fail" do
       expect(subject).to receive(:run_failed)
-      expect { subject.run }.to raise_error(NoMethodError)
+      expect { subject.run }.to raise_error(Chef::Exceptions::RunFailedWrappingError) do |error|
+        expect(error.wrapped_errors.size).to eq 1
+        expect(error.wrapped_errors).to include(NoMethodError)
+      end
     end
   end
 end