views:

73

answers:

3

My colleagues are seasoned C++ hackers switching to .Net. One of the mistakes that they make unintentionally is writing code like this:

catch(ArgumentExcepttion ae)
{
    // Code here logs the exception message
    // And this is supposed to re-throw the exeception
    throw ae; // as opposed to throw;
    // But, as we all know, doing this creates a new exception with a shorter stack trace.
}

I have seen this done in many many places. I cannot really think of a situation where cutting off the stack trace would be useful. I think that should be exceptional situation that deserves a comment. Correct me if I am wrong. If the stack trace is to be cut, I think it is always better to do:

throw new ArgumentException("text", ae /* inner exc */);

Anyhow, what I want to do is detect all such cases and give a warning. A regular expression search is of no help, because of this:

catch(Exception e)
{
    Exception newExc = new Exception("text", e);
    Log(newExc);
    throw newExc;
}

I would have to use a tool such as StyleCop (which I have, version 4.3.3.0 ). I am using VS2008 for now, but will be switching to VS2010 very soon.

Any thoughts on how to accomplish what I am looking for?

+1  A: 

Is the code catching exceptions unnecessarily? If you are only interested in logging the exception, then you only need an catch at the top level of your code (at the last possible point where you can do the logging). This could seriously reduce the number of catches you have to worry about.

ShellShock
This is a good point, but for some reason our logging framework expects us to supply the location of where it happened. Catching locally seems like an obvious solution. Sometimes the Exception-handling logic is even more complicated.
Hamish Grubijan
OK, so pass the exception to your logging framework, which can then use the Exception.StackTrace to get the location of the error.
ShellShock
+1  A: 

I would suggest to look for catch-blocks ending in a throw ...; instead of ending with throw;.

Although you get some false positive, you can filter them out by hand.

Henri
+4  A: 

FxCop has a rule for this: RethrowToPreserveStackDetails

Once an exception is thrown, part of the information it carries is the stack trace. The stack trace is a list of the method call hierarchy that starts with the method that throws the exception and ends with the method that catches the exception. If an exception is re-thrown by specifying the exception in the throw statement, the stack trace is restarted at the current method and the list of method calls between the original method that threw the exception and the current method is lost. To keep the original stack trace information with the exception, use the throw statement without specifying the exception.

I believe FxCop Analysis is built in to VS2010 but I'm not 100% sure...

Here is the Microsoft download link for FxCop.

Austin Salonen
Cool...this is called Code Analysis in VS2008. I guess this rule has been carried over from FxCop.
ShellShock
Wait, how do you get to Code Analysis from VS2008?
Hamish Grubijan
@ShellShock -- getting your attention. Where is Code Analysis in VS2008?
Austin Salonen
In the VS Team System edition. The expensive one. VS2010 Prof doesn't have it either, Premium at least.
Hans Passant
Ok, once you have the expensive edition ... how do you get it?
Hamish Grubijan
Project Properties, Code Analysis tab.
ShellShock