Feature Request: Do not allow to give Exception to rescue_from in controllers or at least show warning

When building dynamic error pages, people (specially beginners) always write rescue_from(Exception, …) in ApplicationController. You can see it (or something similar) is suggested on Stackoverflow and even RailsCasts. However, doing so has serious side-effects such as listed below because it halts the error chain:

  • No longer able to see nice exception details during development

  • gems like exception_notification and airbrake stops working

In addition, I’ve never seen any good use-cases of it in controllers. So I think it only gives us serious issues.

And here’s what I propose:

  • ActiveSupport::Rescuable remains the same

  • ActionController::Rescue overrides rescue_from and change it to not accept Exception(raise some exception or output warning)

Since rescue_from is a class method, we can be notified when the app spins up or re-loads everything during development, not run time. What do you think?

Yuki Nishijima

When building dynamic error pages, people (specially beginners) always write rescue_from(Exception, …) in ApplicationController. You can see it (or something similar) is suggested on Stackoverflow and even RailsCasts.

The RailsCasts example is framed as “you really shouldn’t do this” with explanatory text as to why it’s a bad thing to do.

The StackOverflow example is explicitly attempting to construct a general exception notifier - a replacement for exception_notification and airbrake.

However, doing so has serious side-effects such as listed below because it halts the error chain:

  • No longer able to see nice exception details during development
  • gems like exception_notification and airbrake stops working

In addition, I’ve never seen any good use-cases of it in controllers. So I think it only gives us serious issues.

And here’s what I propose:

  • ActiveSupport::Rescuable remains the same
  • ActionController::Rescue overrides rescue_from and change it to not accept Exception(raise some exception or output warning)

Rescuing Exception is not the best practice, but there are reasons that somebody might want to do it. We can’t save people from doing silly-but-potentially-valid things.

—Matt Jones

Rescuing Exception is not the best practice, but there are reasons that somebody might want to do it. We can’t save people from doing silly-but-potentially-valid things.

I agree, but think a warning would be appropriate.

-Amiel

Make it a warning, and let people turn the warning off if they actually need it for some reason (though really, I can’t think of a valid use case).

Cheers,

-foca