Share via

Why you really want to avoid catching and rethrowing exceptions

  • Article

I like processes that simply drop-dead fail when they have an unrecoverable fault.  Trying to continue is often dangerous and unlikely to actually help anyone.  This policy is all fine and well but in all cases it is vital that the “death” stack of a process be the true problem, not a generic error handler.  Those kinds of handlers ruin the lives of developers who are trying to analyze error logs, and effectively make services like Watson useless.  I can’t stress enough how critical it is to NOT mess with the failure stack.

I’ve seen many of these in the wild, including of course Microsoft code, and it means that instead of getting a nice actionable stack in the log (which would have been a Watson report in the field) you have to go back to the code and try to repro.  In the case below I stared at the source for a while trying to figure out what was likely wrong and it turned out the true problem was quite distant from the likely candidates.

2010/04/05 11:02:14 AM  BEGIN    Peek_Range TEST
2010/04/05 11:02:14 AM  ---- Exception in [StackTest] thrown type=[TestCodeException], eip=[00000004`4716C93E], message=[Index (1) is out-of-range [0..1]!]  

That was the failure right there, and that stack was the one I needed! Nothing after that should have happened... But... execution continues until finally....

2010/04/05 11:02:15 AM  Test completed exceptionally [Peek_Range]: Index (1) is out-of-range [0..1]!

The stack reported at this point is useless… the real problem was millions of cycles ago.  I'm not even including the stack from the log because it's just stupid.

The real stack can be extracted with a repro (at great pain)

And it ends up looking something like this (cleaned up)

StackTest!TestUtilities::ParamCheckIndex<int32>
StackTest!CollectionTestUtils::TestUtilities_SimpleList<int>::RangeCheckIndex
StackTest!CollectionTestUtils::TestUtilities_SimpleList<int>::get_Item

The actual problem is in the test code!   The only useful information from the original log was basically the test that I should run.  Thank goodness it’s a 100% repro.

Comments

  • Anonymous
    April 05, 2010
    Rico, Why do you think that following pattern is so common catch (Exception e) { throw e; }

  • Anonymous
    April 05, 2010
    The comment has been removed

  • Anonymous
    April 05, 2010
    The comment has been removed

  • Anonymous
    April 06, 2010
    .NET 4 introduces a "generic error handler" (i.e., AggregateException) for code that uses the new Task class. I posted my concern over post-mortem debugging of TPL code on the PFX forums: http://social.msdn.microsoft.com/Forums/en-US/parallelextensions/thread/55c17b04-ca51-4f38-b02d-4a83819c1261 Stephen Toub posted a good workaround (on that thread), which should be read by any developer who wants to use Winqual with multi-threaded .NET 4 code.

  • Anonymous
    April 07, 2010
    I wish C# would default to re throwing exceptions when "throw ex" is used inside catch blocks. The problem currently is simply because the behavior is counter intuitive from a usage perspective and unintuitive from a effect perspective. We humans are used to combine verbs with objects (especially true in programming), so it's more intuitive to say "throw ex" than "throw;", meanwhile the desired "re throw" operation is not syntactically visible in the code when using "throw;" from a linguistic perspective. Maybe some day this problem will be solved at the roots and not just through education and warning signs.

  • Anonymous
    April 08, 2010
    Pop, I think that's a great Idea. Maybe we could go further than that. Lets make the default behaviour easier to debug, even in release builds and only by jumping through some extra hoops it will throw the debug information away: Suppose exception is.. try { //...thrown here } catch {} in a release build, and then 5 minutes later the app crashes. Now in my thoughts the default behaviour, unless explicitly disabled on try{} block basis (say for perf,security reason) should be to log the stack trace on every catched exception. Maybe there could be some kind of optimization here that if same block is throw&catching exactly same thing so much that it fills whatever buffer is logging the stack traces, it will on fly optimize this throw catch to happen quicker and take less log space. No idea on how complicated that would be to implement though so maybe it's not worth it.

  • Anonymous
    April 08, 2010
    zzz, It's easy enough to write your own code to do what you want. That's also pretty much the way that I do it - all exceptions get logged so that there are always breadcrumbs left behind for analysis. Pop, I treat the use of "throw ex;" as a bug because it throws away the original stack trace. There are a few use cases where this is the correct behavior but these are rare. It's too late to change the default - too many apps would break.

  • Anonymous
    April 08, 2010
    The comment has been removed

  • Anonymous
    April 09, 2010
    Miral, I also work on industrial control software. Writing fault tolerant and reliable software is very important to us.