JS Ext

Wednesday, April 24, 2013

Ignoring Exceptions like a Boss

As part of my analysis of the legacy code base, I decided to launch the application.  Although this is my first time diving into the application, I have been using the app every day for a few years now.  I knew what inputs to give it and what output to expect.  I tried executed the program and Eclipse told me it terminated immediately.  There was no output.  That is when I looked under the hood.  The main() method had a try/catch in it.  In the catch block, the toString() of the exception (not the exception itself) is logged.

Although the app uses Log4j, no default log4j.properties file was in the classpath.  It turns out the script that we use to invoke the JVM adds another folder with the log4j.properties file in it to the classpath.  I quickly added a simple log4j.properties file and ran it again.  This time I got an error message: "Null Pointer Exception".  That's it.  No stacktrace.  Instead of playing around with the try/catch, I decided to remove the try/catch and make the main() throw Exceptions.  I start the app again and I finally get a stacktrace.  It turns out there is another configuration file that the app needs.  I add the config file and move on.

As time goes on, more and more NullPointerExceptions get thrown.  It turns out, a bunch of methods have this same pattern; even the ones that return values.  For those, it returns null instead of passing the exception up the stack.  So many exceptions were ignored.  If the app was mostly functional, it wouldn't be a big deal.  The app is quite brittle, though.  The environment must be exactly correct or it fails spectacularly.  When it fails, though, you get dozens of exceptions to stdout.  It tends to be easy to figure out the real problem: you just look at the first exception, but it is still a mess.

For the methods that don't ignore exceptions, they translate the exceptions to new exception types.  The exception is logged at every "translation".  When those exceptions get thrown, I see a bunch of the same message (but with different exception class names) logged over and over again.

When implementing software, please try following these rules:

1) Don't just log/ignore an exception unless it is a non-critical exception
2) When logging exceptions, pass in the exception.  Don't just pass in the toString() of the exception
3) Don't log the exception over and over again as that exception goes up the stack

No comments:

Post a Comment

Note: Only a member of this blog may post a comment.