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);
   }
}

A better way to handle request processing is to make two significant changes to the logic, seen below in Listing 9: First, move the try/catch block inside the request-processing loop. That way, any errors are caught and handled inside the processing loop, and they do not cause the loop to break. Thus, the loop continues to process requests, even when a single request fails. Second, change the try/catch block to catch a generic Exception, so any exception is caught inside the loop and requests continue to process:

Listing 9

public void processAllRequests() {
   while (true) {
      Request req = null;
      try {
         req = getNextRequest();
         if (req != null) {
            processRequest(req); // Throws BadRequestException
      }
         else {
            // Request queue is empty, must be done
              break;
         }
      }
      catch (Exception e) {
         log.error("Error processing request: " + req, e);
      }
   }
}

Catching a generic Exception sounds like a direct violation of the maxim suggested at the start of this section—and it is. But ours is a specific, special circumstance. In this case, the generic Exception is being caught to prevent a single exception from stopping an entire system. In situations where requests, transactions, or events are being processed in a loop, that loop needs to continue to process even when exceptions are thrown during processing.

In Listing 9, the try/catch block in the processing loop can be considered the top-level exception handler, and the top-level exception handler needs to capture and log all the exceptions that rise to that level of the code. That way, the exceptions are not ignored and lost, but the exceptions do not interrupt the rest of the requests that need to process.

Every large, complex system has a top-level exception handler (perhaps one per subsystem, depending on how the system performs processing). The top-level exception handler is not intended to fix the underlying problem causing the exception, but it should be able to catch and log the problem without stopping processing. This does not imply that all exceptions should be thrown at this level. Any exception that can be handled at a lower level, should be: that is, the exception should be handled where the logic exists to know more about the conditions when the problem occurred. But if an exception can't be dealt with at a lower level, go ahead and throw it all the way up. That way, all those unrecoverable errors will be handled at one place (at the top-level exception handler), instead of throughout the system.

Don't throw generic Exceptions

The overall problem presented in Listing 1 started when the programmer decided to throw a generic Exception from the cleanupEverything() method. Code can get pretty messy when a method throws six different exceptions: the method declaration becomes unreadable, and the calling method is forced to catch those six different exceptions, as shown in Listing 10:

Listing 10

public void cleanupEverything() throws
      ExceptionOne, ExceptionTwo, ExceptionThree,
      ExceptionFour, ExceptionFive, ExceptionSix {
   cleanupConnections();
   cleanupFiles();
   removeListeners();
}
public void done() {
   try {
      doStuff();
      cleanupEverything();
      doMoreStuff();
   }
   catch (ExceptionOne e1) {
      // Log e1
   }
   catch (ExceptionTwo e2) {
      // Log e2
   }
   catch (ExceptionThree e3) {
      // Log e3
   }
   catch (ExceptionFour e4) {
      // Log e4
   }
   catch (ExceptionFive e5) {
      // Log e5
   }
   catch (ExceptionSix e6) {
      // Log e6
   }
}

But even if the code is a little messy, at least it is explicit. Using the specific exception avoids a couple of very real problems: Throwing a generic Exception hides the details of the underlying problem, thus removing the opportunity to deal with the real problem. Further, throwing a generic Exception forces any call to that method to either catch that generic Exception (which has problems, as discussed previously) or propagate the problem by rethrowing that generic Exception.

Typically, when a method declares it is throwing a generic Exception, it does so for one of two reasons: In one case, the method calls several other methods that might throw many different exceptions (like the Mediator or Façade design patterns) and hides the details of the exception conditions. Instead of creating and throwing a domain-level exception (to wrap the low-level exception), the method simply declares that it throws Exception and ignores the issue. Another situation is where the method instantiates and throws a generic Exception (throw new Exception()) because the programmer has not given thought to which exception should actually be used to express the situation.

Both of these issues can be addressed with a little thought and design: what detailed, domain-level exception should really be thrown? The design might involve simply declaring the method to throw the exceptions that might really occur. Another option is to create a domain-level exception to wrap the thrown exceptions and declare that. In most cases, the exception (or set of exceptions) that a method throws should be as detailed as possible. The detailed exceptions provide more information about the error conditions and thus allow the situation to be handled or at least logged in detail.

The generic Exception class is a checked exception, meaning that any call to a method that declares that it throws Exception must either declare that it throws Exception itself, or wrap the method call in a try/catch block that catches the generic Exception. I explained the problems with this approach previously.

Use generic Exceptions carefully

This article has explored several facets of dealing with generic Exceptions: they should never be thrown, and they should never be ignored. They should rarely (only under specific circumstances) be caught. They do not provide the detailed information that allows you to handle them effectively, and you can wind up catching exceptions you didn't mean to.

Exceptions are a powerful part of Java that, when used properly, can make you a more effective programmer and shorten your development cycle, especially when testing and debugging. When exceptions are used poorly, they will work against you by hiding problems in your system. Watch where and how you use generic Exceptions.

Paul Philion has been developing in Java professionally since JDK 1.0a2. Paul is a Sun Certified Java Programmer, Developer, Architect, and Web Component Developer. Currently he is the principal architect at Acme Rocket Company, where he designs enterprise systems, tinkers with new technologies, and expresses his opinions (loudly). When he's not in the office writing code (and documentation!), he's at the beach, with a glass of wine, writing code.

Learn more about this topic

Join the discussion
Be the first to comment on this article. Our Commenting Policies