Skip to content

Commit

Permalink
Addressed an issue where syntax errors generated inside an eval
Browse files Browse the repository at this point in the history
method cause an Internal Server Error due to a TypeError.

In order to display the extraced source of a syntax error, we try
to locate the node id for the backtrace location. The call to
RubyVM::AbstractSyntaxTree.node_id_for_backtrace_location is expecting
a Thread::Backtrace::Location object, but we are passing a
SourceMapLocation instead.

This commit does two things:
1) it addresses the issue by making sure that we are always passing
a Thread::Backtrace::Location instead
2) it allows the development view to show the extracted source fragment
  • Loading branch information
Mario Caropreso committed Nov 26, 2023
1 parent 35a614c commit a92ce81
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 3 deletions.
Expand Up @@ -248,7 +248,7 @@ def initialize(location, template)
end

def spot(exc)
if RubyVM::AbstractSyntaxTree.respond_to?(:node_id_for_backtrace_location)
if RubyVM::AbstractSyntaxTree.respond_to?(:node_id_for_backtrace_location) && __getobj__.is_a?(Thread::Backtrace::Location)
location = @template.spot(__getobj__)
else
location = super
Expand All @@ -273,7 +273,12 @@ def build_backtrace

(@exception.backtrace_locations || []).map do |loc|
if built_methods.key?(loc.label.to_s)
SourceMapLocation.new(loc, built_methods[loc.label.to_s])
thread_backtrace_location = if loc.respond_to?(:__getobj__)
loc.__getobj__
else
loc
end
SourceMapLocation.new(thread_backtrace_location, built_methods[loc.label.to_s])
else
loc
end
Expand Down
14 changes: 14 additions & 0 deletions actionpack/test/dispatch/exception_wrapper_test.rb
Expand Up @@ -69,6 +69,20 @@ def backtrace
end
end

class_eval "def throw_syntax_error; eval %(
'abc' + pluralize 'def'
); end", "lib/file.rb", 42

test "#source_extracts works with eval syntax error" do
exception = begin throw_syntax_error; rescue SyntaxError => ex; ex; end

wrapper = ExceptionWrapper.new(nil, TopErrorProxy.new(exception, 1))

assert_called_with(wrapper, :source_fragment, ["lib/file.rb", 42], returns: "foo") do
assert_equal [ code: "foo", line_number: 42 ], wrapper.source_extracts
end
end

if defined?(ErrorHighlight) && Gem::Version.new(ErrorHighlight::VERSION) >= Gem::Version.new("0.4.0")
test "#source_extracts works with error_highlight" do
lineno = __LINE__
Expand Down
11 changes: 10 additions & 1 deletion activesupport/lib/active_support/syntax_error_proxy.rb
Expand Up @@ -43,7 +43,16 @@ def backtrace_locations

private
def parse_message_for_trace
__getobj__.to_s.split("\n")
if __getobj__.to_s.start_with?("(eval")
# If the exception is coming from a call to eval, we need to keep
# the path of the file in which eval was called to ensure we can
# return the right source fragment to show the location of the
# error
location = __getobj__.backtrace_locations[0]
["#{location.path}:#{location.lineno}: #{__getobj__}"]
else
__getobj__.to_s.split("\n")
end
end
end
end

0 comments on commit a92ce81

Please sign in to comment.