Skip calling concat() when exception.annotated_source_code returns ni…

[AnomalousBit]

Summary

This fix is to address an identical problem described in #36719.

I can confirm the more general issue of some exceptions not being logged in the console on development still exists. I've had a couple of issues crop up in the past weeks around sassc compilation errors and today with an ActionView::Template::Error (Multiple files with the same output path cannot be linked) error, with the exceptions never being displayed in the console, leaving me completely stumped as to why my Rails app was returning HTTP 500 / error pages.

In my situation, I was able to step down to https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb#L146 and determine that concat() is throwing a new exception, causing the logging to be completely silenced when exception.annotated_source_code returns nil.

I've self-tested this fix and it now logs the exceptions as one would expect. Hopefully this saves people a lot of time that I've spent tracking down silent errors.

Other Information

I was able to trigger this silent exception issue when a ActionView::Template::Error (Multiple files with the same output path cannot be linked...) exception was thrown. The easiest way I know to do this is make a copy of an existing coffeescript file with another preprocessor extension such as having both files exist: /app/assets/javascripts/file.coffee and /app/assets/javascripts/file.coffee.erb then try loading a page in your browser.

[eugeneius]

It doesn't seem possible for annotated_source_code to return nil since #36532. That fix hasn't been released yet, but it's backported and will be part of 6.0.3.

If you point your Gemfile to the 6-0-stable branch, like this:

gem "rails", git: "https://github.com/rails/rails", branch: "6-0-stable"

Can you still reproduce the error without this change?

[AnomalousBit]

Testing the couple of silent exceptions I've come across recently, the stable branch does fix the issue with annotated_source_code returning nil.

The discussion behind #36532 centers around the changes being defensive, wouldn't this minor change just be another defensive check for the concat() call? No worries about whether merging or not, just thought it might still make sense given the context.

Thanks for taking notice and sharing @eugeneius .

[eugeneius]

I don't think we need an extra level of defence here: if the "silent exceptions" issue resurfaces, it's more likely to be due to an error in a different area of the code that causes the same problem.

[olivierphi]

Just spent a few hours on a bug caused by this same issue, before I find this thread...
With the version v6.0.2.2 of the actionpack gem I was getting a mysterious "error 500" page (the default one for the "production" environment) even though I was in "development" mode.
The log file was not displaying any relevant information either (only a vague "Completed 500 Internal Server Error" message after the ERB rendering message).

Commenting this line (message.concat(exception.annotated_source_code) if exception.respond_to?(:annotated_source_code)) makes it work again, and shows that it was a Sprockets::FileNotFound error that was causing the bug (because the file was renamed) ; and it seems that this line was preventing the proper report in "development" mode somehow - or was not defensive enough to handle unexpected results coming up for .the exception.annotated_source_code call (which is indeed returning nil in this case).

Maybe the extra level of defence here could be useful, as it's really hard to debug when there is absolutelynothing showing up in the HTML page nor the logs even in the "development" environment? 🙂

[kaspth]

@drbenton did you check this? #38820 (comment)