Don't `rescue Exception` for retriable resources

Without this, sending an interrupt signal (hitting Ctrl+C) during a
chef-client run that is currently executing a resources that has
`retries` set will not interrupt the run, although the interrupt handler
set up in Chef::Application calls Process.exit, which raises SystemExit.

If the retryable resource is shelling out, the spawned process also gets
the INT signal and will stop -- however, the chef-run would go on,
merely noticing that the resource has failed and theres X-1 retries
left.

It is imaginable that users depend on some (whacky) Ruby script in a
retriable resource that raises SystemExit, and they want this to _not_
stop the chef-run. So it's up for debate if we just want to merge this
or make it configurable.

In general, I'd like Ctrl+C to _stop my chef-run_, but this expectation
could also be misled.

When adapting tests to this change, it became apparent that for some
reason the runner_spec.rb test didn't behave as expected. I've thus made
those tests a little stricter by replacing some allows by expects.

Signed-off-by: Stephan Renatus <srenatus@chef.io>

1 files changed, 12 insertions, 0 deletions

diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index a83b22ea0f..a806c5d1d9 100644
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -549,6 +549,18 @@ describe Chef::Resource do
       expect { retriable_resource.run_action(:purr) }.to raise_error(RuntimeError)
       expect(retriable_resource.retries).to eq(3)
     end
+
+    it "should not rescue from non-StandardError exceptions" do
+      retriable_resource.retries(3)
+      retriable_resource.retry_delay(0) # No need to wait.
+
+      provider = Chef::Provider::SnakeOil.new(retriable_resource, run_context)
+      allow(Chef::Provider::SnakeOil).to receive(:new).and_return(provider)
+      allow(provider).to receive(:action_purr).and_raise(LoadError)
+
+      expect(retriable_resource).not_to receive(:sleep)
+      expect { retriable_resource.run_action(:purr) }.to raise_error(LoadError)
+    end
   end
 
   it "runs an action by finding its provider, loading the current resource and then running the action" do