Which is the proper order to handle exception.cause in ActiveSupport::Rescuable#rescue_from ?

[rinmu]

Summary

Since Rails 5.0, the order in which handler is applied for exceptions including cause has been changed.
It looks like a deliberate change as written in the #25018 comment, but I think there are at least the following problems.

Probrem

class ApplicationController < ActionController::Base
  protect_from_forgery with: :exception

  rescue_from StandardError, with: :handle_500
  rescue_from ActiveRecord::RecordNotFound, with: :handle_404

  private

  def handle_500(error)
    render 'errors/error', status: 500
  end

  def handle_404(error)
    render 'errors/error', status: 404
  end
end

<%= Article.find(10) %>

if Article with id==10 does not exist, handle_ 404 was executed in Rails 4.2.8,
Since Rails 5.0.0, handle_ 500 is now executed.

Errors occurring in view are wrapped in ActionView::Template::Error.
In Rails 4.2.8, when error has cause, It looked for a handler that matched exception.cause.
For Rails 5.0.0 and later, the exception.cause check is done only if the wrapping error does not match any handler.

I think the behavior before 4.2.8 is preferable in the above case.

I know that shouldn't do rescue_from StandardError be listed in RailsGuide.
This may not be an appropriate example.
However, I think that we always encounter this problem when we need to reference cause and want to deal with special errors and generic errors in order.

Solution

How about restoring the logic of #24158 to activesupport/lib/active_support/rescuable.rb ?

System configuration

Rails version:
4.2.8, 5.0.0, 5.1.2

Ruby version:
2.3.1

[TurtleKitty]

I'm having the same trouble with nested exceptions. We upgraded to Rails 5 last week; this was a surprising change. We're running 5.0.4 under Ruby 2.4.1. Upgrading to 5.1.2 seemed to have no effect.

[matthewd]

From #25018 (comment):

If you know you have a particular exception class that implements the "generic wrapper" pattern, you can write a handler for it which eagerly tries to handle the cause first

ActionView::Template::Error is a good example of that pattern; we should probably be including such a "handle the cause instead" handler specifically for it.

So, the general change was indeed deliberate. But in the specific case of ActionView::Template::Error -- around which the previous behaviour was designed, I think -- we do want different something different:

• If the application has explicitly defined a handler for ActionView::Template::Error, that should of course be invoked.
• But otherwise, it should hit a specific handler inside the framework, which will unwrap and focus on the cause.

AFAICS, this situation will only manifest if there's a more generic handler defined that catches ActionView::Template::Error -- which boils down to a rescue_from for one of 😕 StandardError, 😱 Exception, or ☠️☠️☠️ Object/Kernel. As you noted, that's explicitly called out in the guide as something that "will cause serious side-effects"... so that's probably why no-one else has complained.

If you're up for a PR to do the above, it sounds like a reasonable change.

---

@TurtleKitty how closely does your situation match the one @rinmu described?

[TurtleKitty]

Fairly similar. I have some code that did rescue_from Timeout::Error; this worked fine under Rails 4.* and broke when we moved to 5.0. It no longer recognizes Timeout::Error exceptions when wrapped in an ActionView::Template::Error.

I've had to write a new handler that rescues from StandardError and then recurses through the exception causes to find the real reason.

[rinmu]

Thanks for your comments. Please check PR #30601.

[rails-bot]

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 5-1-stable branch or on master, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

[matpowel]

For those who land here in 2020+, this is a super annoying issue in Ruby 2.1+ / Rails 5+ when you have a structured set of custom exceptions in your app and a fallback to catching StandardError in Application controller.

Here is our solution:

  rescue_from StandardError do |exception|
    if exception.cause
      rescue_with_handler(exception.cause)
    else
      ...
    end
  end