Merge pull request rails#54133 from flavorjones/flavorjones-strict-locals-exception-backtrace

fix: `ArgumentError`s raised during template rendering

Author: rafaelfranca

actionview/CHANGELOG.md

@@ -1,3 +1,21 @@
+*   Argument errors related to strict locals in templates now raise an
+    `ActionView::StrictLocalsError`, and all other argument errors are reraised as-is.
+
+    Previously, any `ArgumentError` raised during template rendering was swallowed during strict
+    local error handling, so that an `ArgumentError` unrelated to strict locals (e.g., a helper
+    method invoked with incorrect arguments) would be replaced by a similar `ArgumentError` with an
+    unrelated backtrace, making it difficult to debug templates.
+
+    Now, any `ArgumentError` unrelated to strict locals is reraised, preserving the original
+    backtrace for developers.
+
+    Also note that `ActionView::StrictLocalsError` is a subclass of `ArgumentError`, so any existing
+    code that rescues `ArgumentError` will continue to work.
+
+    Fixes #52227.
+
+    *Mike Dalessio*
+
 *   Improve error highlighting of multi-line methods in ERB templates or
     templates where the error occurs within a do-end block.
 

actionview/lib/action_view/base.rb

@@ -267,15 +267,12 @@ def _run(method, template, locals, buffer, add_to_stack: true, has_strict_locals
         begin
           public_send(method, locals, buffer, **locals, &block)
         rescue ArgumentError => argument_error
-          raise(
-            ArgumentError,
-            argument_error.
-              message.
-                gsub("unknown keyword:", "unknown local:").
-                gsub("missing keyword:", "missing local:").
-                gsub("no keywords accepted", "no locals accepted").
-                concat(" for #{@current_template.short_identifier}")
-          )
+          public_send_line = __LINE__ - 2
+          frame = argument_error.backtrace_locations[1]
+          if frame.path == __FILE__ && frame.lineno == public_send_line
+            raise StrictLocalsError.new(argument_error, @current_template)
+          end
+          raise
         end
       else
         public_send(method, locals, buffer, &block)

actionview/lib/action_view/template/error.rb

@@ -27,6 +27,17 @@ def message
     end
   end
 
+  class StrictLocalsError < ArgumentError # :nodoc:
+    def initialize(argument_error, template)
+      message = argument_error.message.
+                  gsub("unknown keyword:", "unknown local:").
+                  gsub("missing keyword:", "missing local:").
+                  gsub("no keywords accepted", "no locals accepted").
+                  concat(" for #{template.short_identifier}")
+      super(message)
+    end
+  end
+
   class MissingTemplate < ActionViewError # :nodoc:
     attr_reader :path, :paths, :prefixes, :partial
 

actionview/test/template/template_test.rb

@@ -220,22 +220,34 @@ def test_default_locals_can_be_specified
     assert_equal "Hello", render
   end
 
-  def test_required_locals_can_be_specified
+  def test_required_locals_must_be_specified
     error = assert_raises(ActionView::Template::Error) do
       @template = new_template("<%# locals: (message:) -%>")
       render
     end
 
     assert_match(/missing local: :message for hello template/, error.message)
+    assert_instance_of ActionView::StrictLocalsError, error.cause
   end
 
-  def test_extra_locals_raises_error
+  def test_extra_locals_raises_strict_locals_error
     error = assert_raises(ActionView::Template::Error) do
       @template = new_template("<%# locals: (message:) -%>")
       render(message: "Hi", foo: "bar")
     end
 
     assert_match(/unknown local: :foo for hello template/, error.message)
+    assert_instance_of ActionView::StrictLocalsError, error.cause
+  end
+
+  def test_argument_error_in_the_template_is_not_hijacked_by_strict_locals_checking
+    error = assert_raises(ActionView::Template::Error) do
+      @template = new_template("<%# locals: () -%>\n<%= hello(:invalid_argument) %>")
+      render
+    end
+
+    assert_match(/in ['`]hello'/, error.backtrace.first)
+    assert_instance_of ArgumentError, error.cause
   end
 
   def test_rails_injected_locals_does_not_raise_error_if_not_passed