Catching and Throwing exceptions; Why is this considered an anti-pattern

Question

public void DeployCourse(Course course, Client client)
{
    if (course == null) throw new ArgumentNullException("Course cannot be null");       
    if (client == null) throw new ArgumentNullException("Client cannot be null");

    try
    {
        _ftp.Transfer(client.Server.IPAddress, course.PackageUrl, course.CourseName);
    }
    catch (Exception e)
    {
        var newException = new Exception(String.Format(
            "Error deploying Course: {0} to Client: {1}. See inner exception for more details", 
            course.CourseName, client.Name), e);
        throw newException;
    }
}

I've never really got my head around what constitutes "good" exception handling. A quick google search shows that, almost unanimously, people agree that catching and rethrowing exceptions deep in the call stack is bad. Above, I have an example of some code I'm writing. This is pretty low in a typical call stack. I did this because, without this code, it is a bit difficult to find exactly what course failed to deploy. My question is, if I did something similar in many (if not all) methods, to add more context to an exception, why would this be considered anti-pattern?

Thanks!

Answer 1

This question is somewhat vague, so let's try to crisp it up a bit.

Is the pattern of catching an exception, wrapping it up in a different exception, and throwing the new exception to the caller, a good pattern?

Yes, it's pretty good. Like any pattern it has pros and cons.

What are the pros?

It works best when the exception thrown is logically related to the operation which the caller expected the method call to perform. If "operation Foo can throw a FooFailedException" is part of the documented contract then developers know that they need to be catching FooFailedException and -- here's the key point -- only FooFailedException.

If you apply this consistently then you can change the implementation details of your method without worrying that you are breaking a caller.

For instance, if you require a caller to catch, say, an exception associated with failure of an FTP site, and then you change your implementation to also support some other protocol that throws a different exception, then the caller has to change as well to catch the new exception (or they have to catch everything). With this pattern the caller can catch just the one exception no matter what the implementation details are.

This also allows the caller to get diagnostic information in terms of the operation that they tried to do, not in terms of the implementation detail that failed.

What are some of the cons?

1. If the change is made late, then changing which exception is thrown could be a breaking change.

2. Callers might want to be catching the more specific underlying exception.

3. If the underlying exception is thrown because your code has a bug and is calling something wrong itself then you are hiding your bug and passing the consequences of it on to the caller.

4. Some exceptions are like "the network is down right now, try again later", and some exceptions are like "everything is terrible and you really should be shutting this process down". By catching everything and wrapping it, you make it harder to know whether the exception is recoverable or not.

5. We have a list of certificates that have been compromised by hackers; we keep the list on the internet. We wish to not use a certificate if it is on the list of revoked certs. Quick, find the defect:

   bool revoked = false;
   try
   {
       CheckTheRevocationList(out revoked);
   }
   catch(Exception ex)
   {
       new CryptoException("revocation list could not be checked", ex);
   }
   if (!revoked) UseTheCertificate();
   

See the defect? That's a security hole right there. Finding that needle in a haystack of 999 correct usages of the pattern is surprisingly difficult for humans. (This is not theoretical; I found a version of this defect in real live code by writing a static analyzer that looks for it.)

Where can I read more about exception handling patterns and practices?

My articles are here:

http://ericlippert.com/category/exception-handling/

The one you should probably start with is frequently referenced on SO:

http://ericlippert.com/2008/09/10/vexing-exceptions/