Skip to content

Commit

Permalink
Stop rescuing all exceptions.
Browse files Browse the repository at this point in the history
We do want to generally rescue `Exception` and
any subclasses but there are a few exceptions
we should not rescue. For example, if we rescue
`SystemExit`, it prevents `exit` from exiting.

Fixes #2058.
  • Loading branch information
myronmarston committed Aug 18, 2015
1 parent fecfcca commit 8cada17
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 9 deletions.
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,6 @@ Style/RaiseArgs:
- lib/rspec/core/hooks.rb
- lib/rspec/core/option_parser.rb
- lib/rspec/core/pending.rb

RescueException:
Enabled: true
8 changes: 8 additions & 0 deletions lib/rspec/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -178,4 +178,12 @@ def self.const_missing(name)
require MODULES_TO_AUTOLOAD.fetch(name) { return super }
::RSpec.const_get(name)
end

module NonPassthroughExceptions
PASSTHROUGH_EXCEPTIONS = [NoMemoryError, SignalException, Interrupt, SystemExit]

def self.===(exception)
PASSTHROUGH_EXCEPTIONS.none? { |pe| pe === exception }
end
end
end
2 changes: 1 addition & 1 deletion lib/rspec/core/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1735,7 +1735,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 NonPassthroughExceptions => exception
procsy.example.set_aggregate_failures_exception(exception)
end
end
Expand Down
10 changes: 5 additions & 5 deletions lib/rspec/core/example.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 NonPassthroughExceptions => e
set_exception(e)
ensure
run_after_example
end
end
end
rescue Exception => e
rescue NonPassthroughExceptions => e
set_exception(e)
ensure
@example_group_instance = nil # if you love something... let it go
Expand Down Expand Up @@ -400,7 +400,7 @@ def hooks

def with_around_example_hooks
hooks.run(:around, :example, self) { yield }
rescue Exception => e
rescue NonPassthroughExceptions => e
set_exception(e)
end

Expand Down Expand Up @@ -457,7 +457,7 @@ def run_after_example

def verify_mocks
@example_group_instance.verify_mocks_for_rspec if mocks_need_verification?
rescue Exception => e
rescue NonPassthroughExceptions => e
set_exception(e)
end

Expand All @@ -476,7 +476,7 @@ def assign_generated_description

def generate_description
RSpec::Matchers.generated_description
rescue Exception => e
rescue NonPassthroughExceptions => e
location_description + " (Got an error when generating description " \
"from matcher: #{e.class}: #{e.message} -- #{e.backtrace.first})"
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rspec/core/example_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 NonPassthroughExceptions => ex
RSpec.world.wants_to_quit = true if fail_fast?
for_filtered_examples(reporter) { |example| example.fail_with_exception(reporter, ex) }
false
Expand Down
4 changes: 2 additions & 2 deletions lib/rspec/core/hooks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ def run(example)
class AfterHook < Hook
def run(example)
example.instance_exec(example, &block)
rescue Exception => ex
rescue NonPassthroughExceptions => ex
example.set_exception(ex)
end
end
Expand All @@ -371,7 +371,7 @@ def run(example)
class AfterContextHook < Hook
def run(example)
example.instance_exec(example, &block)
rescue Exception => e
rescue NonPassthroughExceptions => e
# TODO: Come up with a better solution for this.
RSpec.configuration.reporter.message <<-EOS
Expand Down
24 changes: 24 additions & 0 deletions spec/integration/subprocess_exit_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
require 'support/aruba_support'

RSpec.describe 'When an example forks and exits it the subprocess' do
include_context "aruba support"
before { clean_current_dir }

it 'prints the results only once' do
write_file_formatted "spec/fork_spec.rb", "
RSpec.describe do
example { exit unless fork }
end
"

# Must pipe output to a file to see double output bug.
# If used the `aruba_support` default of capturing the output
# in a StringIO, we won't see the output from the subprocess
# because it can't write to a StringIO in memory in the parent process.
run_command "spec/fork_spec.rb --out rspec.out"
in_current_dir do
finished_lines = File.read("rspec.out").lines.grep(/Finished in/)
expect(finished_lines.count).to eq(1)
end
end
end

0 comments on commit 8cada17

Please sign in to comment.