Common Mistakes When Working with Exceptions

Rethrowing and Destroying the Stacktrace

This is an evil one. Because it’s really easy to use it and the compiler does not complain about it (it should be easy to spot by the compiler). The code also looks correct if you don’t know what it really does.

What am I talking about?

try
{
    FutileAttemptToResist();
}
catch (BorgException err)
{
    _myDearLog.Error("I'm in da cube! Ohh no!", err);
    throw err;
}

Looks about right? The problem is the throw err; part. It will wipe the stack trace and create a new one from your catch block. You’ll never know where the exception originated from. Just use throw;.

Using Policies for Exception Handling

I’ve started to write several pieces about why you should not use policies (for instance Exception Handling Block in Enterprise library), but I never got really satisfied with it.

The problem with the block is that the examples are still not very good and really tricks you into bad practices.

Anyway, introducing policies can be good if you are following my guidelines in the post “Do NOT catch that exception!”. The problem though is that it’s easy to start adding try/catch/policy everywhere and hence producing unreadable code that is logging the exception multiple times again.

Just handle those exceptions that you can handle. No need for a policy.

Not Including Original Exception When Wrapping

Another common mistake is to not include the original exception when throwing another one. By failing to do so, you are also failing to give information about where the error really happened.

try
{
    GreaseTinMan();
}
catch (InvalidOperationException err)
{
    throw new TooScaredLion
   ("The Lion was not in the m00d", err); //<---- original exception is included, hooray!
}

Not Providing Context Information

This one is actually a golden rule at Microsoft. They really love to give us developers a challenge when something fails. They have built a superior framework with .NET and they need to put us developers back to earth sometimes. Hence the crappy exception descriptions.

This is what I’m talking about:

try
{
   socket.Connect("somethingawful.com", 80);
}
catch (SocketException err)
{
    throw new InvalidOperationException
    ("Socket failed", err);  //I LOVE InvalidOperationException. Read my previous post.
}

What’s wrong? The exception message is just stating the obvious. We already know that the connect fails. But we do not know what it tried to connect to. Always provide contextual information. NHibernate is the glorious king when it comes to developer friendly exceptions. Examine its source code and learn something.

What you really should do is something like this:

void IncreaseStatusForUser(int userId, int newStatus)
{
    try
    {
         var user  = _repository.Get(userId);
         if (user == null)
             throw new UpdateException(string.Format("Failed to find user #{0} 
		when trying to increase status to {1}", userId, newStatus));

         user.Status = newStatus;
         _repository.Save(user);
    }
    catch (DataSourceException err)
    {
        var errMsg = string.Format("Failed to find modify user #{0} 
		when trying to increase status to {1}", userId, newStatus);
        throw new UpdateException(errMsg, err);
    }

I always try to include as much information as possible in my exceptions. One could discuss if that sensitive information should not be included in exceptions. But I say YES! It’s really up to code that EXPOSES the exception (for instance, a logger or a message box) to hide sensitive information. For instance, only log exceptions in a log that the correct people have access to. Exceptions should be dealt with ASAP anyway.

I would never expose an exception to an end user. What use do they have of the exception information? You got a widely used application and need to get exception information? Encrypt it and upload it to your server (with permission from your user, of course).