Wednesday, September 16, 2009

Checked exceptions I love you, but you have to go


Once upon a time Java created an experiment called checked-exceptions, you know, you have to declare exceptions or catch them. Since that time, no other language (I know of) has decided to copy this idea, but somehow the Java developers are in love with checked exceptions. Here, I am going to "try" to convince you that checked-exceptions, even though look like a good idea at first glance, are actually not a good idea at all:

Empirical Evidence

Let's start with an observation of your code base. Look through your code and tell me what percentage of catch blocks do rethrow or print error? My guess is that it is in high 90s. I would go as far as 98% of catch blocks are meaningless, since they just print an error or rethrow the exception which will later be printed as an error. The reason for this is very simple. Most exceptions such as FileNotFoundException, IOException, and so on are sign that we as developers have missed a corner case. The exceptions are used as away of informing us that we, as developers, have messed up. So if we did not have checked exceptions, the exception would be throw and the main method would print it and we would be done with it (optionally we would catch all exceptions in the main log them if we are a server).

Checked exceptions force me to write catch blocks which are meaningless: more code, harder to read, and higher chance that I will mess up the rethrow logic and eat the exception.

Lost in Noise

Now lets look at the 2-5% of the catch blocks which are not rethrow and real interesting logic happens there. Those interesting bits of useful and important information is lost in the noise, since my eye has been trained to skim over the catch blocks. I would much rather have code where a catch would indicate: "pay, attention! here, something interesting is happening!", rather than, "it is just a rethrow." Now, if we did not have checked exceptions, you would write your code without catch blocks, test your code (you do test right?) and realize that under some circumstances an exception is throw and deal with it by writing the catch block. In such a case forgetting to write a catch block is no different than forgetting to write an else block of the if statement. We don't have checked ifs and yet no one misses them, so why do we need to tell developers that FileNotFound can happen. What if the developer knows for a fact that it can not happen since he has just placed the file there, and so such an exception would mean that your filesystem has just disappeared! (and your application is not place to handle that.)

Checked exception make me skim the catch blocks as most are just rethrows, making it likely that you will miss something important.

Unreachable Code

I love to write tests first and implement as a consequence of tests. In such a situation you should always have 100% coverage since you are only writing what the tests are asking for. But you don't! It is less than 100% because checked exceptions force you to write catch blocks which are impossible to execute. Check this code out:
bytesToString(byte[] bytes) {
 ByteArrayOutputStream out = new ByteArrayOutputStream();
 try {
   out.write(bytes);
   out.close()
   return out.toSring();
 } catch (IOException e) {
   // This can never happen!
   // Should I rethrow? Eat it? Print Error?
 }
}

ByteArrayOutputStream will never throw IOException! You can look through its implementation and see that it is true! So why are you making me catch a phantom exception which can never happen and which I can not write a test for? As a result I cannot claim 100% coverage because of things outside my control.

Checked exceptions create dead code which will never execute.

Closures Don't Like You

Java does not have closures but it has visitor pattern. Let me explain with concrete example. I was creating a custom class loader and need to override load() method on MyClassLoader which throws ClassNotFoundException under some circumstances. I use ASM library which allows me to inspect Java bytecodes. The way ASM works is that it is a visitor pattern, I write visitors and as ASM parses the bytecodes it calls specific methods on my visitor implementation. One of my visitors, as it is examining bytecodes, decides that things are not right and needs to throw a ClassNotFondException which the class loader contract says it should throw. But now we have a problem. What we have on a stack is MyClassLoader -> ASMLibrary -> MyVisitor. MyVisitor wants to throw an exception which MyClassLoader expects but it can not since ClassNotFoundException is checked and ASMLibrary does not declare it (nor should it). So I have to throw RuntimeClassNotFoundException from MyVisitor which can pass through ASMLibrary which MyClassLoader can then catch and rethrow as ClassNotFoundException.

Checked exception get in the way of functional programing.

Lost Fidelity

