tucaz.blog.now()

Last week we had a severe problem with exception handling in a project I’m working in. All the exceptions were being suppressed so we couldn’t know what was going on when some functionality didn’t work as we expected it to. This happened because a member of our team misinterpreted exception handling and put try/catch blocks in every method that he coded, but didn’t put any code in the catch block.

The  code spreaded everywhere looked like this:

try
{
     //Aplication logic goes here
}
catch (Exception ex)
{
     string error = ex.Message;
     //No re-throw or any exception handling code. Just this.
}

As soon as we discovered the problem (and who was causing it), Max (a co-worker) wrote this “guide” and we made sure that everybody involved in the project read it. This is pretty basic stuff, but lot’s of people still don’t understand it. I’ll reproduce it below “as is” hoping that it will help you or someone you know

What is the problem? Whenever an expection is trown, the code is trapping it in the catch block and it’s basically destroying it without proper handling and without escalete it in the stack (exception bubbling).

Why is it a problem? This results in one of the worst maintenance nightmares. As the exception is being destroyed before it can be captured, bugs will become almost impossible to track in an QA/Production environment where debug is not available. Even in the development environment, chances are you’re only going to be able to track the root cause stepping through the code line by line.

What to do? Unless you really have a need to do something specific with an exception, just let it escalete through the stack. Exceptions are usually handled at a higher level (like the user interface). Even when you need to do something with the exception, it will usally be necessary to re-throw it at the end of your code uless you’re coding the last layer (like a UI).

Here are some good practice examples:

Example 1:

public string GetUser(string userName)
{
    return new DAL().GetUser(userName);
}

In this example the GetUser represents a intemediate layer code. As we don’t need any special handling, any exceptions resulting from the DAL method call will be simply escalted to the caller through the exception bubbling.

Example 2:

public string GetUser(string userName)
{
    try
    {
        return new DAL().GetUser(userName);
    }
    catch (Exception ex)
    {
        LogHelper.LogException(ex);
        throw;
    }
}

In this case we do need to do something special about the exception. A good example would be a web service, where we should be loggin any errors, but re-throwing it to the caller so the application will be aware of it.

Example 3:

bool isValid = false;
try
{
    //Your application logic goes here
    isValid = true;
}
catch (DivideByZeroException)
{
    //Nothing needs to be done in case of an exception, this will kee the flag isValid as false
}

This example illustrates a situation where you really need to destroy the exception. The Try/Catch block is being used exclusively to set a flag that depends on the result of an operation that might throw an exception, which would mean the status is invalid. Please note three important things:

a) In this case it is fundamental to add a comment to the Catch block in order to let your intention clear.

b) As you’re not planning to do anything special with the exception, a “catch(DivideByZeroException)” syntax is enough, since you won’t need to assign the exception to a local variable (which would generate a compilation warning about an unused variable). e.g.: catch (DivideByZeroException ex).

c) In this type of scenario you’re looking for a specific exception, so make sure you’re catching only that specific type of exception and let anything else bubble up (meaning, do not use “catch { }”). Also, when catching a specific exception, there’s no need for adding extra catch for the generic exception, anything else will automatically bubble up:

catch (DivideByZeroException)
{
    //Nothing needs to be done in case of an exception, this will kee the flag isValid as false
}
catch //There’s no need for this block
{
    throw;
}

Issue: You re-throw the exception using the “throw Exception” syntax:

What is the problem? This resets the exception stack trace.

Why is it a problem? The stack trace is lost and higher levels will perceive the error as being generated by your code. The main issue is the fact that it won’t allow you to track the root cause of the exception.

What to do? The correct syntax to re-throw an exception is to simple state “throw” in your code, this way the stack trace is preserved.

try
{
    //Application logic goes here
}
catch (Exception ex)
{
    LogHelper.LogException(ex);
    throw;
}

What if you need to generate a custom exception? Same issue with the stack trace. In this case the Exception constructor provides a parameter for the inner exception that must be used:

try
{
    //Application logic goes here
}
catch (Exception ex)
{
    LogHelper.LogException(ex);
    throw new Exception("My custom message", ex);
}

Issue: Unnecessary use of a Catch block when all you need is to get some code running at the Finally block:

try
{
	//Application logic goes here
}
catch
{
	throw;
}
finally
{
	//Your finally code goes here
}

What is the problem? The catch block in this case is completely unecessary.

Why is it a problem? Not a big deal, but it represents unecessary lines of code that will make your code look dumb to your collegues.

What to do? Just use a Try/Finally block:

try
{
	//Your application logic goes here
}
finally
{
	//Your finally code goes here
}

ps: methods used in the examples are using an anemic model for simplicity purposes.