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.

Peter Kriens

14 comments:

  1. Hi Peter,

    I may not have interpreted this post correctly, but it sounds like you're advocating this sort of API:

    interface Foo {
    void doStuff() throws Exception;
    }

    I guess this also implies that every method between doStuff() and the entrypoint must also declare "throws Exception"...

    I don't know if it's just my conservative upbringing (my mother taught me to either be specific about the exceptions I throw, or to not throw them at all), but this just seems ugly to me.

    I suppose your underlying point (which I wholeheartedly agree with) is that checked exceptions should never have been there in the first place.

    Perhaps, somewhere in the nebulous future, Java will lose the whole concept of checked exceptions. One can always dream...

    Cheers,
    -Dave

    ReplyDelete
  2. 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.

    ReplyDelete
  3. @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.

    Look at some real code:

    void foo() {
    try {
    doXXX();
    doYYY();
    } catch( Exception e ) {
    throw new RuntimeException(e);
    }
    }

    Or:

    void foo() throws Exception {
    doXXX();
    doYYY();
    }

    Try the throws Exception for some time :-)

    ReplyDelete
  4. 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.

    ReplyDelete
  5. @Christian: I do not think that adding to the existing virality is making things that much worse ... They should just do the same thing.

    ReplyDelete
  6. @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.

    ReplyDelete
  7. 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:


    interface ResultComputer {
    Object computeResult(Object param);
    }

    ResultComputer rc = new ResultComputer() {
    public Object computeResult(Object param) {
    return new URL((String) param); // this throws checked MalformedURLException :-(
    }
    };

    someMethod(rc);

    I hate checked exceptions too.

    ReplyDelete
  8. 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).

    ReplyDelete
  9. 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.

    ReplyDelete
  10. 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.

    Kjetil

    ReplyDelete
  11. I really fail to see why:

    void foo() {
    try {
    bar();
    } catch( Exception e) {
    throw new RuntimeException(e);
    }
    }

    Can be better then:

    void foo() throws Exception {
    bar();
    }

    I find the second one a million times more readable ... and imho that is what counts. Less is more, as also Ockhams razor testifies.

    ReplyDelete
  12. 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.

    Kjetil

    ReplyDelete
  13. I think this discussion has run its course.

    One advice, try it out in a project and see how incredible clean and simple the code body becomes, only after you've done that in a serious project, judge it. Likely you never want to go back.

    ReplyDelete
  14. If you're in this camp, patch checked exceptions out of the compiler. It's far easier.

    ReplyDelete