Exception Handling Basics

The other day, I was reviewing some code, and I saw a series of methods conforming to the following (anti) ‘pattern’

public class CustomerProcessor
{
    public void ProcessCustomer(Customer customer)
    {
        try
        {
            if (customer.IsActive)
                ProcessActiveCustomer(customer);
        }
        catch (Exception ex)
        {
            throw ex;
        }
    }

    private void ProcessActiveCustomer(Customer customer)
    {
        try
        {
            CheckCustomerName(customer);
            WriteCustomerToFile(customer);
        }
        catch (Exception ex)
        {
            throw ex;
        }
    }

    public void CheckCustomerName(Customer customer)
    {
        try
        {
            if (customer.Name == null)
                customer.Name = string.Empty;
        }
        catch (Exception ex)
        {
            throw ex;
        }
    }

    private void WriteCustomerToFile(Customer customer)
    {
        try
        {
            using (StreamWriter writer = new StreamWriter(@"C:\temp\customer.txt"))
            {
                writer.WriteLine(customer.Name);
            }
        }
        catch (Exception ex)
        {
            throw ex;
        }
    }
}

Every method consisted of a try with the actual code of interest inside of it and then a caught general exception that was then thrown. As I looked more through the code base, it became apparent to me that this was some kind of ‘standard’ (and thus perhaps exhibit A of how we get standards wrong). Every method in the project did this.

If you’re reading and you don’t know why this is facepalm, please read on. If you’re well-versed in C# exceptions, this will probably be review for you.

Preserve the Stack Trace

First things (problems) first. When you throw exceptions in C# by using the keyword “throw” with some exception type, you rip a hole in the fabric of your application’s space-time–essentially declaring that if no code that’s called knows how to handle the singularity you’re belching out, the application will crash. I use hyperbolic metaphor to prove a point. Throwing an exception is an action that jolts you out of the normal operation of your program by using a glorified “GOTO,” except that you don’t actually know where it’s going because that’s up to the code that called you.

When you do this, the .NET framework is helpful enough to package up a bunch of runtime information for troubleshooting purposes, including something called the “stack trace.” If you’ve ever seen a .NET (or Java) site really bomb out, you’ve probably seen one of these–it’s a bunch of code with line numbers that basically tells you, “A called B, which called C, which called D … which called Y, which called Z, which threw up and crashed your program.” When you throw an exception in C# the framework saves the stack trace that got you to the method in question. This is true whether the exception happens in your code or deep, deep within some piece of code that you rely on.

So, in the code above, let’s consider what happens when the code is executed on a machine with no C:\temp folder. The StreamWriter constructor is going to throw an exception indicating that the path in question is not found. When it does, you will have a nice exception that tells you ProcessCustomer called ProcessActiveCustomer, which called WriteCustomerToFile, which called new StreamWriter(), which threw an exception because you gave it an invalid path. Pretty neat, huh? You just have to drill into the exception object in the debugger to see all of this (or have your application configured to display this information in a log file, on a web page, etc.).

But what happens next is kind of a bummer. Instead of letting this exception percolate up somewhere, we trap it right there in the method in our catch block. At that point, we throw an exception. Now remember, when you throw an exception object, the stack trace is recorded at the point that you throw, and any previous stack trace is blown away. Instead of it being obvious that the exception originated in the StreamWriter constructor, it appears to have originated in WriteCustomerToFile. But wait, it gets worse. From there, the exception is trapped in ProcessActiveCustomer and then again in ProcessCustomer. Since every method in the code base has this boilerplate, every exception generated will percolate back up to main and appear to have been generated there.

To put this in perspective, you will never be able to see or record the stack trace for the point at which the exception was thrown. Now, in development that’s not the end of the world since you can set the debugger to break where thrown instead of handled, but for production logging, this is awful. You’ll never have the foggiest idea where anything is coming from.

How to fix this? It’s as simple as getting rid of the “throw ex;” in favor of just “throw;” This preserves the stack trace while passing the exception on to the next handler. Another alternative, should you wish to add more information when you throw, would be to do “throw new Exception(ex)” where you pass the exception you’ve caught to a new one that you’re creating. The caught exception will be preserved, intact, and can be accessed in debugging via the “InnerException” property of the one you’re now throwing.

public class CustomerProcessor
{
    public void ProcessCustomer(Customer customer)
    {
        try
        {
            if (customer.IsActive)
                ProcessActiveCustomer(customer);
        }
        catch (Exception ex)
        {
            throw;
        }
    }

    private void ProcessActiveCustomer(Customer customer)
    {
        try
        {
            CheckCustomerName(customer);
            WriteCustomerToFile(customer);
        }
        catch (Exception ex)
        {
            throw;
        }
    }

    public void CheckCustomerName(Customer customer)
    {
        try
        {
            if (customer.Name == null)
                customer.Name = string.Empty;
        }
        catch (Exception ex)
        {
            throw;
        }
    }

