From 5c2eeaea6830091a3bfc8112d05805f1615fa4e5 Mon Sep 17 00:00:00 2001 From: Robin Daugherty Date: Tue, 15 Sep 2020 18:03:33 -0400 Subject: [PATCH] Show real cause of ActionView::Template::Error --- lib/better_errors/raised_exception.rb | 18 ++++-- spec/better_errors/raised_exception_spec.rb | 67 +++++++++++---------- 2 files changed, 48 insertions(+), 37 deletions(-) diff --git a/lib/better_errors/raised_exception.rb b/lib/better_errors/raised_exception.rb index 63eae39e..f97ffee3 100644 --- a/lib/better_errors/raised_exception.rb +++ b/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.to_s == "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, "")) diff --git a/spec/better_errors/raised_exception_spec.rb b/spec/better_errors/raised_exception_spec.rb index 99bc8218..e5fe6837 100644 --- a/spec/better_errors/raised_exception_spec.rb +++ b/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 #") + } + + 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 #") - } - - its(:message) { is_expected.to eq "undefined method `something!' for #" } - 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 #") + # } + # + # its(:message) { is_expected.to eq "undefined method `something!' for #" } + # + # 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))