Suppose java.sql package would be implemented with useful exception such as SqlDuplicateKeyExceptions and SqlForeignKeyViolationException and so on (we can wish) and suppose these exceptions are checked (which they are). We say that the SQL package has high fidelity of exception since each exception is to a very specific problem. Now lets say we have the same set up as before where there is some other layer between us and the SQL package, that layer can either redeclare all of the exceptions, or more likely throw its own. Let's look at an example, Hibernate is object-relational-database-mapper, which means it converts your SQL rows into java objects. So on the stack you have MyApplication -> Hibernate -> SQL. Here Hibernate is trying hard to hide the fact that you are talking to SQL so it throws HibernateExceptions instead of SQLExceptions. And here lies the problem. Your code knows that there is SQL under Hibernate and so it could have handled SqlDuplicateKeyException in some useful way, such as showing an error to the user, but Hibernate was forced to catch the exception and rethrow it as generic HibernateException. We have gone from high fidelitySqlDuplicateKeyException to low fidelity HibernateException. An so MyApplication can not do anything. Now Hibernate could have throw HibernateDuplicateKeyException but that means that Hibernate now has the same exception hierarchy as SQL and we are duplicating effort and repeating ourselves.

Rethrowing checked exceptions causes you to lose fidelity and hence makes it less likely that you could do something useful with the exception later on.

You can't do Anything Anyway

In most cases when exception is throw there is no recovery. We show a generic error to the user and log an exception so that we con file a bug and make sure that that exception will not happen again. Since 90+% of the exception are bugs in our code and all we do is log, why are we forced to rethrow it over and over again.

It is rare that anything useful can be done when checked exception happens, in most case we die with error! Therefor I want that to be the default behavior of my code with no additional typing.

How I deal with the code

Here is my strategy to deal with checked exceptions in java:

  • Always catch all checked exceptions at source and rethrow them as LogRuntimeException.

    • LogRuntimeException is my runtime un-checked exception which says I don't care just log it.

    • Here I have lost Exception fidelity.

  • All of my methods do not declare any exceptions

  • As I discover that I need to deal with a specific exception I go back to the source where LogRuntimeException was thrown and I change it to <Specific>RuntimeException (This is rarer than you think)

    • I am restoring the exception fidelity only where needed.

  • Net effect is that when you come across a try-catch clause you better pay attention as interesting things are happening there.

    • Very few try-catch calluses, code is much easier to read.

    • Very close to 100% test coverage as there is no dead code in my catch blocks.

