Java exception handling design

Question

I am trying to design a simple exception handling strategy for web services middleware using some ideas from here: http://northconcepts.com/blog/2013/01/18/6-tips-to-improve-your-exception-handling/.

However I think it is worth it to have two exception classes inheriting from RuntimeException instead of just one.

The idea is to use one class to represent general faults (NPE, etc.) that can't be handled in place, and my strategy is to catch and log them in a single fault barrier so developers/devOps can troubleshoot them. These exceptions should be wrapped and passed back hiding some implementation details to the users of the API?

The other class will represent the exceptions that should be passed back to the users of the API since they are expecting them and know how to handle them (say specific exceptional conditions that are part a web services API).

Any drawbacks of using these two exception classes or is it better to use just one class for everything?

Thanks.

Answer 1

I don't think this is a good idea. This introduces coupling in a very non-obvious way.

Consider a class or a service. It has clearly defined tasks, it's methods are well documented, it can be swapped out for something else that has the same interface/contract.

Suppose it encounters an exception. The service then can decide whether it is recoverable or not. That, in itself, is fine - if the service determines a failure is recoverable, it should recover. End of story.

Your version is quite different. In your question you are saying "The service decides whether the situation is recoverable by the caller". This is bad. How does your service know that? Why does it care for who calls it? Is it only callable from a very specific other class?

You mention NPEs as "general failure". It might not be. It might be a misconfiguration or bad data sent from a bad user, not programmer errors (Some may argue that it is a programmer error to not account for that possibility, but that's not the point here). Is an IO exception a "general failure"? It might mean either that you need to use a backup connection (simple recovery), or someone ripped out the network adapter from a live system (not that simple to recover from). None of your classes, save the very last thread exception handler can decide whether the app can recover from something or not.

If you change the caller code to deal with some type of exception that was previously "unrecoverable", how do you know that you need to change the part that throws this exception?

The other point I disagree with you on is:

...inheriting from RuntimeException...

...should be passed back to the users of the API since they are expecting them...

I don't think this is a good usage of unchecked exceptions.

If the users of the API are supposed to be expecting these exceptions, why not make them checked and enforce it? Then unchecked exceptions will propagate all the way up and may be logged in a specific way - that will mean exactly what you want - None of the calling code was expecting or could deal with this exception. While "expected" exceptions will have to be dealt with, even if the calling code insists that all it does is log it (or even silently swallows it).

This has the extra benefit that API changes are documented! You can add more "expected exceptions" if they are runtime - and none will notice until it's already live! Whilst with checked once you won't be able to compile if you don't deal with the situation.

Answer 2

The idea is to use one class to represent general faults (NPE, etc.) that can't be handled in place, and my strategy is to catch and log them in a single fault barrier so developers/devOps can troubleshoot them. These exceptions should be wrapped and passed back hiding some implementation details to the users of the API?

It sounds to me like you are doing this wrong. Don't wrap general faults in your code in other exceptions. That'll just make it harder for users of your API to figure out what's going on. Don't create your own fault barriers or log exceptions. The entire web application should have one fault barrier which logs all exceptions. All that you can accomplish by adding your own is logging things twice. Don't add another one for your middleware.

Answer 3

The idea is to use one class to represent general faults

There are no "general" faults. NPE, for example, can be thrown manually, so you can't even tell if it's intentional or not. There are also misused exception classes or something you just don't know about. How would the "wrapping" look in the code? I imagine:

try {
    //...
} catch (ExceptionType1 e) {
    throw new MyException(e);
} catch (ExceptionType2 e) {
    throw new MyException(e);
} catch (...) {
    //...
} catch (ExceptionTypeN e) {
    throw new MyException(e);
}

I've seen lots of that - it is horrible to maintain.

my strategy is to catch and log them in a single fault barrier

Just catching and logging is not always the right way. Here are the rules I try to use:

• If exception can be handled, it should be. No need to tell a user about it, unless it can only be handled by a user.
• If it cannot be handled, then:
  • If you can provide additional information about the context - wrap it. Wrapper exception should accept initial exception as it's cause (getCause()), so stack trace is not broken.
  • If you cannot provide additional information - rethrow it, because it may be handled somewhere above.

Top-level catching is an extreme measure in case something really unexpected happened. I consider it a safety net which prevents showing ugly stack traces to users, who can potentially have malicious intents.

Exceptions should be logged as soon as possible, providing helpful message with enough information about a context to figure out what's happened. If exception is caught and handled, no need to log full stack trace - simple warning would be enough.

Consider logging error message and stack trace with different warning levels: e.g. ERROR for message, DEBUG for stack trace. That way you can keep your logs clean, turning DEBUG logging level only when needed to see a full details. Also you can put log messages with different log levels in separate log files.

Answer 4

I'd stick with one. Anytime you have an exception that isn't catastrophic, it isn't an exception. Log it to warn or error level then move on. Otherwise, you've just reinvented checked exceptions.