3. Good Practices - Ruby Exception Handling

on Jul 20, 2019


Exceptions are tricky. Most people don't give them the attention they deserve and I think it's because most of the time they don't really know what to do about them.

In this episode I want to talk about some good practices that I've learned about handling errors and things you should avoid to save yourself from some headaches.

Let's start with simple things that you should not do. Here we have a variable that might or might not be nil.

product = nil

product.name rescue nil
product.name rescue ''

I usually see people writing this kind of code when they want to avoid getting a NoMethodError when product is nil. Please don't do this. The reason this is bad is because that rescue will swallow any exception you throw at it. If you have a typo in there, for example, rescue will catch it and it'll easily go unnoticed.

produt.name rescue nil

The right way to handle this scenario would be to try! and, in the second example, calling to_s if you really want an empty String when either product or name is nil. Remember you need ActiveSupport for try to be available.

require 'active_support/all'

product.try!(:name)
product.try!(:name).to_s

try! will just return nil if product is nil. Otherwise, it will call the method. Be aware that the bang version of try will raise an error if name is not a valid message for product. If you don't want that, you have to go with simple try, no bang.

product.try(:name)
product.try(:name).to_s

Since Ruby 2.3.0 you can also use safe navigation, which makes this code even simpler

product&.name
product&.name.to_s

Safe navigation has the same effect as the try! here.

Now take a look at this simple controller action.

def create
  Product.create!(product_attributes)
rescue
  flash[:error] = 'Failed to create Product'
  redirect_to action: :index
end

This is trying to create a product with the bang version of create, which fails if any validation fails. Then we rescue from anything, add a generic flash message and redirect the user to another page.

There are several things wrong with this code. First, we should not rescue from anything. Nor should we rescue from Exception like this, which all exceptions inherit from

def create
  Product.create!(product_attributes)
rescue Exception => _
  flash[:error] = 'Failed to create Product'
  redirect_to action: :index
end

In Ruby all errors you can get inherit from Exception, and that means rescuing from Exception will rescue from everything, even the errors you don't need or don't really want to rescue from, like SyntaxError and Interrupt. By rescuing Interrupt, for example, you're basically preventing the user from hitting Ctrl + C while inside this block, which is very bad.

The right approach here is to be specific. What is it that you're rescuing from? Can you know exactly which exceptions you're looking for? Maybe ActiveRecord::RecordInvalid?

def create
  Product.create!(product_attributes)
rescue ActiveRecord::RecordInvalid => _
  flash[:error] = 'Failed to create Product'
  redirect_to action: :index
end

If you don't know which errors to expect and want to really rescue from anything bad that can happen to your code, use StandardError. StandardError is the base class for common runtime errors.

def create
  Product.create!(product_attributes)
rescue StandardError => _
  flash[:error] = 'Failed to create Product'
  redirect_to action: :index
end

By the way, when creating your own error classes, inherit from StandardError, NOT from Exception.

class MyCustomError < Exception # change to StandardError
end

Now to the next issue: we are hiding the problem. We are redirecting the user to a page with a generic error message and completely ignoring the error that occurred. Good luck tracking down any bug when the user comes back to you saying "Hey, I'm getting this Failed to create Product message" and you have nowhere to look at to see the error. No logs, no bug tracking system.

You should not do this, don't hide errors. If the system fails, let it fail and show the user a 500 error page and that's it. Let the error go to your bug tracking system or get dumped to your logs so you can look at it.

One could argue that instead we could add the error message to the flash message we're gonna show the user.

def create
  Product.create!(product_attributes)
rescue StandardError => err
  flash[:error] = "Failed to create Product: #{err.message}"
  redirect_to action: :index
end

I disagree. You don't want your users to see things like Unable to connect to database XYZ with user ABC or other technical details that might expose your system to attackers. Unless we're talking about errors we know are safe, like validation errors, then it should not be exposed to end users. Expose it somewhere developers can see it, not users.

Another kind of scenario that I see often is this. We explicitly raise an error if validation fails here.

def create
  if Product.create(product_attributes)
    redirect_to action: :index
  else
    raise 'Failed to create Product'
  end
end

This is bad because validations are expected to fail if users provide invalid data. Unless you're rescuing this somewhere, this error will get out in the logs or into your bug tracking system, and there's no point in doing that. There's nothing you can do about it here, the user did something wrong and you should show him the problem and consider the request fulfilled, no errors necessary. I've seen bug tracking systems get filled in with errors that could be dismissed like this one and it ends up hiding real problems.

This brings me to a few rules that I always apply when dealing with errors in backend systems:

  1. If it's a technical error, the client should get a generic 'We screwed up' message and the error goes to bug tracking system
  2. If it's a client error like invalid user provided data, then the client gets a very specific error message and the bug tracking system gets nothing

And the last thing about errors is: where should you be handling them?

The answer is: highest level possible. For example: in the controller, rake task or job that you're executing. Why? Because that makes your low level code reusable. Very often error handling means notifying bug tracking systems, or sending out an email or printing out to the logs. You shouldn't do that in lower levels of your code because you might want the same functionality to be executed from two different places and error handling to be different for each of them.

And that's it for this episode. Thank you for watching and I hope you enjoyed this snack!


Comments

Sign In or Sign Up to comment.