Don't Group Exception Catchers

Sometimes we rethrow exceptions. In Java we do this more often than in other languages, because it has checked exceptions. Sometimes we must catch and rethrow a few exceptions that originated from different places in a method. Java 7 introduced grouping of different types of exceptions in a single catch block. But even without the grouping, it is possible to just catch IOException or even Exception and provide a single catch block for all types and all originators (methods that throw). Recently I realized that this is a bad practice. Here is why.

Elephant (2003) by Gus Van Sant

Consider this Java method (I’m using Apache Commons IO):

byte[] read(String uri) {
  try {
    return IOUtils.toByteArray(
      new URL(uri).openStream()
    );
  } catch (IOException ex) {
    throw new IllegalArgumentException(ex);
  }
}

It’s not perfect. Let’s rewrite it to provide more error context, as was suggested earlier:

byte[] read(String uri) {
  try {
    return IOUtils.toByteArray(
      new URL(uri).openStream()
    );
  } catch (IOException ex) {
    throw new IllegalArgumentException(
      String.format(
        "Failed to read from '%s'",
        uri
      ),
      ex
    );
  }
}

Here, the exception may be thrown in three places:

No matter who throws, we catch it in the same catch block and rethrow with the same message. I believe that this is bad because the context of the error provided by rethrowing is less focused on the issue that occurred.

I would suggest this refactoring (I don’t close the input stream, which is wrong, but that’s a topic for a separate discussion):

byte[] read(String uri) {
  URL url;
  try {
    url = new URL(uri);
  } catch (MalformedURLException ex) {
    throw new IllegalArgumentException(
      String.format(
        "Failed to parse the URI '%s'",
        uri
      ),
      ex
    );
  }
  InputStream stream;
  try {
    stream = url.openStream();
  } catch (IOException ex) {
    throw new IllegalArgumentException(
      String.format(
        "Failed to open the stream for '%s'",
        uri
      ),
      ex
    );
  }
  try {
    return IOUtils.toByteArray(stream);
  } catch (IOException ex) {
    throw new IllegalArgumentException(
      String.format(
        "Failed to read the stream for '%s'",
        uri
      ),
      ex
    );
  }
}

This code is much longer, but at the same time more convenient to debug, test, and use in production mode. The catch block is able to explain the situation better and provide better context in the rethrown exception, because it deals only with a single case.

Thus, the rule I’m suggesting: if an exception is caught, each originator must have its own catch block.

Obviously, I believe that grouping exception types in a single catch block is bad practice.