Trusting in Exceptions To Do What They Do

Nobody likes to see exceptions when running an application; it means something went wrong and you should have been on top of it. But with that, often times I find people will write code to prevent exceptions rather than just allowing them to occur and then using it in their favor.

Take this bit of ruby code in a rails controller as an example:

class ArticlesController < ApplicationController
  def show
    @article = Article.find(params[:id])
  end
end

That's pretty solid, but what happens if someone passes in an ID that does not exist? Well in rails' production mode, it would return a 404 error.

Makes sense! That article doesn't exist, so it can't be found. I have dabbled with a little SEO and usability and just a 404 isn't cutting it for me. I think we can redirect our user back on a right track. For one, we know the user is trying to read an article, so why not redirect the user to a list of all the articles and give them a little warning?

Solution One

First check that the article exists, and if not, redirect the user to the index page.

def show
  if Article.exists?(params[:id])
    @article = Article.find(params[:id])
  else
    flash[:error] = 'That article does not exist'
    redirect_to :action => 'index'
  end
end

While this works, this requires two queries to our database; one to check it's existence and one to load it. I think there is a better way, and its secret is to go with the flow of the code.

Solution Two

One thing ActiveRecord::Base#find does do is raise an exception when it cannot find anything, an ActiveRecord::RecordNotFound exception. We can use this bit of knowledge to re-write that method to something a little simpler by trusting it to throw an exception and then for us to rescue it.

def show
  @article = Article.find(params[:id])
rescue ActiveRecord::RecordNotFound
  flash[:error] = 'That article does not exist'
  redirect_to :action => 'index'
end

One call to the database and fewer lines of code (not a lot, but still). This time, that nasty little error has been used in our favor; but one mustn't get lazy when using rescue statements.

Don't Get Lazy

Notice that I used the constant ActiveRecord::RecordNotFound with the statement. Sure, I could have just written rescue, but that hurts us in two ways. First, it becomes less readable without it. Having the RecordNotFound is pretty obvious as to the reason we are executing that statement. Secondly, it can introduce unwanted side effects.

def show
  @article = Article.find(params[:id])
  some_illegal_operation
rescue
  flash[:error] = 'That article does not exist'
  redirect_to :action => 'index'
end

Whenever that method is called, it will always redirect the user to the index page with that error message. Because we've rescued all errors, we're not getting any notice about how our some_illegal_operation has caused us grief. This makes debugging and management harder.

The lesson being, don't be lazy in those rescues. Try to avoid a blanket rescue statement if you can.