views:

277

answers:

5

In the j2ee applications I plan on standardizing the exception handling strategy. No more swallowing exceptions and basically converting all checked exceptions into unchecked (except for some that actually can be recovered from. The standard will be that all Exception will be thrown back up to the container (eg: http://www.onjava.com/pub/a/onjava/2003/11/19/exceptions.html). Exceptions such as SQLException cannot be recovered from, and if it occurs deep within the data layer I am going to force it back upward. However, small sub-routines such as email notifications will be wrapped in a try catch to prevent an exception occurring from breaking the entire application flow. (EG: Why stop an entire process from completing because the user didn't get an email notification that they didn't need anyway!)

Any advice on this? Again to reiterate I am basically re-throwing all exceptions back up to the container which has its own error page, which then logs it to log4j as well.

A: 

For DB Access exception I'd suggest looking at how Spring handles them. I have found this very easy to work with in practice.

Rich Kroll
A: 

Where do you get the idea that you can never recover from SQLException (among others)? In carefuly-written code, you often can. For a start, the whole SQLTransientException hierarchy is meant for exceptions that you may be able to recover from (such as SQLTransientConnectionException). It's a common mistake to treat exceptions as non-recoverable when they in fact can be handled.

I see nothing wrong with your cut your losses policy regarding minor errors like email being down. Of course, the user should be notified.

Matthew Flaschen
Ok.. I'll bite. Apart from retrying, how can you recover from an SQLException?
fforw
I am not familiar with being able to recover from SQLException in my environment. If I could I would. What causes our SQLExceptions is old, poorly developed j2ee apps receiving bad user input. That isn't recoverable. But I'll be on the look out like you said for treating all exceptions as non-recoverable, thanks.
Zombies
In many cases retrying is just what you need. Now, the type of recovery varies. For instance, sometimes you may need to close and reopen the connection. Other time, you just need to reexecute the query. Finally, if you want your app to be really robust, you could actually rewrite the query in terms of simpler constructs if you got, say, a SQLFeatureNotSupportedException ; probably, this last one isn't realistic.
Matthew Flaschen
A: 

Exceptions should be handled by the layer that concerns it. Because an exception is checked does not mean you can handle it or that you must handle it at every layer of your app. And it is at this point you may want to turn them into runtime exception. So you have exceptions you can ignore (log them), exceptions that you can't recover from and exceptions you can recover. The problem is the same exception type, for example SQLException, can enter in more than one category. IOException would also be another example. Programming errors that cannot be recovered should be runtime exceptions and should be caught close to the container for logging purpose or to have your app to fail "gently". Accessing an invalid index in a jdbc PreparedStatement is a programming error. Having connection problem is not. So at the data layer, you manage to make the distinction, then convert your unrecoverable exception into a runtime exception and throw it. And sometimes it does not mean that retrying to connect will work. Business methods should thake care of bussiness related exception only, that is why you don't handle NullPointerExceptions or ArrayIndexOutOfBoundsException everywhere in your code and you don't have to put them in your method signature. Same logic applies to a data source agnostic business method calculating the average salary of employees of dept., it should not throw SQLExceptions but should throw things such as InvalidDeptException if the user supplies a wrong dept. code. Throws declarations is also self documenting the code, if you throw your own runtime exceptions instead, be sure to document properly.

svachon
+1  A: 

I think the e-mail example is the best defense of checked exceptions. You have something that can go wrong and is transient, so the existence of a checked exception makes you think about it. If everything was a Runtime exception, your application would just blow up for trivial reasons and no one would think about it.

Anyway, if for your project, the general answer is throw it up to the top, then wrapping in a Runtime Exception is perfectly valid. A couple of things to think about:

Insist on Exception Chaining. The only reason to not do this is if you are serializing the exception, then things could get interesting with some exceptions containing non-serializable members. That doesn't seem to be the case, so don't allow exception swallowing. Use initCause if you have to. Here is a little helper method in our project:

public static <T extends Throwable> T initCause(T newException, Throwable cause) {
    return (T) newException.initCause(cause);
}

This helps people avoid the need to cast the exceptions.

I prefer avoiding unnecessary chaining of RuntimeException and Error Throwables that inevitably happen when there are many different Checked Exceptions on a method and the developer just catches Exception, so I recommend a static method that does something like this:

public static void throwRuntimeExceptionFromThrowable(Throwable e) {
    if (e == null) {
        return;
    } else if (e instanceof RuntimeException) {
        throw (RuntimeException) e;
    } else if (e instanceof Error) {
        throw (Error) e;
    } else {
        throw new RuntimeException(e.getMessage(), e);
    }
}

There are two options with this. One is to make it void (as this code does), which has the upside of the caller never forgetting to throw the result. The downside is if you have something like this:

public Object someMethod() {
    try {
        return getTheObject();
    } catch (Exception e) {
        throwRuntimeExceptionFromThrowable(e);
    }
}

The compiler won't like you. You will have to return after the catch, and declare a variable before the try.

The other option is to return the Exception as a RuntimeException (just throw the Error) and have the caller do the throw. The compiler is happier, but the caller may forget to do it, and call the method without a throw in front of it, and then the exception gets swallowed.

Yishai
A: 

In general, I'd recommend following the Fault Barrier pattern in your case - it's a further elaboration on the approach that you are planning to follow. Check it out here: http://www.oracle.com/technology/pub/articles/dev2arch/2006/11/effective-exceptions.html (see an older review of it).

For the email, not sure about your particular circumstance. Overall this sounds like a perfect case for AOP. If you're in JEE5, then a custom @Interceptor may be in order - common "non-interrupt" logic due to runtime exceptions may be coded there, and you can bind it to multiple beans/methods afterwards.

anikitin