Effectively using rescue_from

Norbert Crombach, 03 Nov 2007, 15:10 in ruby on rails, last updated 04 Apr 2009, 09:25 (edit).

This article is left here for historical purposes, please read the comments carefully after you’ve read the article.

Even though Ryan Daigle already covered it in his usual timely fashion, I’d like to share some real life examples of how we use the relatively new rescue_from functionality.

Because I usually prefer methods that raise exceptions instead of returning a boolean, like, say, save! instead of just save, I thought it would be nice if I could deal with some common exceptions on a higher level. This lead me to write a Rails patch based on the exception_handler plugin, which I previously used in some projects.

A few weeks later the patch found its way into Rails core, just in time for the 2.0 preview release. Now everybody can rewrite this:

class PostsController < ApplicationController
  def create
    @post = Post.create!(params[:post])
  rescue ActiveRecord::RecordInvalid
    render :action => :new
  end

  def update
    @post = Post.find(params[:id])
    @post.update_attributes!(params[:post])
  rescue ActiveRecord::RecordInvalid
    render :action => :edit
  end
end

to something like this:

class ApplicationController < ActionController::Base
  rescue_from ActiveRecord::RecordInvalid do |exception|
    render :action => (exception.record.new_record? ? :new : :edit)
  end
end

class PostsController < ApplicationController
  def create
    @post = Post.create!(params[:post])
  end
  
  def update
    @post = Post.find(params[:id])
    @post.update_attributes!(params[:post])
  end
end

And it just works.

Comments

Remco about 24 hours later: (delete)

The update action rendering an edit, in case of an exception, will not have @post properly set. ¶
Norbert Crombach 1 day later: (delete)

Good catch. ¶
jare care 1 day later: (delete | show email)

This example is terrible.

First off remember this:

"exceptions should not be expected"

Your tendency of avoiding boolean methods like #save in favor of exception raising methods like #save! is conveying the message that invalid data is something you do NOT expect.

That is wrong. Of course you expect invalid data, therefore that should NOT be handled using exceptions but instead using boolean methods and conditional logic.

Your use of the ternary operator in your #rescue_from block is an example of DRYness going too far. The more "DRY" you try to keep that controller the more conditional logic you're going to have to add in your #rescue_from block. Thats going to get ugly real quick.

For more info check out http://giantrobots.thoughtbot.com/2007/9/26/active-record-programming-with-exceptions ¶
Manfred Stienstra 2 days later: (delete)

We've been having some discussion here at the office about how to properly structure a controller these last days. My (and Thijs') argument against this kind of logic is that it doesn't read like a story any more because you scatter the logic throughout the code. Remember GOSUB?

Norbert has been working on a way to get something like this working without loosing readability. ¶
Bala Paranj 195 days later: (delete | show email)

How will you set the flash message? ¶
Gabe da Silveira 517 days later: (delete | show email)

Ah, so this is where this idea came from. I have this in the current project I'm working on. Aside from what Jare Care said being exactly right on principal, I have painful experience of how braindead this is in practice:

* First, for new developers coming in they just see object.save! and have to track down what actually happens in an unexpected place. This part wouldn't be so bad if it actually provided a net benefit in other ways. However it doesn't because:
* You now have to decide how every single CRUD action responds in a generic fashion. Why not just build your app with scaffold and remove all customization while you're at it?
* But the absolutely terrible mind-blowingly obnoxious showstopper is that now you've neutered the save!, create! and update! methods so they can no longer as intended (ie. to raise an exception if an object _should_ be valid but isn't).

This is the perfect example of Ruby devs building magic solutions for aesthetic reasons at the expense of readability, flexibility and maintainability. There is no reason you can't implement this exact architecture with a regular method other than the fact that you don't like booleans (and even that could be avoided thanks to the semantics of Proc.new vs lambda).

Sorry to be so harsh, but I now have to spend hours extracting this from 20+ controllers because our previous developer thought this would be a cool way to DRY up the CRUD. ¶
Manfred Stienstra 517 days later: (delete)

Gabe, this article was left here for historical purposes. I wouldn't recommend anyone to _ever_ implement controllers like this.

I'll add a short disclaimer. ¶
Norbert Crombach 517 days later: (delete | show email)

For the record, I didn't write controllers this way for long.

I still use rescue_from though. ¶
Steve Yan 523 days later: (delete)

Norbert or Gabe,

Can you clarify what you mean by this not being a best practice? It would seem that in the case of many exceptions that apply across controllers, this is an elegant way to handle exceptions for all. For example, I'm writing a web service that only deals with XML and JSON responses. For any event where the caller is attempting to create a record that already exists, I will always return a 409 with an XML error message. Each controller raises its own Error, and I rescue the error inside ApplicationController with a render_409 method.

I can see how for cases where an exception means different things in different contexts, it can be limiting. But I also see its usefulness. I wanted to pick your brains about whether you think this is not acceptable at all or for only this specific example. ¶
Manfred Stienstra 523 days later: (delete)

There are a few good reasons why you don't want to use exceptions in this case, the first is the general rule "exceptions should not be expected". Exceptions should be used in case something unexpected happens, for instance when a database connection can't be made because the database server is down.

Sometimes you find this wisdom in a slightly different form: "don't use exceptions for program flow". Programmers learned this way back in the days when they used jumps or 'GOTO' to jump from one part of a program to another. This makes it hard to find entry points for certain pieces of code, thus making it harder to read resulting in more bugs and less maintainable code.

I'm sure there are tons of other reasons, but let me finish with a final Ruby specific reason: generating a stack trace takes quite a long time and you don't want to do this in mid execution. ¶
Steve Yan 523 days later: (delete)

Thanks for the explanation! I'll have to reconsider my current approach given this reasoning... ¶