Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #5985 (closed defect: fixed)

Opened 2 years ago

Last modified 2 years ago

[PATCH] Tighten Rescues Clauses to Exception or Subclasses

Reported by: james@grayproductions.net Assigned to: David
Priority: low Milestone: 1.x
Component: ActionPack Version: edge
Severity: minor Keywords: tighten, rescue, Exception
Cc:

Description

There are several places in Rails that that rescue Object, but Ruby does not allow raising anything above Exception:

  >> raise Object
  TypeError: exception class/object expected
          from (irb):1:in `raise'
          from (irb):1
          from :0

Note that the error is about my argument to raise there.

I've gone threw and tightened most rescue clauses that used this to the generic rescue Exception. I added comments to these lines, noting the kinds of errors they targetted. In a couple of places, I could see which Exception subclass would be thrown, so I restricted the rescue further, as appropriate.

The idea behind all of this is to only catch the expected errors, allowing unexpected errors to bubble up to higher layers. In areas where all Exceptions are being caught, the goal was just to make the code more self-documenting.

I assumed these methods were covered by tests, and made sure I didn't cause anything to start failing if it use to be passing.

Attachments

tighten_rescues.diff (10.6 kB) - added by james@grayproductions.net on 08/31/06 02:53:07.
Patch to tighten Rails rescue clauses.

Change History

08/31/06 02:53:07 changed by james@grayproductions.net

  • attachment tighten_rescues.diff added.

Patch to tighten Rails rescue clauses.

08/31/06 03:16:28 changed by bitsweat

  • status changed from new to closed.
  • resolution set to fixed.

(In [4885]) Tighten rescue clauses. Closes #5985.

08/31/06 04:05:15 changed by ulysses

Although true, is there any real motivation? For all intensive purposes, Exception/Object are the same in this usage.

08/31/06 04:06:30 changed by ulysses

Oh it's been applied already. :-)

Likewise, there's not any motivation to unapply it, so rock on.