Beware the dangers of generic Exceptions

Catching and throwing generic Exceptions can get you into trouble quickly and quietly

While working on a recent project, I found a piece of code that performed resource cleanup. Because it had many diverse calls, it could potentially throw six different exceptions. The original programmer, in an attempt to simplify the code (or just save typing), declared that the method throws Exception rather than the six different exceptions that could be thrown. This forced the calling code to be wrapped in a try/catch block that caught Exception. The programmer decided that because the code was for cleanup purposes, the failure cases weren't important, so the catch block remained empty as the system shut down anyway.

Obviously, these aren't the best programming practices, but nothing seems to be terribly wrong...except for a small logic problem in the original code's third line:

Listing 1. Original cleanup code

private void cleanupConnections() throws ExceptionOne, ExceptionTwo {
   for (int i = 0; i < connections.length; i++) {
      connection[i].release(); // Throws ExceptionOne, ExceptionTwo
      connection[i] = null;
   }
   connections = null;
}
protected abstract void cleanupFiles() throws ExceptionThree, ExceptionFour;
protected abstract void removeListeners() throws ExceptionFive, ExceptionSix;
public void cleanupEverything() throws Exception {
   cleanupConnections();
   cleanupFiles();
   removeListeners();
}
public void done() {
   try {
      doStuff();
      cleanupEverything();
      doMoreStuff();
   }
   catch (Exception e) {}
}

In another part of the code, the connections array is not initialized until the first connection is created. But if a connection is never created, then the connections array is null. So in some cases, the call to connections[i].release() results in a NullPointerException. This is a relatively easy problem to fix. Simply add a check for connections != null.

However, the exception is never reported. It is thrown by cleanupConnections(), thrown again by cleanupEverything(), and finally caught in done(). The done() method doesn't do anything with the exception, it doesn't even log it. And because cleanupEverything() is only called through done(), the exception is never seen. So the code never gets fixed.

Thus, in the failure scenario, the cleanupFiles() and removeListeners() methods are never called (so their resources never release), and doMoreStuff() is never called, thus, the final processing in done() never completes. To make matters worse, done() is not called when the system shuts down; instead it is called to complete every transaction. So resources leak in every transaction.

This problem is clearly a major one: errors are not reported and resources leak. But the code itself seems pretty innocent, and, from the way the code was written, this problem proves difficult to trace. However, by applying a few simple guidelines, the problem can be found and fixed:

  • Don't ignore exceptions
  • Don't catch generic Exceptions
  • Don't throw generic Exceptions

Don't ignore exceptions

The most obvious problem with Listing 1's code is that an error in the program is being completely ignored. An unexpected exception (exceptions, by their nature, are unexpected) is being thrown, and the code isn't prepared to deal with that exception. The exception isn't even reported because the code assumes the expected exceptions will have no consequences.

In most cases, an exception should, at the very least, be logged. Several logging packages (see the sidebar "Logging Exceptions") can log system errors and exceptions without significantly affecting system performance. Most logging systems also allow stack traces to print, thus providing valuable information about where and why the exception occurred. Finally, because the logs are typically written to files, a record of exceptions can be reviewed and analyzed. See Listing 11 in the sidebar for an example of logging stack traces.

Logging exceptions is not critical in a few specific situations. One of these is cleaning resources in a finally clause.

Exceptions in finally

In Listing 2, some data is read from a file. The file needs to close regardless of whether an exception reads the data, so the close() method is wrapped in a finally clause. But if an error closes the file, not much can be done about it:

Listing 2

public void loadFile(String fileName) throws IOException {
   InputStream in = null;
   try {
      in = new FileInputStream(fileName);
      readSomeData(in);
   }
   finally {
      if (in != null) {
         try {
            in.close();
         }
         catch(IOException ioe) {
            // Ignored
         }
      }
   }
}

Note that loadFile() still reports an IOException to the calling method if the actual data loading fails due to an I/O (input/output) problem. Also note that even though an exception from close() is ignored, the code states that explicitly in a comment to make it clear to anyone working on the code. You can apply this same procedure to cleaning up all I/O streams, closing sockets and JDBC connections, and so on.

The important thing about ignoring exceptions is ensuring that only a single method is wrapped in the ignoring try/catch block (so other methods in the enclosing block are still called) and that a specific exception is caught. This special circumstance distinctly differs from catching a generic Exception. In all other cases, the exception should be (at the very least) logged, preferably with a stack trace.

Don't catch generic Exceptions

Often in complex software, a given block of code executes methods that throw a variety of exceptions. Dynamically loading a class and instantiating an object can throw several different exceptions, including ClassNotFoundException, InstantiationException, IllegalAccessException, and ClassCastException.

Instead of adding the four different catch blocks to the try block, a busy programmer may simply wrap the method calls in a try/catch block that catches generic Exceptions (see Listing 3 below). While this seems harmless, some unintended side effects could result. For example, if className() is null, Class.forName() will throw a NullPointerException, which will be caught in the method.

In that case, the catch block catches exceptions it never intended to catch because a NullPointerException is a subclass of RuntimeException, which, in turn, is a subclass of Exception. So the generic catch (Exception e) catches all subclasses of RuntimeException, including NullPointerException, IndexOutOfBoundsException, and ArrayStoreException. Typically, a programmer does not intend to catch those exceptions.

In Listing 3, the null className results in a NullPointerException, which indicates to the calling method that the class name is invalid:

