Exception Handling
Recently we've been discussing exception handling at Imaginet with Joel,
Some of the basic premises of exception handling that someone else came up with was the following:
- If a method cannot do what it says it will do, it should throw an exception. Returning a bool is not appropriate.
- Only catch exceptions that you can handle. Throw everything else to the caller. (At the top of the call stack you have to catch all exceptions, the user is the only one who can handle it at that point.)
- Log exceptions only if you do not intend to throw them - no need to log the same exception multiple times.
- Try not to cause unnecessary exceptions to be thrown - use TryParse instead of Parse, use File.Exist() before File.Open()
Here's my additional thoughts on exception handling.
About not logging the exception if you are going to rethrow it: the issue for me is whether to wrap and throw the exception in another exception or whether to throw the original exception. There is very little value in rethrowing the original exception if that is all you are going to do - the notable exception is that it is easier to put a breakpoint and debug an app with a catch/throw statement in it.
However, logging or wrapping/throwing exceptions is about recording the context of the method call - what stored proc I was attempting to call, what parameters were passed into the method, and so on. This is useful diagnostic information that can help to debug the problem in production without looking at the code and stepping thru execution in production or prod-support environments. Here you might rethrow the original exception but log the exception along with the values of the method parameters or local variables and such, so the support person has something to work with. You may choose to wrap an exception in a new excpetion and include some of that diagnostic information in the exception - "Unable to open the file 'c:\joel.txt'" is more useful than "Unable to open file" or whatever the more generic exception message is. On the other hand, there are times (especially when crossing a security boundary) where you want to throw a much more generic exception or message to the client (ie of a webservice for example) than pass through the additional diagnostic details of the original exception to presumably a hacker. In that case it is imperitive that you log the exception details on the secure side (or just stupid if you don't).
Remember that the Message property of an Exception is supposed to be human-readable. I'd like to add that we should always attempt to have the Message be "user understandable" as opposed to "geek/dev consumable". That proves tough to do in reality. If there is other "geek-speak" stuff that would be useful, put them in other properties of the exception (derive a new exception class for these properties) and if you have a top-level exception handler that shows the exception in a dialog, make sure you have a mechanism to show the "user-readable" property (ie. Message) of the Exception, plus a "More..." button that shows the "geek-speak" properties - again, watch out for releasing data to users/hackers that might be used against you.
The real developer voodoo is understanding what possible exceptions could occur in any of your code and try to code defensively (such as TryParse and File.Exists examples) or behave intelligently when those exceptions do occur. You don't want to propogate exceptions up the call tree if you can handle them lower. But there's no point in catching them lower if you don't do anything with them. Something that I have noticed that helps is to document what exceptions that the code you write will throw intentionally - then the caller can look at the docs and see what could occur. That gives the developer who calls your code some idea of what could come at them. (How far down that tree do you go tho? Do I document the possible exceptions that could occur in the code that I am calling but that I am not handling in my code? I generally don't.)
The $10,000 question is what to do with exceptions at the top of the call stack - do you show the user and give them options to retry or abort? How can they make that decision? The web service call timed out - does that mean the user can try again right away? Maybe...or maybe the webservice is completely down, or the configuration file contains a bad url, or something else. Basic services like that I now like to put a "HelloWorld" type method into them that can be called during application initialization - a failure then will likely mean that there are problems that should be solved before the app can run, and the app should not continue. But a successful call there eliminates the possibility of configuration problems, or database access, or whatever. Later network issues that cause exceptions can then say "Hey, I tried and couldn't get through to the server. We'd better shut down now." instead of having some kind of user-influenced retry process which gets difficult. Is the network slow to timeout or are we just failing? How can a user know that? So typically, again I look for categories of exceptions and try to test them up front during initialization, and then if they occur later, most top-level unhandled exceptions end up getting shown to the user, recorded in a log, and the application exits. Or if I know the users are sophisticated in general, I will give them the option of "retrying" but warn them at their peril.