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:
try { // do something } catch (Exception exception) { Logger.LogException("Something bad just happened", exception); }
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” :)
Posts
21 comments:
"More sophisticated versions even featured a stack trace."
Wow! I once tried to write a sophisticated logging mechanism in VB6; it ended up being as much code as the actual app and a total PITA to maintain. Eventually we just ripped it out.
I'd love to hear how you implemented that.
No way John, I had the VB part of my brain surgically removed back in 2002.
Yup,this is one of my pet peeves. When debugging it can take hours to find some genius developer swallowed the context of an error. These days I tend to debug with Break On All Exceptions activated
You should also Google Fail-Fast, that was the subject of my first ever blog post and it goes hand in hand with this post
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.
Nice post.
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.
@David. Actually most people hate the checked exceptions feature that Java has. Nice idea in theory- in practise a terrible idea.
As usual there is some great discussion on checked exceptions on stackoverflow- http://stackoverflow.com/questions/613954/the-case-against-checked-exceptions
Most other languages don't have that feature for a good reason- it sucks in practise.
@Paul. Break on all exceptions is a great thing to turn on if you have inherited someone else's code base.
It can truly save your sanity. :-)
@Richard OD,
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.
Hi David,
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
Damn,
I wrote another comment and lost it. Key points:
@David- here's another interesting SO post:
http://stackoverflow.com/questions/912965/what-are-the-pros-and-cons-of-checked-exception
top 2 upvoted answers:
1) Checked exceptions usually misused.
2) checked exceptions a failed experiment.
I'm pleased you think checked exceptions are great- I don't. I certainly wouldn't accuse anyone of being a sloppy programmer because they don't hold the same opinions as you.
@Mike- check out Redgate's Exception Hunter if you haven't already- http://www.red-gate.com/products/Exception_Hunter/index.htm
@Richard, thanks for the further SO link. I feel a blog post of my own coming on, especially as I feel Anders Hejlsberg's arguments against checked exceptions are inexcusably poor.
I'll post a link here if/ when I get around to writing it :)
@everyone
consulting the GOOD Java programmers bible as written by his hollyness Joshua Bloch
* Item 39: Use exceptions only for exceptional conditions
* Item 40: Use checked exceptions for recoverable conditions and run-time exceptions for programming errors
* Item 41: Avoid unnecessary use of checked exceptions
* Item 42: Favor the use of standard exceptions
* Item 43: Throw exceptions appropriate to the abstraction
* Item 44: Document all exceptions thrown by each method
* Item 45: Include failure-capture information in detail messages
* Item 46: Strive for failure atomicity
* Item 47: Don't ignore exceptions
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.
Good advice Mike,
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!
Andy,
+1 on disabling paste :)
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.
Part of the issue with C# error handling is the lack of clarity in the stack traces.
After writing a large scale application with the idea of errors bubbling to the process boundary, I'm not having to pick apart code to find out why a variable cast to Int32 won't work.
I know its down to the value not parsing correctly, but I don't know what the value is, nor any other supporting information as the stack trace just doesn't tell you.
I know the line, and the error, and that it involves a failed cast, but not the value which was being cast, and there's no way to get to it other than wrapping with try/catch.
I would speculate that the vast majority of people who throw try/catch blocks in all over the code are trying to save themselves the time later on.
I wish I'd done that now....
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.
@Alex
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.
Post a Comment