[Rails-core] 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 http://stackoverflow.com/questions/5331008/dynamic-error-pages-in-rails-3 and even RailsCasts http://railscasts.com/episodes/53-handling-exceptions-revised. 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 -- You received this message because you are subscribed to the Google Groups Ruby on Rails: Core group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscr...@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout.
Re: [Rails-core] Feature Request: Do not allow to give Exception to rescue_from in controllers or at least show warning
On Jun 23, 2014, at 9:48 AM, Yuki Nishijima m...@yukinishijima.net wrote: 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 signature.asc Description: Message signed with OpenPGP using GPGMail
Re: [Rails-core] Feature Request: Do not allow to give Exception to rescue_from in controllers or at least show 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. I agree, but think a warning would be appropriate. -Amiel On Mon, Jun 23, 2014 at 4:03 PM, Matt Jones al2o...@gmail.com wrote: On Jun 23, 2014, at 9:48 AM, Yuki Nishijima m...@yukinishijima.net wrote: 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 http://stackoverflow.com/questions/5331008/dynamic-error-pages-in-rails-3 and even RailsCasts http://railscasts.com/episodes/53-handling-exceptions-revised. 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 -- You received this message because you are subscribed to the Google Groups Ruby on Rails: Core group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscr...@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout.
Re: [Rails-core] Feature Request: Do not allow to give Exception to rescue_from in controllers or at least show warning
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 On Mon, Jun 23, 2014 at 7:54 PM, Amiel Martin am...@carnesmedia.com wrote: 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 On Mon, Jun 23, 2014 at 4:03 PM, Matt Jones al2o...@gmail.com wrote: On Jun 23, 2014, at 9:48 AM, Yuki Nishijima m...@yukinishijima.net wrote: 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 http://stackoverflow.com/questions/5331008/dynamic-error-pages-in-rails-3 and even RailsCasts http://railscasts.com/episodes/53-handling-exceptions-revised. 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 -- You received this message because you are subscribed to the Google Groups Ruby on Rails: Core group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscr...@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups Ruby on Rails: Core group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscr...@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout.
