Quick! What does the following method do when thing.method_that_might_raise! raises SomeAppException? And why is this a code smell?

1def some_method
2  thing.method_that_might_raise!
3ensure
4  return thing
5end

Before giving the answers to these two questions, let’s go over what ensure does.

The ensure clause in Ruby is run regardless of whether a block has thrown an exception or not. A simple example is opening a file1:

 1def file_open_with_auto_close(name, mode = 'w', &block)
 2  f = File.open(name, mode)
 3  puts "calling your block"
 4  yield f
 5ensure
 6  if f
 7    f.close
 8    puts "file safely closed"
 9  end
10end
11
12file_open_with_auto_close('test') do |file|
13  file << 'data'
14  raise 'exception raised'
15end
16#
17#calling your block
18#file safely closed
19#RuntimeError: exception raised
20#  from (irb):14
21#  from (irb):4:in `file_open_with_auto_close'
22#  from (irb):12

Even if there is an exception while processing the file, like the one we raise on line 14, ensure allows us to close the file.

After the ensure clause has run, Ruby either continues the exception handling (in this case irb rescues it and gives us a stack trace) or continues executing the block.

Except if you have an explicit return statement in your ensure clause.

Let’s take a look at the difference in irb, first without an explicit return statement:

 1def ensure_without_return
 2  yield
 3ensure
 4  puts 'ensure'
 5  true
 6end
 7
 8ensure_without_return { puts 'block'; false }
 9#
10#block
11#ensure
12#=> false
13#
14ensure_without_return { raise 'exception raised'; puts 'block'; false }
15#
16#ensure
17#RuntimeError: exception raised
18#  from (irb):21
19#  from (irb):16:in `ensure_without_return'
20#  from (irb):21

Note that although the ensure clause is run after the block from line 8, it has not changed the return value of the method.

And now with an explicit return statement:

 1def ensure_with_return
 2  yield
 3ensure
 4  puts 'ensure'
 5  return true
 6end
 7
 8ensure_with_return { puts 'block'; false }
 9#
10#block
11#ensure
12#=> true
13#
14ensure_with_return { raise 'exception raised'; puts 'block'; false }
15#
16#ensure
17#=> true

The first thing to note is that the return of the method is now determined by the return statement in the ensure clause on line 5.

The second thing to note is that the explicit return statement acts as an implicit rescue clause, allowing the code to resume as if no exception had been raised.

Summarizing:

Back to our original questions. You should now know what the method does when thing.method_that_might_raise! raises SomeAppException.

But why is this a code smell? Consider the following code:

1def some_method
2  thing.method_that_might_raise!
3rescue Exception
4  # we have rescued all possible exceptions
5ensure
6  return thing
7end

Line 3 is a code smell. Rescuing all exceptions is not desirable. From our exploration of ensure we can see that this code is the equivalent of the original code.

Can we refactor it? Yes. Yes we can.

When we can recover from SomeAppException, we can just rescue:

1def some_method
2  begin
3    thing.method_that_might_raise!
4  rescue SomeAppException => e
5    # do something clever here
6  end
7  thing
8end

And when we cannot recover from SomeAppException, we just let the exception propagate up the call stack:

1def some_method
2  thing.method_that_might_raise!
3  thing
4end

1 File.open already does this.

blog comments powered by Disqus