Merge pull request rspec#2703 from rspec/ensure-exception-presenter-i…

…gnores-invalid-cause

Prevent invalid cause from breaking exception presenter

Changelog.md

@@ -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)

lib/rspec/core/formatters/exception_presenter.rb

@@ -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
@@ -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
@@ -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

spec/rspec/core/formatters/exception_presenter_spec.rb

@@ -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
@@ -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(/^ +\|/, ''))
@@ -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)
@@ -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
-          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(/^ +\|/, ''))
@@ -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'