23 February 2012

Exception management antipatterns - Episode I

Exception management is a tricky skill, and perhaps the most misunderstood topic of Java - and I think not only Java - programming. So, I will try to collect a series of posts about antipatterns in exception management: to provide a reference to anyone interested in the topic, of course, but also to analyze and deepen the issue myself.

My first post on the subject concerns a number of basic antipatterns:
  • Throw all away
  • Empty catch
  • Re-throw without cause
  • Log and rethrow
Throw all away
This is the simplest exception management antipattern we can (and shouldn't!) use, and is the tipical approach used when studying a new language: no exception management! While this can be a legitimate approach in training contexts - I'm learning Java IO API and will initially concentrate on the sunny day path, not on exception management - or in testing contexts - I'm writing a test that doesn't cover exception handling: another test will cover it! - it's not admittable in normal, production code... simply because without exception management one exception causes program termination.

public void aMethod() throws Exception {
   doSomething();
}

public void callerMethod() throws Exception {
   aMethod();
}

public void callersCallerMethod() throws Exception {
   callerMethod()
}

...

public static void main(String[] args) throws Exception {
    callersCallerscallersCallers...CallerMethod();
}

etc. etc. etc. ... when doSomething method raises an exception, it goes up through the entire stacktrace and causes program termination. Oh!

Empty catch
This is the antipattern which causes more headaches, as it hides exceptions instead of really managing them: unlike the previous antipattern, Empty catch makes very difficult bug finding and fixing, since it gives no information about the exception taht has occurred: the exception simple disappears, the current operation is not completed successfully, the user is not given any warning about the unespected application behaviour, and the developer has no useful information for analyzing the issue: great!

public void aMethod() {
   try {
      doSomething();
   }
   catch(Exception e) {
   }
}

Re-throw without cause
Another exception management antipattern I've often seen at work is the Re-throw without cause one: exceptions are catched, exception handling code constructs another, tipically custom, exception and throw it... without reference to the initial exception.
This can be useful in some specific contexts, such as code in layers that are called using a remote protocol: if you do not want to expose your internal exception through some type of serialization mechanism (RMI, SOAP, ...), you can throw another interface (known to the client) exception: this newly created exception exposes the problem in the client language.
In regular code, however, creating and throwing and exception without reference to the cause exception is something like Empty catch: it hides the real problem and causes headaches, both to users and debuggers, since it does not give sufficient information to analyze the problem.

public void aMethod() throws ABCException {
   try {
      doSomething();
   }
   catch(XYZException e) {
      throw new ABCException("Error!");
   }
}

Log and rethrow
This antipattern refers to both exception and logging management: it consists in catching an exception for the only purpose of logging it, and then rethrow it (possibly wrapped in another). This is an antipattern because it causes a stack-trace pollution in log file, wich makes more difficult to read and interpret it. Try to search debugging information in logs of an application which common exception management approach is Log and rethrow: you'll find a series of repeated stacktraces, one for each exception; look for significant informations in such a mess is more and more difficult than look for them in a repetition-free log file. Such an approach increases log size without adding significant information, with the effect of diluting thesignificant content.


public void aMethod() throws AnException {
   try {
      doSomething();
   } catch(AnException e)  {
      logger.log("Error!", e);
      throw e;
  }
}

public void bMethod() throws AnException {
   try {
      aMethod();
   } catch(AnException e)  {
      logger.log("Error!", e);
      throw e;
  } 
}
public void cMethod() throws AnException {
   try {
      bMethod();
   } catch(AnException e)  {
      logger.log("Error!", e);
      throw e;

  }

}
 
public void dMethod() throws AnException {
   try {
      cMethod();
   } catch(AnException e)  {
      logger.log("Error!", e);
      handleException(e);

  }

}
 
In this example the AnException is logged four times before it is actually handled, and the log is four times more verbose than necessary - and four time less readable...

3 comments:

  1. Your Log and rethrow example is clearly an antipattern, but in some cases (ex. distributed environment, remote calls etc. ) logging and throwing a different exception could rapresent a good practice in terms of separation, classloading issues and even security.

    ReplyDelete
  2. I second Sam's comment. And further, even in non-distributed environment you don't really know what the user's of your module/class will do. An app could just swallow your exception and not log anything; subsequent behavior could be unpredictable or have hard to debug consequences.
    -- Josef B.

    ReplyDelete
  3. Of all the anti-patterns, ThrowAllAway seems the least of all evils. I agree we should do better than throw all away, but 99% of the time the catch blocks make it even worse (rethrow without cause, empty catch).

    One proper use of log and rethrow might be to add contextual information (like a file path or generated SQL for example) that might not be available in the caught exception.

    ReplyDelete