Exceptions in java, rethrown

Question

In some legacy code I see this, that an overbroad exception is being caught, and then thrown again, Is this a good practice? Does throw e; rethrow the same exception, or create a new one ?

catch (Exception e) {
        StringBuilder sb = new StringBuilder(
                            "Oops. Something went wrong with id: ");
        sb.append(id);
        sb.append(". Exception is: ");
        sb.append(e.toString());
        System.out.println(sb.toString());
        throw e;
}

Answer 1

throw e is rethrowing the same exception. At least it preserves the original stacktrace. It's just writing a message to stdout recording some information about what happened, then letting the original exception proceed on its way.

It's not a great practice, it should be enough to log the exceptions in a central place, recording their stacktraces. If you need to add more information (as in the example, where it logs an id), it's better to nest the original exception in a new exception as the cause, then throw the new exception. I would guess this probably occurs in contexts where there is no centralized logging or where exceptions tend to get eaten somewhere.

Answer 2

My best guess would be it is trying to have two layers of protection. Making sure that an error message is displayed and also asking the client to handle the exception the way it wants to because the catch clause is not doing anything to recover from the exception.

I won't consider it good/bad practice. Depending on your requirements you can consider to go either way like there might be 100 of different clients using your API and each one of them has a different way of recovering from an exception. By displaying what went wrong it is adding a default action layer just below the layer where client decides on how it will handle the exception.

Now back to your question. I think throw e throws the same exception object. As exceptions are objects in java you need to create a new exception object before you can throw it which I can't see happening in your code.

Answer 3

This is usually a bad practice. catch(Exception e) (sometimes called Pokemon Exception Handling for when you gotta catch 'em all) catches every single exception. Such exception handling is rarely useful because:

• It catches runtime exception too.
• You lose information about the type of exception that was thrown.
• You cannot react to or handle specific exceptions.

Your method signature now is public void whatever() throws Exception, which is rarely useful. Now everything further up the chain has no idea what kind of exception you have thrown; they will have to do instanceof checks which defeats the purpose of catching specific-exceptions entirely.

As far as your second exception is concerned, throw e; throws the same exception object. If you wanted to wrap the exception, you can create a new one, which means you would do something like throw new MyCustomException(e);. You would also need to change your method signature.

If there is nothing further up the chain, I guess this isn't as bad (still isn't great, though). It looks like a method that is trying to log all exceptions that are thrown. However, again, there are better ways of doing this.

Answer 4

It can be a good practice. I think I always use conversion to RuntimeExceptions during prototyping. After this if there is a need one can change this into better exception handling. For this my purpose there is an utility class in Guava called Throwables which makes exception propagation.

In your case however the exception should be converted into a runtime exception, because declaring a method throwing a general Exception is just the same as a method throwing RuntimeException for the calling party. In the first case it 'catches everything', in the latter it 'catches anything'. I have not yet experienced the difference between two of these in real-world applications. So I prefer RuntimeExceptions as they require less typing.

Catching Exception

• Checked exceptions (IO exceptions, security errors, concurrency, etc)
• Runtime exceptions (anything, unpredicted garbage, see below)
• Everything - these are 99% of all errors (there are Errors left however)

Catching RuntimeException

• null pointer exceptions, index out of bounds exceptions, access exceptions, + API which wraps propagates exceptions into RuntimeException - this is ALSO A LOT

My point is after when you're catching an Exception you can't really handle all these cases. So it makes no difference except for the less typing for the calling party if you wrap it into a RuntimeException.

Answer 5

throw e does throw the same exception. There might have been reasons for doing this, but there is also a reason not to. In your example code, a message is sent to System.out, but if a stack trace were printed later on System.err, it won't be syncronized, and in fact the two might end up interwoven in your console.

A better approach would be the following:

catch (Exception e) {
    StringBuilder sb = new StringBuilder(
                        "Oops. Something went wrong with id: ");
    sb.append(id);
    sb.append(". Exception is: ");
    sb.append(e.toString());
    throw new Exception(sb.toString(), e); // The original exception is an argument
}

This will create a new Exception with the modified message, and append the original Exception to the stack trace to help with debugging.