From 68302c67233b2fb0a08c8ecbbfcff3ab56a99770 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Mon, 6 Jan 2025 14:32:31 -0500 Subject: [PATCH] fix: ArgumentErrors raised during template rendering 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 --- actionview/CHANGELOG.md | 18 ++++++++++++++++++ actionview/lib/action_view/base.rb | 15 ++++++--------- actionview/lib/action_view/template/error.rb | 11 +++++++++++ actionview/test/template/template_test.rb | 16 ++++++++++++++-- 4 files changed, 49 insertions(+), 11 deletions(-) diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index ea0d79575ef62..d0b636c670daa 100644 --- a/actionview/CHANGELOG.md +++ b/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. diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb index 38ea8ddfb9d8b..59ae9bd8d16a4 100644 --- a/actionview/lib/action_view/base.rb +++ b/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) diff --git a/actionview/lib/action_view/template/error.rb b/actionview/lib/action_view/template/error.rb index b7d522a801b76..3bb757bb819b4 100644 --- a/actionview/lib/action_view/template/error.rb +++ b/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 diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb index 2accd4aeb4357..f937043953946 100644 --- a/actionview/test/template/template_test.rb +++ b/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