Small Decisions Sometimes Cause Big Hacks

When .NET was being designed and introduced, it was notable not only in the ways in which it “borrowed” from Java, but also in the ways in which it parted from it. Some decisions were obviously good (dumping checked exceptions comes to mind) while some were clearly troubling (non-virtual methods by default).

Some, though, seemed insignificant at the time until you come up against them. Today’s example:

When is the stack trace for an exception generated?

In Java, the stack trace is generated when an exception is created, and in .NET it’s generated when the exception is thrown. In practical terms, with 99% of throws looking like this, it hardly matters:

throw new SomeException();

The creation and the throw actually happen on the same line of code. The stack trace is identical in both cases.

However, this seemingly small decision has rippling impact throughout the framework and languages. How many times have you seen this code and cringed?

catch(Exception ex)
{
    DoSomething();
    throw ex;
}

We’ve all seen it. That “throw ex” line destroys the original stack trace and replaces it with your own. The C# language team even had to invent a way to “re-throw an exception without destroying the original stack trace”, which is “throw;” on a line all by itself.

This probably should’ve been the clue that the wrong decision was made.

It’s reasonable to think that there might be a time when you have an exception object, outside of a catch block, and you want to re-throw that exception as though it were never caught. In fact, the framework team themselves came across just such a situation and invented their own hack to handle it, and built it into the Exception class.

When you are talking to a class via remoting, an exception thrown by the remote component is caught by the remote end and serialized back to the transparent proxy. The transparent proxy (which is what the client code is talking to) needs to experience that exception just as though it had been thrown locally. The problem is, we’re not in a catch block; we have a hydrated exception, but need to re-throw it without trashing the stack trace.

There is a private field inside of Exception into which you can stuff a stack trace which says “when I throw you, this is your stack trace; don’t make a new one”. This sample helper method shows you how to do the work:

public static void RethrowWithNoStackTraceLoss(Exception ex)
{
    FieldInfo remoteStackTraceString =
        typeof(Exception).GetField("_remoteStackTraceString",
                                   BindingFlags.Instance | BindingFlags.NonPublic);
    remoteStackTraceString.SetValue(ex, ex.StackTrace);
    throw ex;
}

I first ran across this trick via Peter Provost pointing me to Chris Taylor’s blog post on the subject. I’ve used it several times now (in ObjectBuilder and xUnit.net), and I’m thankful for it every time I run across it.

Since we’re talking about exceptions…

One of my biggest gripes about exceptions relates to reflection. The remoting team obviously went to great lengths to hide the fact that you were talking to a transparent proxy instead of the real object. That’s really the whole reason this hack exists in the first place. You can treat a transparent proxy just like the real thing.

It’s not so easy with Reflection. If you invoke something through reflection, and it throws an exception, the reflection infrastructure catches that exception and wraps it in TargetInvocationException. It’s basically an “I was here!” exception: the thing you actually care about it partially obscured by it. Nobody ever really cares about TargetInvocationException; it’s just a reminder that you’re doing things differently than normal, and you can’t treat some thing like you normally would.

I use this re-throw trick when reflection is an unimportant implementation detail (which it almost always is). For example, when running a test in xUnit.net, the test method is invoked via reflection; the person writing the test shouldn’t ever care about that detail. If every exception that was thrown in a test was reported as a TargetInvocationException, I’m pretty sure people would be telling us it was a bug.

So our implementation of TestCommand.Execute() uses this code instead:

public MethodResult Execute(object testClass)
{
    try
    {
        testMethod.Invoke(testClass, null);
    }
    catch (TargetInvocationException ex)
    {
        ExceptionUtility.RethrowWithNoStackTraceLoss(ex.InnerException);
    }

    return new PassedResult(testMethod, Parameters);
}

We originally spun in a loop at the point where we caught exceptions and turned them into FailedResult, stripping off any TargetInvocationExceptions that were contained. But that made testing for TargetInvocationException difficult in some cases, because you couldn’t differentiate between a TargetInvocationException you didn’t care about from one you did.

As such, everywhere that we use reflection, you’ll find a try/catch like this, eradicating the less-than-useless TargetInvocationException from the exception stack. Every time we have to write it, it makes me wish the reflection team had just done the right thing in the first place.

Comments

[Nate Kohari]

This gets even more irritating when you have a method like:

