Dec 02

While I was reading the Essential Windows Communication Foundation book (from Addision Wesley), I found the following code (on a transaction required for a WCF service context):

[OperationBehavior(TransactionScopeRequired = true,
   TransactionAutoComplete = true)]
public void Transfer( String from, String to, double amount){
  try {
    Withdraw(from, amount);
    Deposit( to, amount);
  catch(Exception ex){
    throw ex;

Lets not even talk about why using the Exception type in a try/catch is bad (though I accept its usage in a book, but only after seeing a justification of why you should not use it in production code)…instead, lets concentrate on what is happening inside the catch block…see anything wrong there? I hope so! You should never re-throw an exception like that. In my opinion, you’ve got 2 options here:

1. You’re “important” in the stack chain and then you’ll want to wrap that exception with your own specific  exception type (for isntance, NHibernate does something like this and wraps ADO.NET exceptions with its own exception type). Here’s what I mean:

catch(Exception ex ) { //btw, don’t catch the Exception type
  //do something here…probably log it
   throw new MyException(ex);

2. You’re not significant,so you can just rethrow that exception after logging:

catch(Exception ex ) { //btw,don’t catch the Exception type
  //do something here…probably log it
   throw; //see? no ex here!

Ok, so why is this important? Because when you re-throw the exception by using technique shown in point 2, you will be preserving the stack trace. When you do it like in the original code, the stack trace that the client will see ends up in your catch block. Probably not what you’ll want in most scenarios!

Do notice that technique 1 will also preserve the stack chain since you’re passing the exception as the inner exception of the new exception you’re throwing. Both of these options are really better than the one shown on the initial code.

I’m only writing this here because I was under the impression that this info should be well know by now, but it seems like I’m wrong…Do keep in mind that it’s not my intent to trash on this book (I’m still on page 211, but I’m liking it so far), but I was a little but surprised by seeing this code there (as always, I’ll put a review after ending it).

4 comments so far

  1. Israel Aece
    3:36 pm - 12-2-2008

    Hello Luis,

    Another hint is that when TransactionAutoComplete property is set to True, isn”t necessary to catch exceptions (unless for logger), because this configuration always will abort transaction if an exception is throwed.

    Full stack trace only will appear for client if IncludeExceptionDetailsInFaults property have been set to true. If this property is set to false (default), client gets a plain FaultException.


  2. Giovanni Bassi
    4:19 am - 12-6-2008

    Luis, I see this type of code all the time. And worse. It seems somebody told a lot of developers that catching exceptions is good, so they place a TCF block from the beginning through the ending of the method. And they rethrow the original exception in the catch block without doing any work, no log, no anything, besides fking up the stack trace. Every time I see this when a bug happens in production I want to shoot myself. Actually, I want to shoot the developer who coded it like that. it seems people don”t learn the concepts anymore, they are coding right from the “.net for dummies” books…

  3. Jasaz
    12:53 pm - 1-5-2010

    How do i know that the Exception is from the called method and not from the calling method because in the first case i need to do “throw;” whereas in the second case its just “throw ex;” ?

  4. luisabreu
    1:01 pm - 1-5-2010

    I wouldn”t recommend doing that because you really have no guarantees about the method that is really throwing the exception (as you can see from the previous code). Can you do your thing by checking the type of the exception?