Tuesday, March 1, 2011

Exception Hygiene

Arguably the worst decision in Java was to have checked exceptions. I've never been thrilled by Java as a language (after Smalltalk all other languages seem, well, just not something to get excited about) but checked exceptions seem to get to me every time. I just had an another bad experience with this exceptionally bad idea.

I recently had to work on some open source code, the code was about a specific servlet. After working around some of the limitations I decide to just fork it. The servlet was not working very reliable and I could not really pinpoint where the problem was. So I decided to just remove all low level exception handling and handle all exceptions in the service method of the servlet. The authors had done the traditional way. This is making APIs without general throw clauses so you have to catch all the bad stuff and then turn it into some specific exception. It was amazing how readable the code became, and, even better how easy it was now to diagnose because anything that went wrong always ended up in the catch block of the service method. Easy to set break points, and easy to handle. The other improvement was that I now could get rid of all of the messy logging code that also cluttered the code. Virtually all code was logging exceptions and this logging could now be done in one place. Removing checked exceptions this way is imho the cheapest and most effective way to improve your code.

So how do I handle exceptions? Well, for me a function all has two exits: a normal return and an exception. I do not believe exception types matter, only badly designed APIs where exceptions are abused for normal behavior require their types. By having this simplified model, any API can throw any exception and I usually declare "throws Exception" on my API methods so I do not have to catch any lower level exceptions. This model makes it easy for an exception to bubble up to the top level method that knows what to do with an exception. Obviously intermediate functions must use the finally block to ensure that any any resources are properly closed.

For the OSGi APIs we've not followed this model because lots of people still need to get used to ignoring checked exceptions. However, if I see how far we've come since the early days then I think we can one day expect this style to be common.

When handling exceptions I follow the style that spring uses. For my own exceptions I only used RuntimeExceptions. If I do not think the exception needs to be catched I use a simple RuntimeException. Later I can still create a special RuntimeException class if someone needs it.

With checked exceptions outside my code I deal like spring does. I convert them into RuntimeExceptions where they happen. So my code does not need to throw Exception all over the place and still looks very readable.

@Dave: Yes, you're right. It is not a beauty but it makes the code much smaller, and thus more readable. And as I argued, more robust. I am not talking saving some small crumbs here.

@Christian: Well, you're basically wasting CPU cycles and source code real estate to achieve what a simple declaration of "throws Exception achieves." And you still create an indirection between the original cause and the (imho wasteful) RuntimeException.

The problem with throws Exception is that it is viral. So if I use it and someone calls my code then I force him to do the same. So if most of my code does not have to deal with checked exceptions then I only have very few places where I have to do the conversion. The rest of the code is then clean of checked exceptions.

@Christian: After some more thinking. Throwing Exception is exactly the same burden as throwing SomeOtherException, the caller has to handle the exception (or be clever and let it bubble up). So it is not as if you're doing something highly uncommon ... You make it sound worse than it is.

The problem with "throws Exception" comes when the method is actually implementing some interface method that is not declared with "throws Exception". This is often annoying in callback interfaces like this:

I second Dave and Christian here. There's nothing I hate more than coming across an API riddled with checked exceptions, and most of all *the* Exception.

You talk about wasted CPU cycles? I suggest you fire up some example code in Google Caliper and see for yourself how much performance is really at stake here. It isn't 1995 any more.

You also mention source code real estate. Method signatures are the most expensive "real estate" in code: your APIs should be as clean and clear as possible. Adding throws Exception to them all is a big step backwards, in my opinion the worst possible way of dealing with checked exceptions.

Tackle (wrap or handle) checked exception as deeply as you can. If a method has still a risk of throwing an exception, communicate this in the method signature (with runtime exceptions).

You "hate" API's that throw Exception because ... you do not want to throw Exception yourself. See what is wrong with this picture?

If everybody would just throw Exception nobody would have a problem ... Better, if we could just get rid of checked exceptions altogether.

The problem is anybody that forgets to add to throw Exception on an interface method, forces any implementer to do this rather utterly useless wrapping, which to often ends up obscuring the cause.

Less is more, having throw Exception on your interface is less work for the implementers. You generally have many more implementers than users of an interface so total complexity shrinks. Hard to hate imho.

I sympathize with Peter's intentions, but not with the "throws Exception everywhere" approach.

Instead of propagating the throws clause, I find it better to catch checked exceptions when I make a calls to the third-party code that declares them. The catch clause wraps and rethrows it as an unchecked exception, preferably with some information about what was going on at the time of the failure. This means less work and clutter along the way to the exception handler at the top, traded for more work at the "bottom".

The real benefit of this approach is capturing additional information about the failure case and reflecting it in the exception message. A short summary like "I am this object, my state is so and so, and I was asked to do this and that when THIS exception occurred" can do wonders for debugging.

What you realize, of course, is that this is useful for unchecked exceptions as well. When you have an important call that fails, gather up important state and parameters and throw a new exception message that says a little more about the situation. Actually, this is useful NO MATTER WHICH EXCEPTION IT IS! What's important is capturing the exception, and providing more information about it. So you end up with "catch (Exception e)" at various strategic points in your code - and useful stack traces with informative "caused by" breaks along the way.

Note that you can still have a good exception type hierarchy with this approach, even if the compiler doesn't know about it. You can even tell the compiler everything about what's going on, if you like. This gives you more work maintaining the thing, and the risk of missing out on useful error information later. But some people like to keep the compiler informed.

The trade-off is between having a) "throws Exception" on most methods of your application code and b) nipping the checked exceptions in the bud and having no "throws" in your methods anywhere. And so, in all the rest of your application code, you get away with void foo() { bar(); }

As an aside, I also find that one benefit of b) is that I can emit more runtime information when exceptions happen, so that stack traces are more useful. Not new RuntimeException(e), but new RuntimeException(this + " failed to bar", e). Add some parameters to the mix there, and you may see my point.