FAQ: Why does FxCop warn against catch(Exception)? - Part 1 [Nick Guerrera]

This is the first installment in a three-part series on why FxCop warns against catch(Exception):
FAQ: Why does FxCop warn against catch(Exception)? - Part 1
FAQ: Why does FxCop warn against catch(Exception)? - Part 2
FAQ: Why does FxCop warn against catch(Exception)? - Part 3

This question comes up a lot, and I think there’s a lot of confusion and controversy about the rule. Many people seem to think that it’s noisy, annoying, and not worth their time. I find that very unfortunate because following its guidance can really help you build better software. Here’s my attempt to convince the Nay-Sayers that catch(Exception) is almost always a terrible idea.

I’ve heard a lot of arguments against DoNotCatchGeneralExceptionTypes, but they generally boil down to one or both of the following:

• “If I call an API which doesn’t document which exceptions it can throw, then I have no choice but to catch all possible exceptions!”
• “My application needs to be robust; it can’t crash just because of an exception that I forgot to handle!”

Both arguments are ill-conceived because they presume that you can handle an exception even if you don’t know what failure it represents or how it impacts the code that will execute next. There’s an implicit assumption in both arguments that the mere act of catching an exception is equivalent to handling it appropriately, which simply isn’t true.

For the sake of the discussion to follow, exceptions can be divided in to two general categories:

1. Those that signal bugs (i.e. API misuse), or catastrophic system failures
   (e.g. ArgumentException, NullReferenceException, ExecutionEngineException.)
2. Those that signal unpreventable yet recoverable error conditions.
   (e.g. FileNotFoundException, SocketException.)

In languages like Java, which have a notion of checked exceptions, only exceptions of the second kind are eligible to be checked while exceptions of the first kind must be left unchecked. On that basis and for lack of better terminology, I will hereafter refer to exceptions in the first bucket as ‘unchecked’ and exceptions in the second bucket as ‘checked’. (Note the quotes.) I am using the terms loosely to mean that if exceptions were checked in .NET, then those in the first bucket would remain unchecked while those in the second bucket could become checked.

Independent of where you stand on the checked exception debate, this distinction is very important to hold in your mind when writing exception handling code. In particular, do you really ever want to catch the ‘unchecked’ exceptions at all? Probably not! Instead, you should prevent them from occurring in the first place. For example, check if a variable is null before dereferencing it, don’t catch (NullReferenceException).

If a bug in your code manifests itself as an exception (and most of the ‘unchecked’ exceptions are just that), then it’s best to let the application terminate right at the point of failure. When the bug is discovered, you’ll get exactly the diagnostic information you need to fix it quickly, add the appropriate regression tests, and move on. On the other hand, debugging a system which aggressively swallows general exceptions feels like you’re one of the police investigators in “CSI: Miami”. The culprit has left the crime scene and you have to work backwards from the subtle clues in your program’s incorrect behavior.

And that’s the principal reason to avoid catch (Exception): it hides bugs in your code. These bugs are far more costly to resolve and since they can go undetected, the quality of your software suffers.

Some readers might be thinking that it would be helpful if the .NET Framework exception class hierarchy reflected the ‘checked’ vs. ‘unchecked’ nature of exceptions. In fact, it’s rather seductive to imagine a base class, say System.CheckedException, for all of the ‘checked’ exceptions. The argument follows that you could then replace catch (Exception) with catch (CheckedException)… and the problem would be solved since you no longer catch those insidious ‘unchecked’ exceptions. Unfortunately, this approach would be just as flawed. FxCop would still warn against catch (CheckedException) because it’s still far too general. By definition, ‘checked’ exceptions are unpreventable and must therefore be caught. Nevertheless, catching them is the easy part; the hard part is deciding what to do next, and that decision is purely a function of the specific exception type and it’s meaning in the given context. For example, to recover from FileNotFoundException for your configuration file, you might use a default configuration, and to recover from a SocketException, you might fall back to a local store until the network becomes available again. However, if you don’t know what to do about it, then let it go.

Returning to the first argument against this rule, please don’t get me wrong, I definitely empathize with the need for accurate exception information in API documentation. However, in the absence of such documentation, the safest approach is to treat all API as
“innocent until proven guilty.” Wait until you have hard evidence that a particular exception can be raised before you attempt to handle it.

Also, be sure to test your exception handling code. For example, delete a file out from underneath your application while it’s running and see how it deals with FileNotFoundException. It’s simply not sufficient to write optimistic and untested code, just because the exception is mentioned in the API documentation. I can’t tell you how many times I’ve seen untested catch handlers fail to work as expected. The classic example is a formatting exception raised in the error message that’s never provoked from a test. Spurious and unnecessary catch (Exception) blocks can completely torpedo your code coverage! (Remember: if it’s not tested, it’s broken.)

One important thing to note is that if you follow our advice and catch fewer exceptions, then exceptions will naturally travel further up the call stack. Sometimes they’ll terminate the application, but other times someone further up the call stack will know what to do and your only job is to clean up after yourself in finally blocks as the stack unwinds.

To summarize, here are some important things to consider when dealing with exceptions in managed code:

1. Do not catch preventable (‘unchecked’) exceptions.
   Only catch the specific unpreventable (‘checked’) exceptions that you are certain can be raised and that you know how to handle.
2. Test your exception handling code.
3. Use finally blocks liberally for clean-up code to keep your state as consistent as possible in case an exception that you cannot handle needs to be handled further up the stack.

And finally, I’d like to point out that our team has learned this lesson the hard way. There are actually far too many catch (Exception) blocks in FxCop itself and it has caused us a great deal of grief. We’re working on removing them and I’m confident it will help us to deliver better software!