Just Say No! – To Empty Exception Handlers

Reading Time: 3 minutes

We’ve been having a bit of trouble with a small piece of software on a customer site. It’s just not working and there are no indications of why. So I cracked open the source and found a light dusting of these…

try
{
    SqlConnection = new SqlConnection(connectionString);
    //the rest of the Sql code
}
catch (SqlException)
{
}

Oh look, it’s an empty catch. An empty catch is about as justifiable as a goto – I’m not saying that you should never use one, just that you need to have a good hard think about what you’re actually doing and consider carefully whether or not you’ve got the structure of your code correct.

The case against is easy to see above; although it may be normal for one type of SqlException to be thrown and ignored (perhaps as the result of a deadlock), you can’t guarantee that every SqlException is thrown for that reason. In the deadlock example your code should more follow this pattern.

try
{
    SqlConnection = new SqlConnection(connectionString);
    //the rest of the Sql code
}
catch (SqlException ex)
{
        if (1205 == ex.Number) //deadlock (see http://technet.microsoft.com/en-us/library/cc645603.aspx)
        {
            //we ignore deadlocks 
        }
        else
            throw;
}

This tests the SqlException to see if it’s a deadlock and ignores it – if it’s not a deadlock then it re-throws it.

I would argue however that this doesn’t go far enough. Exceptions should not be used for flow control which means that the very fact an exception has been thrown indicates that something has gone wrong. If you’re not going to handle that in code then the very least you should do is enable someone to find out that it happened.

There are many logging and debugging gizmos out there (such as log4net) and they usually contain a concept of levels of information. It would be a rare circumstance indeed where an empty catch was preferable to a minimal debug log, e.g.

try
{
    SqlConnection = new SqlConnection(connectionString);
    //the rest of the Sql code
}
catch (SqlException ex)
{
        if (1205 == ex.Number) //deadlock (see http://technet.microsoft.com/en-us/library/cc645603.aspx)
        {
            log4netInstance.Debug("SomeOperation caught Sql deadlock => {0}", ex.ToString());
        }
        else
            throw;
}

If this exception is normal and the app is expected to cope then you can turn the “Debug” level of logging off in configuration. If the app starts having problems then you can turn the “Debug” level back on and see all the exceptions that are being thrown. A subtle little thing like this can make a big difference.

On the one hand we have a quick phone call to one of the customer’s IT pros to explain that their recent security audit has resulted in them accidentally revoking the app’s authentication to the DB and that perhaps they’d like to reverse that.

On the other hand we have you, the developer, being sent to the customer’s site where you have to explain to some very senior people why your application has stopped working and what you intend to do about the poor quality, unreliable piece of software that you’ve supplied.

Never underestimate what can happen to your app on a customer site: Murphey’s Law is one of the fundamental underpinnings of computer science.

Share with