Be Narrow In Your Rescues

methodology

Tue Jan 01 16:47:00 -0800 2008

When trying to block certain kinds of exceptions, it’s tempting to write catch-all cases. Like:

string.match(/inet addr: ([\d.]+)/)[1] rescue ""

or:

File.delete('file_that_might_not_exist') rescue nil

In the first case we want no match to return nil. In the second we just want the file gone, and aren’t concerned if it wasn’t there initially. The problem is, both of these mask all exceptions, so we may not find out about other kinds of errors.

So we want to differentiate between user-generated errors and programatic errors. User-generated errors are a natural result of the imperfect data coming from a user or some other source outside our program. We can’t force the world to always give us perfect data, so we handle these error gracefully.

Programatic errors are mistakes that we, as the developer, have made in crafting the business logic of the app. In this case, we want to hear about it as soon and as loudly as possible - so that we can find the flaw and fix it. (This is the Rule of Repair: “When you must fail, fail noisily and as soon as possible.”)

So in our examples above, one option is to skip the rescue and test for the case that we’re actually looking for. On the regexp match:

(m = string.match(/inet addr: ([\d.]+)/)) ? m[1] : nil

This will protect only the error we care about (no match) and let through any others.

The other option is to capture the specific exception you are looking for only. This might be the right choice on the delete file example:

begin
  File.delete('file_that_might_not_exist')
rescue Errno::ENOENT
end

Alas, not all on one line, but correctness beats succinctness when the two come in conflict. (Although you could also try FileUtils.rm('filename', :force => true) here.)