codegourmet

savory code and other culinary highlights

Unmasking Exceptions

| Comments

Raising core StandardErrors can mask bugs in your code layer. But even re-raising exceptions from deeper service layers can mask errors that have nothing to do with the current exception use case.

Do not raise StandardErrors

This happened to me not long ago: I was raising an ArgumentError after a parameter sanity check, and the tests passed although the code had two bugs in it.

The following example is oversimplified to reproduce the error:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
class CatRepository
  def initialize(shelter_id); end
end

def store_cat(cat)
  if cat.type == 'dogg'
    raise ArgumentError.new("no dogs allowed!")
  end

  CatRepository.new().store(cat)
end

def test_raises_if_dog
  assert_raises ArgumentError do
    store_cat(Dog.new)
  end
end

The test is green, although we can clearly spot two errors in the code:

1
2
if cat.type == 'dogg' # typo
CatRepository.new() # missing shelter_id param

So what happend? The CatRepository constructor was raising an ArgumentError because of the missing shelter_id. The test was checking for an ArgumentError and succeeded, thus masking the fact that the ‘dogg’ condition was never reached!

One solution is merely curing the symptoms but nevertheless important: write a failing test:

1
2
3
4
5
6
def test_raises_if_dog
  assert_raises ArgumentError do
    store_cat(Dog.new)
  end
  store_cat(Cat.new) # shouldn't fail
end

And voila, our test will now error out because the shelter_id bug will trigger in the newly added line. When you write a test, always make your test fail if it passes on first try (for example you could switch Dog with Cat and check if the assertion fails).

But as stated before, this is curing the symptoms. Unmasking the bug can be accomplished by raising a custom exception:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
class NotACatError < ArgumentError; end

def store_cat(cat)
  if cat.type == 'dogg'
    raise NotACatError.new("no dogs allowed!")
  end

  CatRepository.new().store(cat)
end

def test_raises_if_dog
  assert_raises NotACatError do
    store_cat(Dog.new)
  end
end

Our test will now fail with something along the lines of:
expected NotACatError but got ArgumentError

We will then be able to fix the constructor parameter bug and then notice that our assertion still fails:
expected NotACatError but didn't raise

This will enable us to find and fix the dogg typo.

The additional benefit is that if you derive your custom exceptions from a common base class specific to your code layer (ShelterError), you can easily distinguish framework errors from core errors (bugs). Also, upper code layers can catch all your service level errors in bulk and handle them in an appropriate way.

Masking bugs with custom exceptions

So now that we’re using custom exceptions to not hide information, we will look into a case where this still fails.

In this use case we’re catching a core error and re-raise it as a custom framework error. We’re doing this because we want to signal the upper code layers that this is merely a configuration error that we anticipated and not a bug.

Again note that this is just a simplified example, so please forgive the evil dispatch that I’m using below:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
def store_cat
end

def store_dog
end

def store_animal(animal)
  begin
    putss "remember gloves!" if animal.type == 'cat'
    send("store_#{animal.type}", animal)
  rescue NoMethodError => error
    raise UnknownAnimalError.new
  end
end

Of course, in real life we want to use respond_to?() or something like that, but for the sake of the example bear with me.

Do you see the bug?
putss "remember gloves!" if animal.type == 'cat'

The putss statement is a typo and will trigger a NoMethodError, which we will catch accidentally! So for cats, this method will always give an UnknownAnimalError, no matter what - again, we masked a bug!

The solution to this is that if you catch and re-raise exceptions from lower code levels, never discard the information that has been provided with the original exception:

1
2
3
rescue NoMethodError => error
  raise UnknownAnimalError.new(error.to_s)
end

So now we will still get an UnknownAnimalError, but this time the exception message will be decorated with the original message: "UnknownAnimalError: NoMethodError: undefined method 'putss'"

A real-world example

Before we’re jumping to the conclusion (ahaha), let’s have a look at the following example:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
module ProgressDownloader
  require 'curb'

  class ConnectionError < RuntimeError; end

  def self.perform(url, file_path)
    begin
      Curl::Easy.download(url, file_path) do |curl|
        curl.on_progress do |total, current, _, _|
          puts "progress: #{current / total * 100}%"
          true
        end
      end
    rescue Curl::Err::CurlError
      raise ConnectionError.new("couldn't retrieve file")
    end
  end
end

ProgressDownloader.perform("http://blog.codegourmet.de/data.json", "/tmp/data.json")

The code above works like a charm: it downloads files and the on_progress handler will show a percentage in regular intervals.

When a connection times out or something strange happens, our ConnectionError will notify the outside world that the connection failed.

So what happens if we introduce a bug? Let’s say someone forgot returning true in the on_progress callback. This will make the downloader abort every download instantly, as we can see in the documentation.

A Curl::Err::AbortedByCallbackError will be triggered, which we re-raise as ConnectionError. We might spend quite some time debugging the construction of the URL or host name, since the bug is masked again and appears as a simple connection error!

So let’s preserve the original exception like we did it in the previous example:

1
2
3
rescue Curl::Err::CurlError => error
  raise ConnectionError.new("couldn't retrieve file: #{error.to_s}")
end

So now if we trigger the error, we will get the following message: ConnectionError: couldn't retrieve file: Curl::Err::AbortedByCallbackError

This is googleable and the first hit will lead us to a github description of this issue (which seems to have been resolved by now).

Conclusion

Avoid raising core errors.

Avoid testing for core errors being raised.

Derive from core errors, or even better: make your errors inherit from a base error class specific to your framework.

Catching and re-raising is common practice to decorate an error with additional information stemming from your code layer. But be careful to only decorate the error and not lose the original information.

Happy Coding! – codegourmet

Comments