Wednesday, August 19, 2009

Back when I was a VB developer, we used to wrap every single function with an exception handler. We would catch it and write it to a log. More sophisticated versions even featured a stack trace. Yes, really, we were that advanced :) The reason we did that was simple, VB didn’t have structured exception handling and if your application threw an unhandled error it simply crashed. There was no default way of knowing where the exception had taken place.

.NET has structured exception handling, but the VB mindset of wrapping every piece of code in a try-catch block, where the catch catches System.Exception, is still common, I see it again and again in enterprise development teams. Usually it includes some logging framework and looks something like this:

Catching System.Exception is the worst possible exception handling strategy. What happens when the application continues executing? It’s probably now in an inconsistent state that will cause extremely hard to debug problems in some unrelated place, or even worse, inconsistent corrupt data that will live on in the database for years to come.

If you re-throw from the catch block the exception will get caught again in the calling method and you get a ton of log messages that don’t really help you at all.

It is much better to simply allow any exceptions to bubble up to the top of the stack and leave a clear message and stack trace in a log and, if possible, some indication that there’s been a problem on a UI.

In fact there are only three places where you should handle exceptions; at the process boundary, when you are sure you can improve the experience of the person debugging your application, and when your software can recover gracefully from an expected exception.

You should always catch exceptions at the process boundary and log them. If the process has a UI, you should also inform the user that there has been an unexpected problem and end the current business process. If you allow the customer to struggle on you will most likely end up with either corrupt data, or a further, much harder to debug, problem further down the line.

Improving the debugging experience is another good reason for exception handling. If you know something is likely to go wrong; a configuration error for example, it’s worth catching a typed error (never System.Exception) and adding some context. You could explain clearly that a configuration section is missing or incorrect and give explicit instructions on how to fix it. Be very careful though, it is very easy to end up sending someone on a wild goose chase if you inadvertently catch an exception you were not expecting.

Recovering gracefully from an expected exception is, of course, another good reason for catching an exception. However, you should also remember that exceptions are quite expensive in terms of performance, and it’s always better to check first rather than relying on an exception to report a condition. Avoid using exceptions as conditionals.

So remember: the first rule of exception handling: do not do exception handling. Or, “when in doubt, leave it out” :)

I agree with you 100%. I don't think it's always necessary to always handle exceptions at process boundaries though. For example, if you are building a rich-client application that connects to a server via remoting, any exception that occurs during a server call will seamlessly bubble all the way to the top-level exception handler on the client.

For once Mike, I can safely say I disagree with nearly every word you have written here. I think you are fundamentally wrong in your approach to exception handling. Not handling exceptions is almost as bad as trying to hide them.

The first rule of exception handling should be that no exception should ever be unexpected. One of the things I like about Java is that it forces the programming to either directly handle every exception that the methods it calls can raise, or to state its intent to re-raise them within the method signature. Most other languages do not have this feature, but the programmer should still be aware of every exception that could occur during a method's execution and should make a decision on what to do about all of them.

Take an example of a method that opens a connection to a database, writes some data to it and shuts the connection. Connecting to the database and writing to it could cause all sorts of exceptions. Masking those exceptions and not reporting failure is the worst thing that the code can do. The next worst thing is for it to blindly make the connection and write the data, without a regard to any exceptions that might occur. This simply shifts responsibility for handling exceptions up the stack to ever higher methods which have ever less knowledge of what might have caused the exception.

Far better would be for the method to log the exception and then return false to say it couldn't save the data. The calling code then need concern itself solely with the database write succeeding or failing, rather than with a mass of exceptions which it shouldn't need to know about.

The only exceptions which ought to be allowed to bubble up to the very top of the application are "shit happened" exceptions like running out of memory, from which it is extremely difficult, if not impossible, to recover.

There is also an important boundary across which bubbled exceptions should never be allowed to cross, except in fully documented ways. That is of course the component/ package/ library (call it what you will) level. If I ship a re-distributable component, I expect that component to (a) handle all exceptions that occur within it or (b) document which exceptions it may throw and under what circumstances they will occur.

I challenge you to back up your claim that MOST Java programmers dislike checked exceptions. I have never personally met one that does. I'd also expect anyone who does take such a view to write sloppy, lazy code. Anders Hejlsberg basically summed up why most other languages don't do checked exceptions. Checked exceptions were designed to force people to do exception handling properly. Sloppy, lazy programmers just find ways around it though, or end up with messy, hard to read, hard to debug code. So other language developers took the view that it was better to not have checked exceptions as it didn't encourage good programming practice in bad programmers. In my view, that is the wrong decision and punishes good programmers by denying them a useful tool just because bad programmers may misuse it.

