This is a mobile version, full one is here.
Yegor Bugayenko
30 August 2022
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.
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:
- By the constructor
of
java.net.URL
- By the method
openStream()
- By the method
toByteArray
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.