When you work on a project that’s been around for quite a while, you’re probably going to - sooner or later - encounter a few skeletons in the closet. These skeletons might be sections of code or design patterns that people know about, know they are bad, and yet no one is addressing them for various reasons. Probably because nobody wants to do the redesign if the code just “works” - even if it’s terrible. What’s worse is when these same structures or patterns are being repeated, because people are blindly following them!
I recently joined a new project for one of our clients and this particular skeleton was a Ruby rescuing Exception class. When I talked with my team members about the rescuing Exception class, they knew they shouldn’t have it in there from a good design standpoint, but nobody could answer why it was living on in the code. After this discussion, we decided to gradually remove this from our code.
The story could have ended here, but there was one test that really scared me - a very spooky skeleton! I’ll get to that part soon, but first let’s recap the most obvious reason for not rescuing Exception.
Why you should not rescue Exception class?
In Ruby, the `Exception` class is the base class for the whole family of Exceptions. If you take a look at the official documentation you will notice a lengthy list of Exceptions subclasses. Many of these Exceptions are not your regular, application level exceptions. There are things like `NoMemoryError`, `SignalException`, `Interrupt`, `SystemExit`, `LoadError` and even `SyntaxError`! All these exceptions are way above your everyday application code. They are meant to be handled by VM or other systems in very rare and special cases.
The exceptions that are of interest from an application point of view are descendants of `StandardError`. What’s more, if you specify a rescue without providing an expected error class it will default to `StandardError`. A similar situation happens for a `raise` - when no arguments are provided it will raise a `RuntimeError`, which is below `StandardError` in the hierarchy tree.
rescue => e
# ==
rescue StandardError => eand:
raise "My error"
#==
rescue RuntimeError, "MyError"My advice here is simply not to rescue directly from `Exception`, and instead use `StandardError` as a base class for your custom error.
MyError = Class.new(StandardError)
class MySecondError < StandardError
endStory of a single failing test
One day I was doing refactoring of a single service class. I was happy with how everything was going until I stumbled across a single failing test.
To give you a better overview of the problem, let’s assume we have some code that looks similar to this:
class BaseService
Failure = Struct.new(:error)
Success = Struct.new(:ok)
end
class SomeSeriousOperation
def call
charge_client_200_times
# do something else
BaseService::Success.new
rescue Exception => e
BaseService::Failure.new(e.message)
end
def charge_client_200_times
# Empty clients accounts
end
endAnd here is the corresponding test for that:
require 'rails_helper'
RSpec.describe SomeSeriousOperation do
if "doesn't charge clients" do
service_instance = SomeSeriousOperation.new
expect(service_instance).to_not receive(:charge_client_200_times)
service_instance.call
end
endAs you can expect, this test fails. The problem is, that it fails in a really strange way. It fails by passing.
If you take a closer look, there is no possibility for this test to pass. What’s going on? The test is saying that we don’t want call charge_clients_200_times method but it is the first thing this service does! So what happened? That’s right, `rescue Exception` happened.
This is serious. It basically means we have a false-positive test in our suite. It gets even more serious if you realize that there may be more examples like this. The most dangerous consequence is that it shatters your confidence about the tests in your system, and the application as a whole. At the end of the day the main reason for writing tests is to gain confidence in your software correctness! I stated the problem could be arising from that `rescue Exception` but what exactly happened? Let’s verify that. We will just drop a breakpoint in the error handling part and inspect what’s going on:
class SomeSeriousOperation
def call
charge_client_200_times
# do something else
BaseService::Success.new
rescue Exception => e
binding.pry
BaseService::Failure.new(e.message)
end
def charge_client_200_times
# Empty clients accounts
end
endNow if we run our suite and look at the variable `e` that is holding our error we will see something like that:
It looks like the error we caught and silenced in our application code is the error that RSpec uses to handle your mock expectations. It holds exactly what we would expect to blow our tests. When we visit RSpec’s github and search for a `MockExpectationError` class we will easily uncover why our code breaks this:
Like many creepy code occurrences, the solution to this is, of course, trivial. Instead of rescuing `Exception` class we rescue regular `StandardError` instead.
class SomeSeriousOperation
def call
charge_client_200_times
# do something else
BaseService::Success.new
rescue => e
BaseService::Failure.new(e.message)
end
def charge_client_200_times
# Empty clients accounts
end
endAnd if we run this, we will see it blows as it always should:
Summary
Because of this problem we decided to immediately get rid of rescue Exception in the whole project. If this small an error could break tests, who knows what catastrophes it could cause in the wild? Luckily for us, it turned out we had only a few false-positive tests in the code base. Most of them had incorrectly defined mocks or were no longer relevant. It nicely illustrated the problem of using a rescuing `Exception` class. It not only rescues the VM-level error but can also hit you in the back in a much less obvious way!
Need an extra set of eyes over your code? We can help to ensure your code is free from errors like the rescuing exception class. Employ the services of our expert developers to improve your web applications and apps, and rescue you from novice mistakes. Call us today to arrange a chat about your project.
