October 22nd 2010

Throw It!

How I'm using Ruby exceptions and Rails' rescue_from to keep controllers clean and error response handling DRY and RESTful.

Quite often in a Rails app we'd do something like this in a controller that needs to fetch a record from database:

class PostsController < ApplicationController
  before_filter :find_post, :except => [:index, :new, :create]
  before_filter :access_permitted?, :except => [:index, :new, :create]
  
  # Actions...
  
  private
  
  # Retrieves the post, redirecting back to index on failure with a custom message
  def find_post
    @post = Post.find(params[:id])
  rescue ActiveRecord::NotFound => err
    flash[:error] = "Could not find post with id #{params[:id]}"
    redirect_to posts_path
  end
  
  # Makes sure the current user is allowed to access this post.
  # Redirect back to index when auth fails
  def access_permitted?
    unless @post.user == current_user
      flash[:message] = "You are not allowed to access this post!"
      redirect_to posts_path
    end
  end
end

In my opinion, this has two problems:

  1. The code is not DRY
  2. The response is not RESTful

Let me explain:

All across your app, you will have similar functionality. You might have a UsersController and a CommentsController, and all of those need to find an instance, authorize the user and so on.

All this functionality needs separate error handling code when going down the path of redirecting the frontend user to another page with custom error messages, which can result in a lot of (mostly) duplicate code, where only the error message and endpoint url vary. When you want to change the error handling for records that could not be found, you have to do it across all your controllers over and over again.

Also, this (simple) example does not cover the handling of other formats than HTML. If you had to deal with, say, JSON requests, you'd also have to introduce respond_to blocks over and over again. Imagine you need some additional auth in a certain action (users may edit a wiki page but are not allowed to destroy it) - even more failure response code is required.

Additionally, this approach is not RESTful: A resource that is missing should not respond with a HTTP redirect, which actually says that either the resource was Moved Permanently (301) to a different or was Found (302) at a temporarily different url (see RFC2616) (the Rails default for redirect_to being 302).

After all handling auth or database retrieval problems is handling of exceptional cases in a Rails app's regular flow, so why not use exceptions?

Rails comes with a built in ActionController class method called rescue_from which, from what I've seen so far, is not as popular or widely used as it should be. Basically it intercepts exceptions at the top level of the request (before the infamous 'There seems to be a problem' 500.html page is rendered) and lets you handle them the way you want.

Let's have a look at how this could be implemented in the ApplicationController for this app.

class ApplicationController < ActionController::Base
  # Just define an example no permission exception for auth. 
  # Might be thrown by authorize before filter on fail.
  class NoPermissionError < StandardError; end;

  rescue_from ApplicationController::NoPermissionError do |e|
    response_for_error 403
  end
  
  rescue_from ActiveRecord::NotFound do |e|
    response_for_error 404
  end
  
  private
  
  def response_for_error(status)
    respond_to do |f|
      # Show a neat html page inside the app's layout for web users
      f.html { render :template => "errors/#{status}", :status => status }

      # Everything else (JSON, XML, YAML, Whatnot) gets a blank page with status
      # which can then be understood and processed by the API client, 
      # JavaScript library (on Ajax) etc.
      f.all { render :nothing => true, :status => status }
    end
  end
end

Of course, the status-specific templates must be added inside the app/views/errors/ directory for the response_for_error method to work properly. This is left for the reader as an excercise ;)

The cleaned up PostsController would look somewhat like this:

class PostsController < ApplicationController
  before_filter :find_post, :except => [:index, :new, :create]
  before_filter :access_permitted?, :except => [:index, :new, :create]
  
  # Actions...
  
  private
  
  def find_post
    @post = Post.find(params[:id])
  end
  
  def access_permitted?
    raise ApplicationController::NoPermissionError unless @post.user == current_user
  end
end

It's certainly not a lot less code in the PostsController when comparing it to the previous version, but actually we've got quite a bit of extra functionality with this:

  1. We are rendering actual error pages with proper HTTP status codes
  2. The handling of all the different http error condition statuses is handled coherently:

    Frontend users will see a customized error page inside the app's layout

    "Backend" users (JSON, YAML, XML, Ajax) will get a uniform response with an appropriate status code

  3. All across the app, throwing the approprite exception when an error condition occurs bubbles up to the rescue method and will be handled the same way.

    For example, your models might have exhaustive permission control. With this kind of error handling, you could throw a NoPermissionError inside of your model and it will bubble up through the controller for the current request and render the appropriate error page - without a need for a single line of exception handling code inside the specific controller that is handling the request.

Considering you will likely remove the same duplicate code from quite a few controllers which your app surely has, you also end up with a fair share of removed lines of code while still getting extra functionality.

Once I've gotten used to bubbling up exceptions and handling them centrally in the ApplicationController, I've also found writing actual controller code a lot easier: It's very comforting to know that any exceptional cases will be handled properly and don't need my further attention apart from throwing the right exception when something goes wrong.

p.s. Of course this approach will fail miserably if you are catching all exceptions in your actions like this:

  def update
    @post.update_attributes!(params[:post])
  rescue => err
    # Do something
  end

You are not doing this, right?

If you cannot handle an exception, you should not catch it because otherwise you will prevent other code that is capable of handling it to work. Actually, as long as you do not raise again, nobody outside will notice that there was an error in the first place.

Thanks for reading, let me know about your approaches!

Written by Christoph Olszowka
blog comments powered by Disqus