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 |
|
The test is green, although we can clearly spot two errors in the code:
1 2 |
|
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 |
|
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 |
|
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 |
|
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 |
|
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 |
|
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 |
|
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.