I found bobince's response in the StackOverflow piece particularly telling. He claims "The thing about checked exceptions is that they are not really exceptions by the usual understanding of the concept. Instead, they are API alternative return values." I'm at a loss to understand where he gets that "usual understanding of the concept" from, other than through an ignorance of the history of exception handling. Exceptions ARE alternate return values: they are exceptional return values indicating the result is outside some pre-defined "normal" range and so need to be handled in some exceptional way.

He also claims "In every other language I can let an IO exception happen, and if I do nothing to handle it, my application will stop and I'll get a handy stack trace to look at. This is the best thing that can happen." This is complete crap. Any program that dies with a stack trace just because an it can't write to a file say should have its listing printed out, wrapped around an iron bar, which should then to used to bludgeon the author into a bloody pulp (metaphorically speaking of course). Presenting a stack trace to a user is utterly inexcusable. It is the WORSE thing that can happen. Even dying with no message is preferable to spewing up the guts of a program at the user's feet with a stack trace.

Having read through the Stack Overflow piece, I'm tending toward the simple conclusion that people object to checked exceptions because of sloppy practices and/ or because they have never been taught how to handle exceptions well.

I tend to disagree with you in a small way. Bubble up the exception if you need to. The problem I have is freeing up external resources on exception. Things like database connections, streams, and other resources need to be cleaned up less you end up with nice memory leaks especially if the app is going to continue running after an exception. The finally block is your friend in this case. It also fosters good coding practices of cleaning up after yourself.

Of course it would be very nice if no exception is ever unexpected, but unfortunately for Morts like me unexpected exceptions, "shit happened" as you so eloquently put it, are a fact of life.

In .NET we don't have Java style checked exceptions (thanks Richard) so it's very hard to know what unexpected exceptions might be thrown by a dependency. What I'm ranting against is wrapping every single block of code in a try{ ... }catch(Exception){ ... } statement. It is far better to allow unexpected exceptions to bubble up to the process boundary and get logged and reported.

I don't get your point with the database connection example. If an unhandled exception is thrown during a data-access operation, execution will end, the stack will pop to the top of the process, and your logging framework will record it. It cannot blindly carry on as you put it. In fact, blindly carrying on is exactly what I'm arguing against.

If I consume a third party component, of course I'd like it to only throw documented exceptions. But, and this is the crucial point, if it does throw an unexpected exception I want to know about it. What it shouldn't do is simply bury the exception and pretend that nothing bad has happened, leaving me scratching my head as to why it's not working as expected.

i think many people do this so that they can see (log) what exceptions are being throw and where. (of course they must rethrow as you say)

for me this is a big omission from CLR. It should be possible for me to add an 'exception thrown' handler that gets called everytime an exception is thrown, telling me the stack and the exception object, greate for debugging

Thought your post was excelent. I have also felt very emtional, about this topic because of the classic try catch, within a try catch and then throwing a new exception. I think you explained quite clearly. If you cant do something about the exception dont catch it, and if that makes for a ugly UI, perhaps you have bigger problems, than exception handling.

A peice of code I recently found in our system helpfully captures configuration error gracefully, however due to cut-and-paste reuse the user is always advised that the missing configuration 'should contain an email address' :)

I honestly believe that disabling the paste function in a development environment would actually increase code quality!

Some time ago I had to deal with what you define a "genius programmer" at my old workplace who sprinkled every single method in our application, plus all the libraries connected to it with the exception hiding antipattern, such as try -> catch (log) -> resume. It gave the rest of the team such headaches, and even when we tried to explain him about why this is such a horrible practice he still claimed that it was better that way because the user shouldn't know an error has occurred, and a half-loaded page with inconsistent state is better than the classic user friendly error page. Crazy stuff. Fail Fast is one of the most important rules in software development, as one of the other commenters said. Luckyly I didn't have to work on that stuff for a long time, and I heard the project found salvation later on, when they managed to clean the code of that mess and everything got back on track.

I think some of the criticisms of this post are largely missing the message, I think the headline message Don't exceptions, is not what the OP is saying at all in reality, and of course you handle exceptions where the encapsulation or abstraction requires you handle the exception. I'm assuming the post takes these points by Bloch as a given

I don't think the post is contradicting Joshua Bloch, I think it trying to add detail, but the detail can be interpreted differently by different programmers.

The only place where I differ with the OP. Is that I think checked Exceptions form a powerful tool for enforcement of robust code.

I hate when every line is wrapped with a try/catch, but what you said is why i think it happens. Yes, we can log the data as we're junking through it to get the values, but ideally we'd have a way to know what the data was that failed. In my experience it's almost always data that causes the failure. When working with batch apps you're often chugging through huge files and 1 messed up record could cause the entire thing to fail and if you don't know it can kind of be painful to figure it out. That being said, all these try/catches I see around every line destroys the readability and indent of the code. I'll take some data issue hunting over that personally. Or write to a memory log as you go and write it to file in the one master exception handler so it gets written only in the case of an error.