Listing 3

public SomeInterface buildInstance(String className) {
   SomeInterface impl = null;
   try {
      Class clazz = Class.forName(className);
      impl = (SomeInterface)clazz.newInstance();
   }
   catch (Exception e) {
      log.error("Error creating class: " + className);
   }
   return impl;
}

Another consequence of the generic catch clause is that logging is limited because catch doesn't know the specific exception being caught. Some programmers, when faced with this problem, resort to adding a check to see the exception type (see Listing 4), which contradicts the purpose of using catch blocks:

Listing 4

catch (Exception e) {
      if (e instanceof ClassNotFoundException) {
         log.error("Invalid class name: " + className + ", " + e.toString());
      }
      else {
         log.error("Cannot create class: " + className + ", " + e.toString());
      }
   }

Listing 5 provides a complete example of catching specific exceptions a programmer might be interested in. The instanceof operator is not required because the specific exceptions are caught. Each of the checked exceptions (ClassNotFoundException, InstantiationException, IllegalAccessException) is caught and dealt with. The special case that would produce a ClassCastException (the class loads properly, but does not implement the SomeInterface interface) is also verified by checking for that exception:

Listing 5

public SomeInterface buildInstance(String className) {
   SomeInterface impl = null;
   try {
      Class clazz = Class.forName(className);
      impl = (SomeInterface)clazz.newInstance();
   }
   catch (ClassNotFoundException e) {
      log.error("Invalid class name: " + className + ", " + e.toString());
   }
   catch (InstantiationException e) {
      log.error("Cannot create class: " + className + ", " + e.toString());
   }
   catch (IllegalAccessException e) {
      log.error("Cannot create class: " + className + ", " + e.toString());
   }
   catch (ClassCastException e) {
      log.error("Invalid class type, " + className
         + " does not implement " + SomeInterface.class.getName());
   }
   return impl;
}

In some cases, it is preferable to rethrow a known exception (or perhaps create a new exception) than try to deal with it in the method. This allows the calling method to handle the error condition by putting the exception into a known context.

Listing 6 below provides an alternate version of the buildInterface() method, which throws a ClassNotFoundException if a problem occurs while loading and instantiating the class. In this example, the calling method is assured to receive either a properly instantiated object or an exception. Thus, the calling method does not need to check if the returned object is null.

Note that this example uses the Java 1.4 method of creating a new exception wrapped around another exception to preserve the original stack trace information. Otherwise, the stack trace would indicate the method buildInstance() as the method where the exception originated, instead of the underlying exception thrown by newInstance():

Listing 6

public SomeInterface buildInstance(String className)
     throws ClassNotFoundException {
   try {
      Class clazz = Class.forName(className);
      return (SomeInterface)clazz.newInstance();
   }
   catch (ClassNotFoundException e) {
      log.error("Invalid class name: " + className + ", " + e.toString());
      throw e;
   }
   catch (InstantiationException e) {
      throw new ClassNotFoundException("Cannot create class: " + className, e);
   }
   catch (IllegalAccessException e) {
      throw new ClassNotFoundException("Cannot create class: " + className, e);
   }
   catch (ClassCastException e) {
      throw new ClassNotFoundException(className
        + " does not implement " + SomeInterface.class.getName(), e);
   }
}

In some cases, the code may be able to recover from certain error conditions. In these cases, catching specific exceptions is important so the code can figure out whether a condition is recoverable. Look at the class instantiation example in Listing 6 with this in mind.

In Listing 7, the code returns a default object for an invalid className, but throws an exception for illegal operations, like an invalid cast or a security violation.

Note: IllegalClassException is a domain exception class mentioned here for demonstration purposes.

Listing 7

public SomeInterface buildInstance(String className)
     throws IllegalClassException {
   SomeInterface impl = null;
   try {
      Class clazz = Class.forName(className);
      return (SomeInterface)clazz.newInstance();
   }
   catch (ClassNotFoundException e) {
      log.warn("Invalid class name: " + className + ", using default");
   }
   catch (InstantiationException e) {
      log.warn("Invalid class name: " + className + ", using default");
   }
   catch (IllegalAccessException e) {
      throw new IllegalClassException("Cannot create class: " + className, e);
   }
   catch (ClassCastException e) {
      throw new IllegalClassException(className
        + " does not implement " + SomeInterface.class.getName(), e);
   }
   if (impl == null) {
      impl = new DefaultImplemantation();
   }
   return impl;
}

When generic Exceptions should be caught

Certain cases justify when it is handy, and required, to catch generic Exceptions. These cases are very specific, but important to large, failure-tolerant systems. In Listing 8, requests are read from a queue of requests and processed in order. But if any exceptions occur while the request is being processed (either a BadRequestException or any subclass of RuntimeException, including NullPointerException), then that exception will be caught outside the processing while loop. So any error causes the processing loop to stop, and any remaining requests will not be processed. That represents a poor way of handling an error during request processing:

Listing 8

public void processAllRequests() {
   Request req = null;
   try {
      while (true) {
        req = getNextRequest();
         if (req != null) {
            processRequest(req); // throws BadRequestException
         }
         else {
            // Request queue is empty, must be done
              break;
         }
      }
   }
   catch (BadRequestException e) {
      log.error("Invalid request: " + req, e);
   }
}
1 2 Page
Recommended
Join the discussion
Be the first to comment on this article. Our Commenting Policies
See more