public object Foo()
{
try
{
return DoSomethingDangerous();
}
catch (Exception ex)
{
LogException();
ExceptionHelper.Rethrow(ex);
}
}

The compiler's static analysis will choke on this, since it doesn't analyze the Rethrow() method and recognize that it will always result in an exception being thrown. This means you have to rewrite the method as:

public object Foo()
{
object result = null;

try
{
result = DoSomethingDangerous();
}
catch (Exception ex)
{
LogException();
ExceptionHelper.Rethrow(ex);
}

return result;
}

While this isn't the end of the world, it's definitely less concise than it needs to be.

[Marc Brooks]

The advantage of building the stack trace at the throw-point is that you can build exceptions that are never actually thrown with a very low cost. The building of the stack trace is expensive, the building of the Exception object is cheap. Another advantage is that you can keep throwing that same Exception object everytime and get a new (correct) stack trace each time. This is extremely valuable for initialization errors in a Provider infrastructure like I talk about here:

http://musingmarc.blogspot.com/2005/11/pattern-for-rethrowing-exceptions.html

[Brad Wilson]

Marc,

You could get the best of both worlds by making StackTrace settable, and saying "A stack trace will be generated at throw time if there isn't already one there". You want to re-throw with a new stack, then set it to null and throw again.

That's a lot better than poking a private field via reflection, IMO. :)

[Benny]

This seems like a good Idé for a extension method, or am I wrong?

[Brad Wilson]

Yep, I agree this makes a good extension method off of Exception. :) Most of the things we end up putting in classes named XxxUtility tend to be things that probably end up working well as extensions off of Xxx. :)

[Ziv Caspi]

Now assume there wasn't a TargetInvocationException wrapping done by the Reflection layer. You wouldn't been able to differentiate exceptions thrown by that layer and by your own code. You really _do_ care, because otherwise the Reflection later can't tell you things go wrong reliably (there's always the option the reflected code throws exceptions that look like the Reflection layer's).
I would say that it's exactly this design decision which allows you to write that code reliably. If they simply have thrown exceptions around that looked no different than the reflected code's you'd have (rightly) complained about that.

[Brad Wilson]

Ziv,

I disagree. The faults in the reflection code itself could've thrown TargetInvocationException. Reverse the pattern. That was the right way to handle it. Wrapping everything up was not.

[Jeff Brown]

I agree with @Ziv and don't mind the TargetInvocationException except for the fact that we have to resort to hacks like this in order to rethrow its inner exception. That's the stinker. Java does similar things but at least you can rethrow the inner exception and be done with it, if that's what you want.

Anyways, thanks for writing this post! As a result I'm now using the following code in Gallio:

///
/// Rethrows an exception without discarding its stack trace.
/// This enables the inner exception of
/// to be unwrapped.
///
///
/// This implementation is based on code by Brad Wilson.
///
/// The exception to rethrow
[DebuggerStepThrough, DebuggerHidden]
[ReflectionPermission(SecurityAction.Assert, MemberAccess=true)]
public static void RethrowWithNoStackTraceLoss(Exception ex)
{
if (ex == null)
throw new ArgumentNullException("ex");

// Hack the stack trace so it appears to have been preserved.
// If we can't hack the stack trace, then there's not much we can do as anything we
// choose will alter the semantics of test execution.
if (RuntimeDetection.IsUsingMono)
{
MethodInfo method = typeof(Exception).GetMethod("FixRemotingException", BindingFlags.Instance | BindingFlags.NonPublic);
ex = (Exception) method.Invoke(ex, null);
}
else
{
MethodInfo method = typeof(Exception).GetMethod("InternalPreserveStackTrace", BindingFlags.Instance | BindingFlags.NonPublic);
method.Invoke(ex, null);
}

throw ex;
}

[Jeff Brown]

Ugh... looks like the blog software ate my XML doc comments. Didn't realize the comment field allowed HTML.

[Jose Arturo Cano]

Hi, I'm working in vb.net
If I use:
Try
....
Catch ex As Exception
RethrowWithNoStackTraceLoss(ex);
End Try

The stack trace is not preserved.
It works only if I use:

Try
....
Catch ex As Reflection.TargetInvocationException
RethrowWithNoStackTraceLoss(ex);
End Try

The problem is that "Reflection.TargetInvocationException" does not catch all the exceptions, only the ones thrown by reflected methods.

Does anyone knows a way for this to work catching all the exceptions?