Merge pull request #2063 from rspec/stop-rescuing-all-exceptions

Stop rescuing all exceptions.

Author: myronmarston

.rubocop_rspec_base.yml

@@ -101,9 +101,9 @@ Proc:
 RedundantReturn:
   AllowMultipleReturnValues: true
 
-# We have to rescue Exception in the `raise_error` matcher for it to work properly.
+# Exceptions should be rescued with `Support::AllExceptionsExceptOnesWeMustNotRescue`
 RescueException:
-  Enabled: false
+  Enabled: true
 
 # We haven't adopted the `fail` to signal exceptions vs `raise` for re-raises convention.
 SignalException:

Changelog.md

@@ -24,6 +24,8 @@ Enhancements:
   (Jon Rowe, #2052)
 * Append the root `cause` of a failure or error to the printed failure
   output when a `cause` is available. (Adam Magan)
+* Stop rescuing `NoMemoryError`, `SignalExcepetion`, `Interrupt` and
+  `SystemExit`. It is dangerous to interfere with these. (Myron Marston, #2063)
 
 Bug Fixes:
 

lib/rspec/core/configuration.rb

@@ -1754,7 +1754,7 @@ def define_built_in_hooks
         around(:example, :aggregate_failures => true) do |procsy|
           begin
             aggregate_failures(nil, :hide_backtrace => true, &procsy)
-          rescue Exception => exception
+          rescue Support::AllExceptionsExceptOnesWeMustNotRescue => exception
             procsy.example.set_aggregate_failures_exception(exception)
           end
         end

lib/rspec/core/example.rb

@@ -227,14 +227,14 @@ def run(example_group_instance, reporter)
               rescue Pending::SkipDeclaredInExample
                 # no-op, required metadata has already been set by the `skip`
                 # method.
-              rescue Exception => e
+              rescue AllExceptionsExcludingDangerousOnesOnRubiesThatAllowIt => e
                 set_exception(e)
               ensure
                 run_after_example
               end
             end
           end
-        rescue Exception => e
+        rescue Support::AllExceptionsExceptOnesWeMustNotRescue => e
           set_exception(e)
         ensure
           @example_group_instance = nil # if you love something... let it go
@@ -245,6 +245,20 @@ def run(example_group_instance, reporter)
         RSpec.current_example = nil
       end
 
+      if RSpec::Support::Ruby.jruby? || RUBY_VERSION.to_f < 1.9
+        # :nocov:
+        # For some reason, rescuing `Support::AllExceptionsExceptOnesWeMustNotRescue`
+        # in place of `Exception` above can cause the exit status to be the wrong
+        # thing. I have no idea why. See:
+        # https://github.com/rspec/rspec-core/pull/2063#discussion_r38284978
+        # @private
+        AllExceptionsExcludingDangerousOnesOnRubiesThatAllowIt = Exception
+        # :nocov:
+      else
+        # @private
+        AllExceptionsExcludingDangerousOnesOnRubiesThatAllowIt = Support::AllExceptionsExceptOnesWeMustNotRescue
+      end
+
       # Wraps both a `Proc` and an {Example} for use in {Hooks#around
       # around} hooks. In around hooks we need to yield this special
       # kind of object (rather than the raw {Example}) because when
@@ -400,7 +414,7 @@ def hooks
 
       def with_around_example_hooks
         hooks.run(:around, :example, self) { yield }
-      rescue Exception => e
+      rescue Support::AllExceptionsExceptOnesWeMustNotRescue => e
         set_exception(e)
       end
 
@@ -457,7 +471,7 @@ def run_after_example
 
       def verify_mocks
         @example_group_instance.verify_mocks_for_rspec if mocks_need_verification?
-      rescue Exception => e
+      rescue Support::AllExceptionsExceptOnesWeMustNotRescue => e
         set_exception(e)
       end
 
@@ -476,7 +490,7 @@ def assign_generated_description
 
       def generate_description
         RSpec::Matchers.generated_description
-      rescue Exception => e
+      rescue Support::AllExceptionsExceptOnesWeMustNotRescue => e
         location_description + " (Got an error when generating description " \
           "from matcher: #{e.class}: #{e.message} -- #{e.backtrace.first})"
       end

lib/rspec/core/example_group.rb

@@ -524,7 +524,7 @@ def self.run(reporter=RSpec::Core::NullReporter)
         rescue Pending::SkipDeclaredInExample => ex
           for_filtered_examples(reporter) { |example| example.skip_with_exception(reporter, ex) }
           true
-        rescue Exception => ex
+        rescue Support::AllExceptionsExceptOnesWeMustNotRescue => ex
           RSpec.world.wants_to_quit = true if fail_fast?
           for_filtered_examples(reporter) { |example| example.fail_with_exception(reporter, ex) }
           false

lib/rspec/core/hooks.rb

@@ -362,7 +362,7 @@ def run(example)
       class AfterHook < Hook
         def run(example)
           example.instance_exec(example, &block)
-        rescue Exception => ex
+        rescue Support::AllExceptionsExceptOnesWeMustNotRescue => ex
           example.set_exception(ex)
         end
       end
@@ -371,7 +371,7 @@ def run(example)
       class AfterContextHook < Hook
         def run(example)
           example.instance_exec(example, &block)
-        rescue Exception => e
+        rescue Support::AllExceptionsExceptOnesWeMustNotRescue => e
           # TODO: Come up with a better solution for this.
           RSpec.configuration.reporter.message <<-EOS