Archive for July 14th, 2006

Unintentionally Squashing Exceptions

Friday, July 14th, 2006

At my last job we had a utility method that created a SysException (the one and only application exception) given a message and an optional cause exception. The primary method (there are several overloads) was:

public static SysException createSysException(String message, Throwable t) {

   SysException e = null;
   if (t == null) {
      e = new SysException(message);
   }
   else {
      if (t instanceof SysException) {
         // don't create a new exception,
         // just add message and fill stack trace
         e = (SysException) t;
         ...
         e.fillInStackTrace();
      }
      else if (t instanceof RemoteException) {
         RemoteException re = (RemoteException)t;
         e = new SysException(message, re.detail);
      }
      else {
         e = new SysException(message, t);
      }
   }

   return e;
}

and this is certainly decent code. By centralizing exception generation, we could log the errors, do validation, etc. But the crux of the problem is that the usage pattern was:

try {
   ...
}
catch (MyException e) {
   log.error("uh oh", e);
   throw ExceptionUtils.createSysException("uh oh", e);
}

Again, decent code. Except that it could have been:

try {
   ...
}
catch (MyException e) {
   log.error("uh oh", e);
   ExceptionUtils.createSysException("uh oh", e);
}

and the compiler would have been just as happy. And given the current implementation of createSysException(), there is no automatic logging, so unless the caller throws the generated exception, it gets squashed. That’s why I was mucking around in the code – there were several places where the exception wasn’t thrown and execution was allowed to proceed, with occasional unintended consequences.

This made debugging harder since it was an intermittent problem. We lost the original information from the cause exception, and because the execution didn’t stop after the call to ExceptionUtils.createSysException, variables were null and we’d failing on a NullPointerException. This lead us to the problem but it took a while to find the true cause of the original problem.

So my solution was to (in addition to solving this particular exception’s cause) change

public static SysException createSysException(String message, Throwable t) {

to

public static void throwSysException(String message, Throwable t) throws SysException{

The method would be identical, except it would throw the exception instead of returning the exception and allowing the caller to disregard it. Lucky me, I only needed to change ~500 files.

Creative Commons License
This work is licensed under a Creative Commons Attribution 3.0 License.