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
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 => e
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 end
Story 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 end
And 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 end
As 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 end
Now 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
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 end
And if we run this, we will see it blows as it always should:
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.