FxCop backlog themes: Exceptions
Since I started monitoring traffic on this blog a little more closely about a week ago, I had the unexpected surprise that the posts on HTML encoding and server vs. client cultures were getting a lot more hits than I expected. I had been planning on starting a series of "how to" posts on those topics this weekend, but that was before David Kean from the FxCop team was kind enough to direct a bunch of folks my way with a post about my recent FxCop posts. Since it would seem that I've now got quite a few new subscribers who are interested in FxCop, I figured I might as well continue on that theme for a while...
Jumping the gun
I've got exception stuff on the brain this morning, so that's what I'm going to write about. However, if you have decided to tackle an FxCop backlog, exceptions are probably not the first major theme you should cover. Then again, even assuming that my recent posts have inspired you to immediately start a backlog winnowing project, you should still be at least a month away from covering any major theme. I'm going to try to write about the much easier disposition and finalization topic within the next couple of weeks. Heck, it might even happen tomorrow morning if I don't get coerced into some sort of housework. In the meantime, just keep in mind that the sequence in which I end up covering the topics here isn't necessarily the sequence in which you'll want to cover them in your backlog process.
Starting from scratch
Unless your team already has very specific guidelines in place around the generation and handling of exceptions (along with training and code reviews to reinforce those guidelines), it's pretty safe to assume that there is a very wide variety of exception treatment in your code base. In addition, the developers on your team likely have similarly disparate understandings of when and how exceptions ought to be generated and handled. Instead of jumping right into covering the existing FxCop rules around exceptions, it would probably be a much better idea to start with some training around the basics of exceptions. Chances are excellent that individual developers will be carrying quite a bit of baggage based on their experiences with exception treatment in other languages or platforms, and you will most likely need to devote quite a bit of time to dispelling inappropriate preconceptions.
Here are a few resources that you may wish to consult during your exception training:
- The .NET Framework Design Guidelines for Exceptions
Some parts are a bit outdated, and the formatting doesn't necessarily make for easy reading, but this is probably still the best place to start.
- The .NET Framework Developer's Guide to Handling and Throwing Exceptions
Some parts are very basic, but the Best Practices for Handling Exceptions section is good complement to the design guidelines.
- Krzysztof Cwalina's posts on exceptions
Most of the information contained in these posts ought to be in the official design guidelines documentation on MSDN. Unfortunately, since the latter content doesn't seem to get updated all the often, you'll need to read Krzysztof's posts as well.
- The FxCop team FAQs on which exception types to throw and why you should catch System.Exception
By a stroke of luck, the "what to throw" FAQ came out just as we were wrapping up the exceptions topic at FinRad, which saved me the trouble of writing a very similar set of guidelines for internal consumption.
- Rico Mariani's posts on the costs of exceptions
This content is helpful if you're trying to understand the cost of exception handling, but watch out for a bit of performance-oriented bias. If you're concerned about the usability of your APIs, you'll probably end up wanting to throw exceptions a little more often than Rico might like. ;)
- Chris Brumme's post on the exception model
Interesting as it is, this isn't likely to be required reading for most folks on your team. However, it is something you might want to read if you're going to be the one giving the training on the exception topic.
If you're concerned about globalization and localization issues, you might also want to have a look at my post on client vs. server cultures. The disparity between application user and administrator languages can present challenges when emitting localized exception messages for any application, and it's something you may wish to consider as you develop your internal exception generation guidelines, even if you don't happen to be writing web applications.
The built-in rules
As part of the exceptions theme, you'll want to start activating the FxCop rules that touch on exception generation and handling. The following table lists the rules available in FxCop 1.35 and Visual Studio 2005 static analysis:
I suppose that it could be argued that Usage.DoNotIgnoreMethodResults might also be worth including while activating exception rules. If your team is resistant to the idea of throwing exceptions instead of using failure return codes, then a bit of time cleaning up violations of this rule might be just the thing to convince them. Otherwise, it's probably safe to activate this rule whenever it otherwise seems reasonable during your backlog process. If you're following the guidelines, you'll probably want to fix most violations of this rule by modifying the invoked method to throw exceptions rather than return a status code, which is a breaking change for public methods, and scheduling breaking changes can be a wee bit tricky.
Custom rules
If you've read the design guidelines concerning exceptions, it should be pretty obvious that the above eight rules don't exactly cover all the potential problems for which you might want to screen. Here are a few custom rules for exceptions that we're either already using at FinRad or will be introducing once we get a little further along in the backlog cleanup:
- Do not throw System.NotImplementedException
Pretty obvious, although you'll occasionally need to create temporary exclusions, particularly for when someone checks in a class skeleton to be fleshed out by somebody else. Also, as a bit of side note, when you introduce this guideline, you might want to stress that the exception signifies that the code is not implemented yet, not that it's not implemented here. For "not implemented here", you should be using System.NotSupportedException instead.
- Methods that throw System.NotSupportedException should not include other instructions
Simple as it might seem, this rule is a bit tricky to author since the VB compiler has the nasty habit of adding extra instructions (e.g.: return statements) to methods. Also, one needs to keep in mind that a custom message might be provided for the exception, and that message might be retrieved via a method call.
- Throw System.ComponentModel.InvalidEnumArgumentException for an invalid enum argument value
Not really relevant to the rule itself, but it's really annoying that this particular exception lives in the System.ComponentModel namespace rather than System.
- An argument exception should be thrown directly from the method whose argument is being validated
I had hoped that the parameter name verification on Usage.InstantiateArgumentExceptionsCorrectly would cover this issue but, after looking at the rule logic via Reflector, I've decided that a custom rule is going to be need for this one. I'm planning on activating it at the same time as Design.ValidateArgumentsOfPublicMethods (which will definitely be happening before we move to Orcas <gdr>).
- Use recommended exception logging methods
In order to make logged exceptions more noticeable, we decided to create a new helper method that is capable of writing exception details to both a central repository and whatever target location was previously used. The decentralized logging problem was the consequence of an application architecture that you may not share, so this might not be particularly relevant for your team. However, if you do have similarly decentralized logging, you may want to consider a similar approach. If you do decide to add a special exception logging method, then you'll probably also want to add a custom rule to verify that it's being used.
- Review use of System.Diagnostics.Debug methods
Some folks have a tendency to use Debug.Assert or Debug.Fail where they really ought to be throwing exceptions.
- Review catch without throw
Last but definitely not least, this is our "do not eat exceptions" rule. Unfortunately, fixing an undesirable violation of this rule can be very time-consuming, but discovering places where an application is hiding exceptions has tremendous impact on reliability, and the short-term cost is well worth the long-term benefit.
Personally, I'd love to see at least the more generally useful rules from the above list make it into the rule set shipped by Microsoft. I seem to remember seeing something from the code analysis team that promised some new rules around exceptions for Orcas, but I can't find it anymore, and I haven't seen any new exception rules either. It's a pity, because I was really hoping to get rid of my custom rules, particularly the less than ideal verification for other instructions in methods that throw System.NotSupportedException...
1This rule is not available in Visual Studio 2005, but it is shipped with both FxCop 1.35 and Orcas beta 1.