    private void WriteCustomerToFile(Customer customer)
    {
        try
        {
            using (StreamWriter writer = new StreamWriter(@"C:\temp\customer.txt"))
            {
                writer.WriteLine(customer.Name);
            }
        }
        catch (Exception ex)
        {
            throw;
        }
    }
}

(It would actually be better here to remove the Exception ex altogether in favor of just “catch {” but I’m leaving it in for illustration purposes)

Minimize Exception-Aware Code

Now that the stack trace is going to be preserved, the pattern here isn’t actively hurting anything in terms of program flow or output. But that doesn’t mean we’re done cleaning up. There’s still a lot of code here that doesn’t need to be.

In this example, consider that there are only two methods that can generate exceptions: ProcessCustomer (if passed a null reference) and WriteCustomerToFile (various things that can go wrong with file I/O). And yet, we have exception handling in every method, even methods that are literally incapable of generating them on their own. Exception throwing and handling is extremely disruptive and it makes your code very hard to reason about. This is because exceptions, as mentioned earlier, are like GOTO statements that whip the context of your program from wherever the exception is generated to whatever place ultimately handles exceptions. Oh, and the boilerplate for handling them makes methods hard to read.

The approach shown above is a kind of needlessly defensive approach that makes the code incredibly dense and confusing. Rather than a strafing, shock-and-awe show of force for dealing with exceptions, the best approach is to reason carefully about where they might be generated and how one might handle them. Consider the following rewrite:

public class CustomerProcessor
{
    public void ProcessCustomer(Customer customer)
    {
        if(customer == null)
            Console.WriteLine("You can't give me a null customer!");
        try
        {
            ProcessActiveCustomer(customer);
        }
        catch (SomethingWentWrongWritingCustomerFileException)
        {
            Console.WriteLine("There was a problem writing the customer to disk.");
        }
    }

    private void ProcessActiveCustomer(Customer customer)
    {
        CheckCustomerName(customer);
        WriteCustomerToFile(customer);
    }

    public void CheckCustomerName(Customer customer)
    {
        if (customer.Name == null)
            customer.Name = string.Empty;
    }

    private void WriteCustomerToFile(Customer customer)
    {
        try
        {
            using (var writer = new StreamWriter(@"C:\temp\customer.txt"))
            {
                writer.WriteLine(customer.Name);
            }
        }
        catch (Exception ex)
        {
            throw new SomethingWentWrongWritingCustomerFileException("Ruh-roh", ex);
        }
    }
}

Notice that we only think about exceptions at the ‘endpoints’ of the little application. At the entry point, we guard against a null argument instead of handling it with an exception. As a rule of thumb, it’s better to handle validation via querying objects than by trying things and catching exceptions, both from a performance and from a readability standpoint. The other point of external interaction where we think about exceptions is where we’re calling out to the filesystem. For this example, I handle any exception generated by stuffing it into a custom exception type and throwing that back to my caller. This is a practice that I’ve adopted so that I know at a glance when debugging if it’s an exception I’ve previously reasoned about and am trapping or if some new problem is leaking through that I didn’t anticipate. YMMV on this approach, but the thing to take away is that I deal with exceptions as soon as they come to me from something beyond my control, and then not again until I’m somewhere in the program that I want to report things to the user. (In an actual application, I would handle things more granularly than simply catching Exception, opting instead to go as fine-grained as I needed to in order to provide meaningful reporting on the problem)

Here it doesn’t seem to make a ton of difference, but in a large application it will–believe me. You’ll be much happier if your exception handling logic is relegated to the places in the app where you provide feedback to the user and where you call external stuff. In the guts of your program, this logic isn’t necessary if you simply take care to write code that doesn’t contain mistakes like null dereferences.

What about things like out of memory exceptions? Don’t you want to trap those when they happen? Nope. Those are catastrophic exceptions beyond your control, and all of the logging and granular worrying about exceptions in the world isn’t going to un-ring that bell. When these happen, you don’t want your process to limp along unpredictably in some weird state–you want it to die.

On the Lookout for Code Smells

One other meta-consideration worth mentioning here is that if you find it painful to code because you’re putting the same few lines of code in every class or every method, stop and smell what your code is trying to tell you. Having the same thing over and over is very much not DRY and not advised. You can spray deodorant on it with something like a code snippet, but I’d liken this to addressing a body odor problem by spraying yourself with cologne and then putting on a full body sweatsuit–code snippets for redundant code make things worse while hiding the symptoms.

If you really feel that you must have exception handling in every method, there are IL Weaving tools such as PostSharp that free you from the boilerplate while letting you retain the functionality and granularity you want. As a general rule of thumb, if you’re cranking out a lot of code and thinking, “there’s got to be a better way to do this,” stop and do some googling because there almost certainly is.