31 comments:

  1. Do you think it would be feasible to prune checked exceptions from Java, say, just make the compiler to interpret them just like unchecked ones (maybe with a command line flag)?

    Would there be any incompatibility problems, besides forward-source-compatibility (older versions of javac would not be able to compile it)? I couldn't think of any case... in this case, die, checked exceptions! XD

    ReplyDelete
  2. Rather than wrapping exceptions, have you seen @SneakyThrows from project lombok? I'm not sure I agree with it, but it's an interesting idea

    ReplyDelete
  3. Interesting article.

    One question, at which level do you catch LogRuntimeException? Entry point into your application?

    ReplyDelete
  4. There are good things about checked exceptions, too. I know this opinion runs counter to the direction that most practitioners are moving in.

    Checked exceptions have been overused, no question. But I am glad that checked exceptions are there because it gives me the option of using them where they really add value.

    Exception conditions are rare by definition--it is quite easy to overlook them when constructing tests.

    No doubt unchecked exceptions make things easier for the developer. So does dynamic typing--at the cost of a class of errors that are only detected at runtime.

    If I throw a checked exception from a library method, I am telling the library user that this is a condition that can arise and you must deal with it it some way, even if it might only be passing it up the chain, or silently (shudder) swallowing it. This might be a condition that the library user wouldn't otherwise even realize is a possibility, much less make plans to deal with it.

    I don't think your analogy to a "checked if" is apt, since not all (or even most) if blocks actually require an else block. It just isn't the same as an exceptional condition.

    ReplyDelete
  5. I covered similar ground in my checked exceptions are waste post.

    Joshua Bloch also recommends against using checked exceptions in Effective Java.

    ReplyDelete
  6. Nice post! Back when I wrote more Java code I used to have these sorts of problems frequently -- exception handling causing more annoyance/clutter than usefulness. It is often the case that you would rather crash with a reasonable error message (which you can debug later) than try to continue execution of the program in a bad state. Having a generic LogRuntimeException seems like a good approach to this problem.

    ReplyDelete
  7. "Since that time, no other language (I know of) has decided to copy this idea"

    Actually, Spec# has checked exceptions. I suppose this enables the static program verifier to prove object invariants and method contracts even for cases where such exceptions are thrown.

    ReplyDelete
  8. Checked exceptions certainly have a place, but are often overused, or used incorrectly.

    It's certainly annoying catching exceptions which can't happen. Notable examples are UnsupportedCharsetException and CloneNotSupportedException. Although your byte[] to String example came across a bit contrived, given that there's a String constructor which takes a byte[]...

    Some Exception classes are checked when they should in fact be unchecked. This seems like a rather subjective decision to make when designing an API. Integer.parseInt() may throw a NumberFormatException, but this is unchecked. Over time, we developers have learned to check for this exception, after having been burned a few times by bad numeric data. If NumberFormatException were a checked exception, it would be extremely annoying, I'll grant you that.

    What if instead of only "checked/unchecked" there were some "severity" indicator, and your IDE could warn you if it looked like some exception wasn't being handled adequately?

    ReplyDelete
  9. Your strategy is similar to Ant, with a BuildException something you can throw anywhere. But remote calls doesn't like the undeclared stuff so well, another reason why not to like RMI or SOAP

    ReplyDelete
  10. An other empirical evidence is found in a lot of large code base in the wild : in order to try to reduce the number of catch/rethrow of the checked exceptions, a MyApplicationException checked exception is created at the top in the exception hierarchy of the application.

    Then all checked exceptions are chained in a MyApplicationException...
    and all method signature then have a "throws MyApplicationException"...
    And now you don't even need to catch the MyApplicationException...

    See a pattern here ? it is an emulation of an unchecked exception (as the exception is allowed to bubble up without catch/rethrow), but with more verbosity ('throws' clause everywhere) and lost semantic (the original exception is always a 'cause' of the MyApplicationException)

    ReplyDelete
  11. I have same opinion. Like following code snip, BadLocationException will never happen but I have to write the dead try-catch code or there will be a compile error.

    StringReader inp;
    try {
    inp = new StringReader(doc.getText(0, doc.getLength()));
    } catch (BadLocationException e) {
    // This should never happen
    TCE.logger.log(Level.WARNING, "Met BadLocationException - "+e.getMessage());
    return null;
    }

    I like the idea of LogRuntimeException.

    ReplyDelete
  12. Checked exceptions are hardly convenient in the anonymous inner class example you provide, but the solution would be the same if you only had checked exceptions. The problem is that the writer of the ASM library does not expect the exception to be thrown, and hence doesn't declare it.

    You're also missing one of the most common patterns in checked exceptions in Java: rethrow and wrap using the XXXException(String message, Throwable cause) constructor. This can be used to provide a lot of information useful in diagnosis.

    Consider the hypothetical 'SqlDuplicateKeyException' you mention. Logging this in a large, busy application is close to useless, as there is no context.

    However, if this is rethrown at multiple levels with cause information it is a lot clearer what has happened

    AccountCannotBeCreatedException ("Account SecretSwissBankAccount cannot be created") caused by DataShardException ("Error updating data on shard 10.61.10.12") caused by SqlDuplicateKeyException ("ORA-00001: unique constraint violated.")

    Conveys a lot more information than

    LogRuntimeException("ORA-00001: unique constraint violated.")

    Using 'generic' exceptions like SQLException that have code or state information means that you can handle the various classes of exceptions (recoverable by retry, recoverable by reconnect, unrecoverable) for multiple versions of multiple vendors databases using relatively small lookup tables. Many libraries (including I believe Hibernate) use this in order to provide more logical information to their callers, without having a) to link against the drivers themselves to get an OracleDuplicateKeyException, b) rely on the JDBC specification for this, c) Use Class.getName().

    ReplyDelete
  13. 'Checked exceptions' as a concept are strikingly similar to 'tall midgets'.

    ReplyDelete
  14. OMG!

    Checked exceptions are Good Thing.

    I was Delphi programmer before Java.
    Delphi has only runtime exceptions.
    It's a Evil!

    Some exceptions you can find only after several weeks in production!

    It's like a dynamic typed language with its weaknesses but without its benefits.

    Checked exception is also a kind of documentation, which lazy programmer can't omit (look Delphi help for example - there is a lack of information about throwing exceptions)!

    ReplyDelete
  15. I think checked exceptions are a good idea, but are so easily abused that they need changing. The way C# deals with exceptions is equally as terrible, just by "pretending they're not there". It's like writing "throws Exception" on everything.
    The evidence is in Microsoft's own frameworks. Ever used Sharepoint? You don't get exception messages when things go wrong, just "object not set to an instance of an object". Silverlight? No exceptions, we'll just give you a blank screen, 'cos we didn't inform you that what you were doing could throw exceptions.
    So I think unchecked exceptions are just as bad as checked exceptions. What we need is something else...although I'm not sure what that is yet...

    ReplyDelete
  16. RE: Lost in Noise, Empirical Evidence

    If a catch block is just a catch and rethrow, then you don't need to define it. Just let the offending method call throw the exception and declare it. No catch clutter there.

    If you're wrapping the exception and re-throwing it, then you're adding context that could be useful further up the stack.

    FileNotFoundException and IOException are not simply edge cases. If you're not careful, you'll pay for it later when IT introduces a networked file system (AFS, GFS, NFS) under your code to help manage your application or site.

    RE: Unreachable Code

    You're writing bytes using an OutputStream abstraction, which requires a checked IOException. If you don't like it, use a different abstraction or write your own.

    RE: Lost Fidelity

    You should never catch an exception and throw a new one. You should always wrap your exceptions. HibernateException.getCause() = the original underlying SQLException. That way you don't lose fidelity.

    RE: You can't do Anything Anyway

    If your method has a return type, you can return null or some default value.

    When faced with a checked exception you have to ask yourself: do I want to handle it here or pass it up the stack to, say, the web application container or the client code. If you trust that they'll do the right thing, then pass it on. Otherwise, you know better the details of what just happened, so it's usually best for you to handle it yourself, even if it's just logging it or ignoring it.

    RE: How I deal with the code

    Where you log the exception is also important. A LogRuntimeException is likely to only be handled at the highest-level entry point in your code. If you're wrapping your exceptions correctly, you may have to dig through a few getCause() results to find the line where the error originated.

    The line where you throw the exception, which can be logged as well, can also be important. That line may contain some comment that helps you understand what's going on. You decided to catch at that location for a reason. Catching everything generically at the top level loses that location-based context.

    ReplyDelete
  17. Re: unreachable code,

    Unreachable code isn't the worst thing in the world; you just slap down a "throw new AssertionError(e)" and you're done, and coverage tools well ought to recognize that an AssertionError should never be reached.

    Anyway, are you sure your example code compiles? Because the methods of ByteArrayOutputStream don't throw IOException. The problem only comes up when your reference is of type OutputSream.

    ReplyDelete
  18. Even if you can't do anything, catching, wrapping and rethrowing an exception allows you to get more context (e.g. method arguments) into the stack trace. Too bad Java isn't able to do this automatically.

    Hibernate converts the generic (and not so useful) SQLException into a detailed exception hierarchy, e.g. ConstraintViolationException, so this may not be the best example for your argument.

    Should also mention that you can call Throwable.getCause(); handling errors this way isn't pretty, of course.

    ReplyDelete
  19. happygiraffe said: Rather than wrapping exceptions, have you seen @SneakyThrows from project lombok? I'm not sure I agree with it, but it's an interesting idea…

    project lombok is interesting, but isn't it IDE specific right now? Supported only by Eclipse?

    ReplyDelete
  20. The bytesToString method can be tested. By creating a FakeIOExceptionThrowingOutputStream that always throws an exception in the write method.

    With that said, it is totally not worthwhile and I agree with your point.

    Unfortunately Java has some chunkiness to it that makes some things difficult or impossible to test, or just not worth testing.

    ReplyDelete
  21. Checked exceptions have a respectable history in Modula-3 (paraphrasing Hoare, a great improvement on its successors), and possibly CLU. The problem is that Java f***ed it up, like many of its features. They missed off the FATAL pragma, which allowed programmers to declare a checked exception as fatal within a scope and stop checking it.

    Also, from what I've seen, a lot of coders just don't put the time in to structure their exception hierarchies play together well.

    In the longer term, I'm sure there could have been language constructs to manage the common cases more effectively than try/catch/ignore. Nothing changed.

    ReplyDelete
  22. One major problem IMO about java's implementation of typed errors system is that it doesn't mix well with the type system. For instance when I say List[string] I'd like to say sometimes List[string throws ParseException] and this will mean that throws is part of a type of the value that you can either receive as a parameter or return. There is also some space for syntax improvement, but checked exception is in no way a bad idea per ce.

    In .net, when passing around delegates or anonymous functions (like anonymous classes) it becomes quite hard often (because of lazyness) to figure out where to catch the exception. A type system could very useful in such cases and even more when multi-threading and parallelism is involved.

    It is not checked exceptions that breaks abstraction boundaries, it is just bad design. What breaks abstraction boundaries is having exceptions crossing all the stack to the top with information it carries.

    I guess to find the use of checked exceptions u need to structure your code in a way were your pure algorithmic code resides away of exception, then reading files code handles exception and passes pure data to this algorithms.

    The problem is very similar to null being default to any type.

    Errors showing in type system is quite useful, it can be certainly implemented or used better than how it is in Java.

    ReplyDelete
  23. Checked exceptions should be thrown by methods where failure is possible due to problems outside the control of the code (typically related to IO - eg disk space, files not existing etc).

    In most cases, these checked exceptions can be rethrown as unchecked exceptions with additional detail and nested checked exception, which is important when supporting production systems - eg:

    try {
    File f = new File(filename);
    ..
    } catch (IOException) {
    throw new RuntimeException("IO Exception processing " + filename, e);
    }

    ReplyDelete
  24. The anti-checked camp is large because most people these days work on web apps, for which I agree most exceptions are unrecoverable.

    However, when workflows aren't limited to user request-response loops, things aren't so easy. For code paths that don't start or end at a user, if something goes wrong, there's got to be a catch clause somewhere to handle it. And you better not forget it. I work on an app like this, and 95%+ of my catch clauses do interesting work, and God bless the checked ex for ensuring that I write them.

    Are writing catch clauses fun? Not usually. But ignoring exceptional cases is not a responsible alternative. Checked ex's are unpopular because developers are lazy.

    People always say of unchecked ex's: Only catch the ones you want to handle. Well how do you know? Better hope the docs are complete (ya right). And don't forget that with unchecked ex's, not only do you have to worry about exceptions thrown by the methods you call directly, but also all the methods they call, and so on down the stack. With checked ex's, you always know what to expect.

    Anything, three abstraction layers down, could throw, and you have to somehow figure it out. And even if you do, catching an exception defined beneath an abstraction layer by definition breaks abstraction. Should you really be catching your high fidelity SQLExceptions in the presentation tier?

    Java's checked exceptions are an linguistical treasure, and people will begin to realize that when they're gone.

    ReplyDelete
  25. I think Bruce Eckel should be credited here - he's been one of the first people to question checked exceptions.

    ReplyDelete
  26. The alternative to throwing FileNotFoundException is to return an error code and check it in the caller (if ... else). But what if your method is supposed to return some data? Checked exceptions are a means of communication between a caller and a method, independent of the normal flow of control. When used carefully, they can improve readability of the code, and also testability (fewer code paths). One problem in Java is that exception handling (locating the boundaries of the block in which an exception is thrown) is very slow, because of the way Java stores and looks up this information.

    Checked exceptions should not be thrown when your program enters a bad state from which it cannot recover. For this you have runtime exceptions. Rather, checked exceptions should indicate wrong user input, bad commandline arguments, etc.

    Your example with the ByteArrayOutputStream does not demonstrate a problem with checked exceptions, but with Java. Only close() declares IOException, but it never throws it, because it doesn't do anything anyway (at least in Java 5), so it should not declare it.

    ReplyDelete
  27. I think a good trade-off for checked exceptions would be if there was an annotation equivalent. We could easily indicate to the tools the possible code's behavior and don't force anybody to rethrow the exception or otherwise clutter the code.

    ReplyDelete
  28. In your implementation classes you can declare in the method signature any runtime exceptions that may occur in the method. This may be useful if there is a particular runtime exception that you want people to be on the look out for. It doesn't have to be declared in the interface or anything, so it doesn't have the same "forced ripple" effect that checked exceptions have, and the users of the class are not forced to catch it. The topic of checked exceptions is one that Rod Johnson has talked about at length in his books prior to Spring if one is looking for supplemental reading.

    ReplyDelete
  29. @John: the "forced ripple" is a pro for checked exceptions, not a con. Much of this checked ex bashing would subside if they would just add some sugar to the language to easily wrap them in runtime's. The multi-catch would've helped, and it sucks that it's not going in Java 7.

    ReplyDelete
  30. Hey, if you want to turn them off entirely, you can do that.

    This jar, if you add it to the classpath when you compile with javac, will treat every exception as if it was an unchecked exception:

    http://projectlombok.org/disableCheckedExceptions.html

    ReplyDelete
  31. "2-5% of the catch blocks which are not rethrow"

    That's crazy. Please don't blame a tool because you're using it incorrectly. Every language has areas where you can do dumb things and cause problems.

    ReplyDelete

The comments you read and contribute here belong only to the person who posted them. We reserve the right to remove off-topic comments.