Wednesday, November 5, 2014

Is it a bad practice to catch ‘Exception’?

I recently encountered an organization that religiously enforces the Checkstyle IllegalCatch rule (reference here) and mandates following it for all production code.  This rule seeks to ensure that developers don’t catch Throwable, Exception, or RuntimeException directly. The assertion is that it is always a better practice to catch more specific exceptions.  This rule seems a bit draconian to me, so I did a little surfing to see what others thought.  It seems the question has been debated at length, sometimes heatedly.  In fact, like discussion involving religion or politics, discussion of this issue is sometimes more heated than the strength of the supporting arguments provided.

Common reasons that people answer ‘yes’ to this question and assert that catching Throwable, Exception or RuntimeException is universally a bad practice seem to distill to the following reasons:

  • Developers can’t expect to handle all kinds of exceptions, which is what catching Exception implies (i.e. you’re code is not worthy).
  • It’s possible that classes up the stack will have a better ability to handle the unexpected exception thrown.
  • Detailed exceptions can often provide additional detail about the problem.
  • Developers routinely handle exceptions poorly loosing information needed to diagnose reported bugs or environmental issues.

It is interesting that a common “exception” (pun intended) to this rule given by the skeptical is that application entry points need to have a global catch.  Presumably the reasoning is that logging the exception to standard error (which is what the JVM does with uncaught exceptions by default) isn’t what’s wanted in most cases.

I am one of the ‘skeptics’ and disagree with the global catch prohibition, at least for Exception and RuntimeException, for several reasons.  Let's treat Throwable as a separate case that I'll address later in the entry.

There is no reliable way to identify the list of specific exceptions a developer can ‘expect’ and could be coded in catch clauses.  Most of us leverage third-party products in our code very frequently.  Unless the products you use throw checked exceptions or takes the trouble to document what exceptions it throws in the throws specification for all methods, there’s no way to get a specific list of the exceptions you can reasonably expect from product code.  If you don’t have a list of specific exceptions you can ‘expect’, there’s no reasonable way to code specific catches for those exceptions.

Specific catches violate DRY most of the time.  Most of the time, our response to all types of exceptions is identical.   Either we log the exception or convert it to some type of RuntimeException and re-throw it.  If our response to the most specific exceptions will be identical, coding multiple catches with identical logic just to adhere to the illegal catch rule just creates code bloat.  Yes, as of JDK 1.7, there’s a syntax to help eliminate the code bloat (as illustrated below).  Many of us aren’t on 1.7 in production yet.
JDK 1.7 catch example:
catch (IOException|SQLException ex) {
    logger.log(ex);
    throw new RuntimeException(ex);
}
Pre-JDK 1.7 catch example with DRY violation:
catch (IOException ex) {
    handleException(ex);
}
catch (SQLException ex) {
    handleException(ex);
}

The JDK itself is replete with instances of catching Exception.  In looking for best practice guidance for general coding questions such as this, I consult the JDK source itself.  Presumably the people who write the JDK are an authority on how the language was intended to be used.  If general catches were a bad practice, why are JDK authors doing it routinely? Don’t take my word for it.  Look for references to Exception in Eclipse and start auditing the search results.  Instances of global catches are too numerous to list here.  To save you time, MessageFormat (and many other java.text classes) and CompletedFuture (and many other java.util.concurrent classes) have global catch code.  Instances of catching RuntimeException, Error, and Throwable exist but appear to occur less often.

The JDK itself provides a way to supply handling logic for uncaught exceptions (as of JDK 1.5).  Classes implementing Thread.UncaughtExceptionHandler can be associated with any Thread or ThreadGroup.  Should a thread experience an uncaught exception, the JVM will automatically invoke the handler for the thread (or it’s thread group) if it was defined.  This feature can be used to supply logic to log exceptions (to somewhere other than the default standard error) or notify administrators in some other way.  Apart from the syntax difference, this really is a type of global catch.  Why provide this feature if it's a 'bad practice' to use it?  Unfortunately, exceptions logged at this level will have little knowledge of the context surrounding why the exception was generated.

Incidentally, it's not uncommon to farm global catches out to products that execute that global catch for us.  I'm thinking of Spring interceptors and handler adapters that are often configured to effectively issue global catches on our behalf and escort users to a standard error page.  Isn't this type of practice effectively a global catch, just with a different syntax? Isn't it hypocritical to configure products to issue global catches, but deliberately prohibit global catches directly?

Global catches combined with adding runtime context information has saved me boat-loads of time. In many cases, handling an exception either means logging that exception or recasting the exception in some way and including a meaningful context that might be useful to developers fixing the issue.  I routinely use ContextedRuntimeException from the Apache Commons Lang product for this purpose.  I’ve provided an illustration of this concept below (from the Apache Doc).

   try {
     ...
   } catch (Exception e) {
     throw new ContextedRuntimeException("Error posting account transaction", e)
          .addContextValue("Account Number", accountNumber)
          .addContextValue("Amount Posted", amountPosted)
          .addContextValue("Previous Balance", previousBalance)
   }

The stack trace output for the ContextedRuntimeException will report context information as well as the root cause.  For most reported exceptions using this technique, I’ve enough information to diagnose the bug or at least replicate the bug most of the time.  This technique *only* works when the exception and context are caught close to where the exception was generated. Also, it's important to record the original exception as a cause as it may have additional information useful to solving the issue.  Letting the JVM log the exception to standard error by default will not capture the information about the problem being captured here. Yes, it is possible that the additional information captured might not be valuable.  It is better to 'have' and not 'need' then the reverse.

The assertion that developers sometimes handle exception logic (code within a catch block) poorly and swallow information needed to diagnose problems is a fair point.  However, developers can insert poor exception handling logic for specific exceptions as well general exceptions.  Prohibiting global catches in all cases does not solve poor exception handling coding issues.  Best practices for exception handling logic is a topic that deserves more in depth treatment, perhaps in another blog entry.

There are reasons to catch specific exceptions.  Sometimes specific exceptions have additional information that should be captured.  Some exceptions represent business process errors or user errors and not unintended application exceptions. I'm not trying to advise that developers should catch Exception exclusively.  I do believe, it should be an option and not outright prohibited.

I would like to see two JDK improvements with exception handling.  Java needs a syntax to list “exceptions” for a global catch.  For instance, if I could easily code a catch that specified that I would like to catch all exceptions except instances of VirtualMachineError (out of memory conditions, JVM internal errors, etc.), the illegal catch rule would be easier to adhere to.  For example, I'd like to be able to specify a catch clause like:
catch all except(VirtualMachineError|LinkageError ex) {
    handleException(ex);
}

It would also help if it were possible to specify a default Thread.UncaughtExceptionHandler at JVM startup and supply notification logic for memory conditions and low-level virtual machine errors.  The default behavior of logging to standard error isn't wanted in many cases.  Note that it is possible to set the default exception handler for the entire JVM via Thread.setDefaultUncaughtExceptionHandler().