This is the second installment in a three-part series on why FxCop warns against catch(Exception):FAQ: Why does FxCop warn against catch(Exception)? - Part 1FAQ: Why does FxCop warn against catch(Exception)? - Part 2FAQ: Why does FxCop warn against catch(Exception)? - Part 3
On Wednesday, I explained why catch (Exception) is a bad idea, and many of you replied with interesting comments. Since it was too cumbersome to write everything I had to say in the comments section, I’ve replied to your comments below.
K: “Do you have a list of the 'unchecked' exceptions?”
No. I don’t have a definitive list of the exceptions which should always be prevented and never caught. The best advice I can offer here is to follow the API documentation on a case-by-case basis. Don’t immediately assume that you need catch blocks for each of the exceptions that are listed in the documentation for a particular API. Instead, for each exception, ask you yourself if there’s a simple way to prevent it. If there is, then put in the preventative code and leave out the catch block.
This can be tough to discern in some circumstances. For example, it may seem that a call to File.Exists can prevent FileNotFoundException, but there’s a race condition where the operation will still fail if the file is deleted in between the time File.Exists returns true and the call is initiated. Also, there are some methods in the .NET Framework which misuse exception types. For example, in an ideal world, you should never catch ArgumentException, but Enum.Parse throws it to indicate that the input does not represent one of the enum values. Since there’s no way to prevent that short of rewriting Enum.Parse, you have to consider ArgumentException ‘checked’ in that specific case.
If you’re unsure about a specific API, try asking for help on http://forums.microsoft.com
Gill: “… there is one issue that troubles me: what if my program handles 10 specific exceptions I defined in the same way (when all these exceptions hint at different problems that are related), in this case I would want some catch blocks to perform the same code. If these exceptions don't share a common base, there is no way to make all of them perform the same code short of using the same lines (or method call) in each catch block.”
I don’t think there are that many methods out there which throw 10 unrelated and unpreventable exceptions, but I can relate to your frustration with API which throw unrelated exceptions to indicate related failures. Refactoring the shared handler code in to a helper method is the simplest way to deal with the issue. If you find yourself catching the same set of exceptions from the same method in more than one place, consider refactoring out the entire try/catch/catch/… series in to a helper method of its own.
Gill: “Furthermore, if all developers worked correctly and used this guideline my code might be facing unexpected exceptions from 3rd party code bugs. If this 3rd party isn't available to fix the bug, my program can fail at unexpected times.”
I don’t follow the reasoning here. If you hit a bug in 3rd party code, then catch (Exception) is just going to make things worse. You should treat this unhandled exception no different from any other. It may turn out that you can work around the problem by using the API differently or adding a catch handler for the specific exception type that was raised if it turns out to be recoverable.
Richard: “Of course, it would help if the Exception class was abstract, and the Framework never threw a general Exception! For example, try calling System.Drawing.ColorTranslator.FromHtml("#aaax")”
Youch! You’re right; it’s absolutely terrible to throw an instance of System.Exception and FxCop warns against that as well. I will do my best to argue for reopening and fixing this bug!
Anonymous: “If my program has a feature to auto-save unsaved work every 10 minutes, and the auto-save procedure (which may use a third-party library) throws an unchecked exception, are you suggesting that the program should simply crash out, because this exception must not be handled?”
Yes, that is in fact exactly what I’m suggesting. A bug is a bug is a bug. It doesn’t matter if it’s in your code or 3rd party code, catch (Exception) will just make it harder to find and fix. Having an auto-save feature is a great way to mitigate the risk of data loss. However, if the auto-save code itself is buggy, then all bets are off. It’s sort of like discovering that your parachute won’t open…
Jeff: “Well here is my slight problem with this. The best example I can give you as to why you absolutely have [to] Catch(Exception ex) to do this is a bug I reported back in Beta 2 … The Browser Control …”
Chris: “… the WebBrowser control will silently eat the NullReferenceException. I want to know when if my code has a bug, but the WebBrowser hides my bugs.”
That’s awful! I will talk to the owners of the Web browser control about getting this fixed. One of the things that I failed to mention in my original post is that it’s much worse to swallow exceptions in library code than in application code. If you’re developing a reusable library, this rule (like many others in FxCop) is doubly important!
Pop Catalin Sever: “Let the entire application crash? Are you kidding? That's really unacceptable. And btw if you notify the user that something bad has happened in most cases he will help out recover the application or some data. If you just let the application close I think that very likely you're going to be in big trouble.”
Let the application continue to run in an indeterminate state and carelessly corrupt data? Are you kidding? ;)
While I agree that the user should be notified that something has gone wrong, I don’t agree that the application should continue running afterwards. Instead, a friendly dialog should allow the user to send an error report before the application closes. If data loss is a potential issue in your application, then implement an auto-save feature. It’s worth noting that this is exactly the approach that’s used in Microsoft Office.
Beginning in .NET 2.0, you don’t need to implement the crash dialog box yourself. If you let the exception go unhandled, a default dialog box will be displayed that allows the user to send an error report to Microsoft. Note that for Windows Forms applications, you need to call Application.SetUnhandledExceptionMode(UnhandledExceptionMode.ThrowException) to make this work.
If you prefer, you can hook the AppDomain.UnhandledException event and display your own dialog that allows users to send a bug report directly to you.
Jeremy: “Here are another three cases where you absolutely must catch Exception (and at the very least provide for adequate logging, even if you then rethrow):
1. In every method you spin up in a new thread. 2. In every method called arbitrarily on threads you don't control (ie. WebMethods, methods exposed via remoting, delegates passed across remoting, windows service control overrides, etc.) 3. Main Seeing a pattern here? :)”
It sounds like you got burned by the default exception handling policy for secondary threads in .NET 1.0 and 1.1. Thankfully, the situation is vastly improved in .NET 2.0. See http://msdn2.microsoft.com/en-us/library/ms228965.aspx.
If you can’t move to 2.0 just yet, then a better approach than sprinkling catch (Exception) in all of these places is to hook AppDomain.UnhandledException (and Application.ThreadException for Windows Forms applications). In your handler, you can throw up a dialog like the default dialog in .NET 2.0 and call Environment.Exit afterwards.