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 => e

and:

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.

images/blog/2017-03-31-unexpected-case-of-rescue-exception/passing_spec.png

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:

images/blog/2017-03-31-unexpected-case-of-rescue-exception/debugger.png

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:

images/blog/2017-03-31-unexpected-case-of-rescue-exception/rspec_error_generator.png

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
end

And if we run this, we will see it blows as it always should:

images/blog/2017-03-31-unexpected-case-of-rescue-exception/failing_spec.png

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.