Properly fallback when ERB column can't be computed

[casperisfine]

Fix: #48173

If for some reason the column identification fails, we should fallback to the old error rendering with only line information.

cc @tenderlove

FYI @jasonkim

[obie]

This can't be merged fast enough.

[byroot]

Sorry, I and @tenderlove are travelling right now, so it may take a few days. In the meantime you can point your Gemfile at this branch

[obie]

It’s fine, I put a hotfix in place and it does solve the problem nicely. Safe travels!

…

On May 11, 2023 at 12:37 AM -0600, Jean Boussier ***@***.***>, wrote: Sorry, I and @tenderlove are travelling right now, so it may take a few days. In the meantime you can point your Gemfile at this branch — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: ***@***.***>

[tenderlove]

Looks good, thanks. Though, I wonder why it can't identify the location. I'll merge this, but I think we should find the reason it can't identify the column.

[obie]

Got rid of my hotfix since it had stopped working anyway and this problem is still present in current HEAD 0de4017

Certain kinds of exceptions will get the 500 page in development mode instead of normal exception screen with console. Now with no known hotfix.

Heads up @casperisfine @tenderlove @zarqman

[byroot]

@obie can you share the exception and backtrace? Without it we can't know where the problem might be.

[jpcody]

Update: Shopify/better-html#121 was our issue.

---

I'm not @obie, but I'm seeing the same after attempting to upgrade our app from 7.0.8 to 7.1.3.2 (or 7.1.0 for that matter). Within ActionDispatch::DebugExceptions#render_for_browser_request, the call to create_template fails within the call to ActionView::Template::Handlers::ERB#translate_location.

In my case, backtrace_location.lineno has the wrong line number (it returns 33, and my error is on line 31). Line 33 is the correct source of the error, and line 31 is an empty line. Parsing the empty line fails, and public/500.html renders.

Here's how the template in question is set up. In every failure case, the exception page doesn't render, but it's often for different reasons (failure to tokenize the string, exception in find_offset, different exception in find_offset).

In this scenario, I'm raising exceptions at different spots to see what the source location reports.

<%# boom - line_number: 1, exception page renders %>
<%= render_breadcrumb_header(@breadcrumbs) do %>
  <%# boom - line_number: 2, exception page can't render %>
  <% if policy(Report).create? %>
    <%# boom - line_number: 2, exception page can't render %>
    <%= sidebar_button_to t("reports.new_button"),
      new_selector_reports_path,
      modifier: "light" %>
    <%# boom - line_number: 2, exception page can't render %>
  <% end %>
  <%# boom - line_number: 2, exception page can't render %>
<% end %>
<%# boom - line_number: 15, exception page can't render %>

Interestingly, if I change the sidebar_button_to method call to be a single line, the line numbers become correct, and this entire problem becomes moot for us. The issue is also addressed if I wrap the method call in parens.

I chased down a red herring that might or might not be a problem with tokenizing ERB strings, so I'll tuck it away here…

But whether I pass the contents of line 33 or line 31 to ERB::Util.tokenize, I get a NotImplementedError. The line intended to raise the error is:

# Trigger an i18n error from a line in a top-level `index.html.erb` template
        <h2 class="container__title"><%= t(".ttttemplates_header") %></h2>

And in a console, I get the following results

# Trigger the same exception manually
> ERB::Util.tokenize("        <h2 class=\"container__title\"><%= t(\".ttttemplates_header\") %></h2>")
# NotImplementedError: NotImplementedError
# from /Users/jpcody/.asdf/installs/ruby/3.3.1/lib/ruby/gems/3.3.0/gems/activesupport-7.1.3.2/lib/active_support/core_ext/erb/util.rb:173:in `tokenize'

# Still unsuccessful with a proper reference
> > ERB::Util.tokenize("        <h2 class=\"container__title\"><%= t(\".templates_header\") %></h2>")

# Still unsuccessful with simpler HTML
> ERB::Util.tokenize("<h2><%= t(\".templates_header\") %></h2>")

# Now successful without trailing content
> ERB::Util.tokenize("<h2><%= t(\".templates_header\") %>")

It almost seems like the string scanner in the ERB util can't handle anything after the ERB tags.