Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exceptions raised from templates should show template code not compiled methods #51090

Open
dhh opened this issue Feb 14, 2024 · 7 comments · May be fixed by #51304
Open

Exceptions raised from templates should show template code not compiled methods #51090

dhh opened this issue Feb 14, 2024 · 7 comments · May be fixed by #51304

Comments

@dhh
Copy link
Member

dhh commented Feb 14, 2024

Seems like we've regressed on exceptions raised from within ERB templates. They're showing the backtrace of the compiled method instead of the underlying template. That's not very helpful.

CleanShot 2024-02-14 at 08 36 52@2x

@rafaelfranca
Copy link
Member

I just tried to reproduce in a new Rails application using Rails on main and could not:

Screenshot 2024-02-14 at 2 35 26 PM

My Ruby version is ruby 3.2.2 (2023-03-30 revision e51014f9c0) [aarch64-linux].

What is your Ruby version?

@davidcsete
Copy link

I just tried to reproduce in a new Rails application using Rails on main and could not:

Screenshot 2024-02-14 at 2 35 26 PM My Ruby version is `ruby 3.2.2 (2023-03-30 revision e51014f9c0) [aarch64-linux]`.

What is your Ruby version?

image

I could reproduce it on an apple silicone using ruby version 3.2.2

@rafaelfranca
Copy link
Member

I just tried on apple silicon and linux the following steps:

rails new --dev my_app
cd my_app
bin/rails g scaffold posts title body
bin/rails db:migrate

Then I edited posts/index.html.erb to add the invalid code, started the server and got the error pointing to the template. I'm also using 3.2.2.

Which steps did you take to reproduce?

@dhh
Copy link
Member Author

dhh commented Feb 14, 2024

Oh this is a strange one. This triggers the busted exception:

<ul>
  <li><%= link_to post.title, post %></li>
</ul>

<%= link_to "New book", new_book_path %>

but this does not:

<ul>
  <li>
    <%= link_to post.title, post %>
  </li>
</ul>

<%= link_to "New book", new_book_path %>

@rafaelfranca
Copy link
Member

aha! I think maybe the compiled template line isn't matching the source line.

@rafaelfranca
Copy link
Member

Just confirmed this also happens on 7.1

@nikhilbhatt
Copy link
Contributor

nikhilbhatt commented Mar 12, 2024

Minimum code to regenerate the issue -->

<li><%= link_to post.title, post %></li>

Even if empty spaces are present after closing braces it will result in showing compiled string.

<%= link_to post.title, post %>  <empty_spaces>

Problem

  1. File- activesupport/lib/active_support/core_ext/erb/util.rb method - tokenize
  2. In this tokenize method we are not handling end text that is present after the code %>
  3. Because of this if empty spaces or text is present after code %> it will result in raise NotImplementedError
  4. And the final output will be the compiled code because this method will return nil.

Solution

  1. Handle the edge case if some text is present at the end in string.
  unless source.eos? || source.exist?(/#{start_re}|#{finish_re}/m)
     tokens << [:TEXT, source.rest]
     source.terminate
  end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants