Merge pull request #477 from BetterErrors/fix/rails-6-action-view-tem…

…plate-error

Show real cause of ActionView::Template::Error with Rails 6

lib/better_errors/raised_exception.rb

@@ -4,8 +4,18 @@ class RaisedException
     attr_reader :exception, :message, :backtrace
 
     def initialize(exception)
-      if exception.respond_to?(:original_exception) && exception.original_exception
-        # This supports some specific Rails exceptions, and is not intended to act the same as `#cause`.
+      if exception.class.name == "ActionView::Template::Error" && exception.respond_to?(:cause)
+        # Rails 6+ exceptions of this type wrap the "real" exception, and the real exception
+        # is actually more useful than the ActionView-provided wrapper. Once Better Errors
+        # supports showing all exceptions in the cause stack, this should go away. Or perhaps
+        # this can be changed to provide guidance by showing the second error in the cause stack
+        # under this condition.
+        exception = exception.cause if exception.cause
+      elsif exception.respond_to?(:original_exception) && exception.original_exception
+        # This supports some specific Rails exceptions, and this is not intended to act the same as
+        # the Ruby's {Exception#cause}.
+        # It's possible this should only support ActionView::Template::Error, but by not changing
+        # this we're preserving longstanding behavior of Better Errors with Rails < 6.
         exception = exception.original_exception
       end
 
@@ -57,10 +67,6 @@ def setup_backtrace_from_backtrace
 
     def massage_syntax_error
       case exception.class.to_s
-      when "ActionView::Template::Error"
-        if exception.respond_to?(:file_name) && exception.respond_to?(:line_number)
-          backtrace.unshift(StackFrame.new(exception.file_name, exception.line_number.to_i, "view template"))
-        end
       when "Haml::SyntaxError", "Sprockets::Coffeelint::Error"
         if /\A(.+?):(\d+)/ =~ exception.backtrace.first
           backtrace.unshift(StackFrame.new($1, $2.to_i, ""))

spec/better_errors/raised_exception_spec.rb

@@ -10,12 +10,32 @@ module BetterErrors
     its(:message)   { is_expected.to eq "whoops" }
     its(:type)      { is_expected.to eq RuntimeError }
 
-    context "when the exception wraps another exception" do
+    context 'when the exception is an ActionView::Template::Error that responds to #cause (Rails 6+)' do
+      before do
+        stub_const(
+          "ActionView::Template::Error",
+          Class.new(StandardError) do
+            def cause
+              RuntimeError.new("something went wrong!")
+            end
+          end
+        )
+      end
+      let(:exception) {
+        ActionView::Template::Error.new("undefined method `something!' for #<Class:0x00deadbeef>")
+      }
+
+      its(:message) { is_expected.to eq "something went wrong!" }
+      its(:type) { is_expected.to eq RuntimeError }
+    end
+
+    context 'when the exception is a Rails < 6 exception that has an #original_exception' do
       let(:original_exception) { RuntimeError.new("something went wrong!") }
       let(:exception) { double(:original_exception => original_exception) }
 
       its(:exception) { is_expected.to eq original_exception }
-      its(:message)   { is_expected.to eq "something went wrong!" }
+      its(:message) { is_expected.to eq "something went wrong!" }
+      its(:type) { is_expected.to eq RuntimeError }
     end
 
     context "when the exception is a SyntaxError" do
@@ -50,35 +70,20 @@ module BetterErrors
       end
     end
 
-    context "when the exception is an ActionView::Template::Error" do
-      before do
-        stub_const(
-          "ActionView::Template::Error",
-          Class.new(StandardError) do
-            def file_name
-              "app/views/foo/bar.haml"
-            end
-
-            def line_number
-              42
-            end
-          end
-        )
-      end
-
-      let(:exception) {
-        ActionView::Template::Error.new("undefined method `something!' for #<Class:0x00deadbeef>")
-      }
-
-      its(:message) { is_expected.to eq "undefined method `something!' for #<Class:0x00deadbeef>" }
-      its(:type)    { is_expected.to eq ActionView::Template::Error }
-
-      it "has the right filename and line number in the backtrace" do
-        expect(subject.backtrace.first.filename).to eq("app/views/foo/bar.haml")
-        expect(subject.backtrace.first.line).to eq(42)
-      end
-    end
-
+    # context "when the exception is an ActionView::Template::Error" do
+    #
+    #   let(:exception) {
+    #     ActionView::Template::Error.new("undefined method `something!' for #<Class:0x00deadbeef>")
+    #   }
+    #
+    #   its(:message) { is_expected.to eq "undefined method `something!' for #<Class:0x00deadbeef>" }
+    #
+    #   it "has the right filename and line number in the backtrace" do
+    #     expect(subject.backtrace.first.filename).to eq("app/views/foo/bar.haml")
+    #     expect(subject.backtrace.first.line).to eq(42)
+    #   end
+    # end
+    #
     context "when the exception is a Coffeelint syntax error" do
       before do
         stub_const("Sprockets::Coffeelint::Error", Class.new(SyntaxError))