#896 Be a little more discerning about which exceptions to handle - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#896 ✓committed

Be a little more discerning about which exceptions to handle

Reported by Matt Palmer | August 25th, 2008 @ 07:43 AM | in 3.x

Throughout the Rails source, it is common to rescue Exception, however this isn't a particularly good idea as various quite fatal exceptions don't get a chance to do their thing (NoMemoryError, for one, although SignalException, SystemExit, and SystemStackError all signify something a little more deadly than something mundane like an ArgumentError).

I've prepared a patch that goes through and "fixes" the occurances of "rescue Exception" to something slightly more reasonable. The fixes were usually either just s/Exception/StandardError/, with the occasional more complicated fix. The few "rescue Exception" blocks I left were ones that just added some useful info then re-raised.

My changes are in the exception_happiness branch of git://github.com/mpalmer/rails.git.

Comments and changes to this ticket

Pratik August 30th, 2008 @ 05:55 PM
That sounds good. Could you please update a .diff file using git-format-patch ?

Thanks.
DHH September 10th, 2008 @ 05:49 AM
- State changed from “new” to “incomplete”
Change to open when patch is attached.
Jeremy Kemper May 4th, 2010 @ 06:48 PM
- Milestone changed from “2.x” to “3.x”
[bulk edit]
You flagged this item as spam.
acechase May 27th, 2010 @ 07:23 PM
Hey guys,

Perhaps I should open another ticket for this, please let me know if that's the case. I have a patch that addresses what is for me the most egregious case of overeager exception handling. In activerecord/lib/active_record/connection_adapters/abstract_adapter.rb the #log method rescues Exception and then re-raises as an ActiveRecord::StatementInvalid exception.

Some people have hit this problem when they have slow queries that are hanging the system and so I think the response is generally "fix your query". But we're hitting this problem when we try to exit out of our background processes that are just cranking through a whole bunch of data processing.

In these background processes we have designed them to be tolerant to exceptions and expect the process to die when it's sent a SIGINT or SystemExit. However, we also want those processes to stay alive and continue on to the next job in the queue if any particular job hits an unexpected exception. So we take the approach of wrapping our job invocations in a 'rescue Exception' block and re-raising (allowing the process to die) in the case where the exception is a SystemExit or SignalException. So the AR approach of rescuing and re-raising as a StatementInvalid breaks this approach and leaves us in a situation where we have to do resort to less reliable methods of inspecting the rescued exceptions, like parsing the exception message. It also leads to confusing behavior where you sometimes see the process die cleanly (when it's not invoking an AR statement at the time the SystemExit is received) or continue running (when the exception was caught and re-wrapped).

I'm guessing there are other folks out there how are experiencing similar problems and so I'd like to offer a patch that applies against the 2-3-stable branch (rather than 3.x).
- system_exception_no_wrap.diff 1.3 KB
Repository May 29th, 2010 @ 05:01 AM
- State changed from “incomplete” to “committed”
(from [3d6ed501870f2a790c2d0e288e0f294e8c5f7be3]) Don't rewrap system level exceptions with StatementInvalid

Signed-off-by: Michael Koziarski michael@koziarski.com
[#896 state:committed] http://github.com/rails/rails/commit/3d6ed501870f2a790c2d0e288e0f29...