Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent invalid cause from breaking exception presenter #2703

Merged
merged 2 commits into from Mar 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions Changelog.md
Expand Up @@ -5,6 +5,9 @@ Bug Fixes:

* Emit a warning when `around` hook is used with `:context` scope
(Phil Pirozhkov, #2687)
* Prevent invalid implementations of `Exception#cause` from being treated as a
valid cause (and causing strange errors) in `RSpec::Core::Formatters::ExceptionPresenter`.
(Jon Rowe, #2703)

### 3.9.1 / 2019-12-28
[Full Changelog](http://github.com/rspec/rspec-core/compare/v3.9.0...v3.9.1)
Expand Down
9 changes: 6 additions & 3 deletions lib/rspec/core/formatters/exception_presenter.rb
Expand Up @@ -43,7 +43,7 @@ def formatted_backtrace(exception=@exception)

if RSpec::Support::RubyFeatures.supports_exception_cause?
def formatted_cause(exception)
last_cause = final_exception(exception)
last_cause = final_exception(exception, [exception])
cause = []

if exception.cause
Expand All @@ -55,7 +55,9 @@ def formatted_cause(exception)
cause << " #{line}"
end

cause << (" #{backtrace_formatter.format_backtrace(last_cause.backtrace, example.metadata).first}")
unless last_cause.backtrace.empty?
cause << (" #{backtrace_formatter.format_backtrace(last_cause.backtrace, example.metadata).first}")
end
end

cause
Expand Down Expand Up @@ -96,7 +98,8 @@ def fully_formatted_lines(failure_number, colorizer)

def final_exception(exception, previous=[])
cause = exception.cause
if cause && !previous.include?(cause)

if cause && Exception === cause && !previous.include?(cause)
previous << cause
final_exception(cause, previous)
else
Expand Down
48 changes: 39 additions & 9 deletions spec/rspec/core/formatters/exception_presenter_spec.rb
Expand Up @@ -11,7 +11,22 @@ module RSpec::Core
before do
allow(example.execution_result).to receive(:exception) { exception }
example.metadata[:absolute_file_path] = __FILE__
allow(exception).to receive(:cause) if RSpec::Support::RubyFeatures.supports_exception_cause?
end

# This is a slightly more realistic exception than our instance_double
# created, as this will behave correctly with `Exception#===`, note we
# monkey patch the backtrace / cause in because these are not public
# api but we need specific values for our fakes.
class FakeException < Exception
def initialize(message, backtrace = [], cause = nil)
super(message)
@backtrace = backtrace
@cause = cause
end
attr_reader :backtrace
if RSpec::Support::RubyFeatures.supports_exception_cause?
attr_accessor :cause
end
end

describe "#fully_formatted" do
Expand All @@ -25,7 +40,7 @@ module RSpec::Core
line_num = __LINE__ + 1
# The failure happened here! Handles encoding too! ЙЦ
end
let(:exception) { instance_double(Exception, :message => "Boom\nBam", :backtrace => [ "#{__FILE__}:#{line_num}"]) }
let(:exception) { FakeException.new("Boom\nBam", [ "#{__FILE__}:#{line_num}"]) }

it "formats the exception with all the normal details" do
expect(presenter.fully_formatted(1)).to eq(<<-EOS.gsub(/^ +\|/, ''))
Expand Down Expand Up @@ -159,16 +174,14 @@ module RSpec::Core
EOS
end

let(:the_exception) { instance_double(Exception, :cause => second_exception, :message => "Boom\nBam", :backtrace => [ "#{__FILE__}:#{line_num}"]) }
let(:the_exception) { FakeException.new("Boom\nBam", [ "#{__FILE__}:#{line_num}"], second_exception) }

let(:second_exception) do
instance_double(Exception, :cause => first_exception, :message => "Second\nexception", :backtrace => ["#{__FILE__}:#{__LINE__}"])
FakeException.new("Second\nexception", ["#{__FILE__}:#{__LINE__}"], first_exception)
end

caused_by_line_num = __LINE__ + 2
let(:first_exception) do
instance_double(Exception, :cause => nil, :message => "Real\nculprit", :backtrace => ["#{__FILE__}:#{__LINE__}"])
end
caused_by_line_num = __LINE__ + 1
let(:first_exception) { FakeException.new("Real\nculprit", ["#{__FILE__}:#{__LINE__}"]) }

it 'includes the first exception that caused the failure', :if => RSpec::Support::RubyFeatures.supports_exception_cause? do
the_presenter = Formatters::ExceptionPresenter.new(the_exception, example)
Expand Down Expand Up @@ -211,8 +224,9 @@ module RSpec::Core

it 'wont produce a stack error when the cause is an older exception', :if => RSpec::Support::RubyFeatures.supports_exception_cause? do
allow(the_exception).to receive(:cause) do
pirj marked this conversation as resolved.
Show resolved Hide resolved
instance_double(Exception, :cause => the_exception, :message => "A loop", :backtrace => the_exception.backtrace)
FakeException.new("A loop", the_exception.backtrace, the_exception)
end

the_presenter = Formatters::ExceptionPresenter.new(the_exception, example)

expect(the_presenter.fully_formatted(1)).to eq(<<-EOS.gsub(/^ +\|/, ''))
Expand All @@ -230,6 +244,22 @@ module RSpec::Core
EOS
end

it 'will work when cause is incorrectly overridden', :if => RSpec::Support::RubyFeatures.supports_exception_cause? do
incorrect_cause_exception = FakeException.new("A badly implemented exception", [], "An incorrect cause")

the_presenter = Formatters::ExceptionPresenter.new(incorrect_cause_exception, example)

expect(the_presenter.fully_formatted(1)).to eq(<<-EOS.gsub(/^ +\|/, ''))
|
| 1) Example
| Failure/Error: Unable to find matching line from backtrace
| A badly implemented exception
| # ------------------
| # --- Caused by: ---
| # A badly implemented exception
EOS
end

it "adds extra failure lines from the example metadata" do
extra_example = example.clone
failure_line = 'http://www.example.com/job_details/123'
Expand Down