Exception contracts (or lack thereof)

Visual Studio’s Code Analysis for Managed Code (formerly known as FxCop) has warned for approximately a decade against using generic catch (Exception) blocks. Many strongly-worded articles and blogs have been written to explain the reasoning. Perhaps even more digital ink has been spilled defending generic exception handlers — just witness the controversy on the Code Analysis blog.

Is there “one right answer” here? Does reality ever cleanly match up with ideals? It seems inevitable that the average developer will encounter a situation where there appears to be no better option than a catch-all handler. I’m no stranger to this, having personally written “catch (Exception)” dozens of times. But all of this is missing the point — it’s not about what to catch or rethrow, but really about abstractions and exception contracts (or lack thereof).

Consider the code you might see in a typical 100 KLOC application. In this example, we have a communication module which facilitates strongly typed messaging between a network boundary, leveraging a couple third-party libraries. Note that the responses are returned asynchronously, more or less, by posting to an outgoing queue.

void HandleRequest(RequestMessage request)
{
    NetworkRequest encodedRequest = this.Encode(request);
    NetworkResponse response = this.Send(encodedRequest);
    ResponseMessage decodedResponse = this.Decode(response);
    this.Post(decodedResponse);
}

After a while, it turns out the application is crashing due to underlying network failures. “No problem,” says the maintenance developer. “I can just handle the exception and post to the error queue.”

void HandleRequest(RequestMessage request)
{
    try
    {
        NetworkRequest encodedRequest = this.Encode(request);
        NetworkResponse response = this.Send(encodedRequest);
        ResponseMessage decodedResponse = this.Decode(response);
        this.Post(this.decodedResponse);
    }
    catch (NetworkException e)
    {
        this.PostError(e.ToString());
    }
}

The problem is solved for a little while, but at least two other unhandled exception types come up, occurring in rare network disconnection scenarios. (“These weren’t mentioned in the network library documentation!” complains the maintenance dev.) The code now looks like this:

void HandleRequest(RequestMessage request)
{
    try
    {
        NetworkRequest encodedRequest = this.Encode(request);
        NetworkResponse response = this.Send(encodedRequest);
        ResponseMessage decodedResponse = this.Decode(response);
        this.Post(this.decodedResponse);
    }
    catch (NetworkException e)
    {
        this.PostError(e.ToString());
    }
    catch (InvalidOperationException e)
    {
        this.PostError(e.ToString());
    }
    catch (TimeoutException e)
    {
        this.PostError(e.ToString());
    }
}

Alas, the story doesn’t end here. The third-party library used for encoding and decoding messages also comes under fire for throwing exceptions of many shapes and sizes, with several of them undocumented. After much trial and error, the code now looks like this (catch bodies elided for brevity):

void HandleRequest(RequestMessage request)
{
    try
    {
        NetworkRequest encodedRequest = this.Encode(request);
        NetworkResponse response = this.Send(encodedRequest);
        ResponseMessage decodedResponse = this.Decode(response);
        this.Post(this.decodedResponse);
    }
    catch (NetworkException e)
    // ...
    catch (InvalidOperationException e)
    // ...
    catch (TimeoutException e)
    // ...
    catch (DecoderException e)
    // ...
    catch (EncoderException e)
    // ...
    catch (InvalidDataException e)
    // ...
    catch (FormatException e)
    // ...
}

Most developers when faced with this ugly reality take the next logical step:

void HandleRequest(RequestMessage request)
{
    try
    {
        NetworkRequest encodedRequest = this.Encode(request);
        NetworkResponse response = this.Send(encodedRequest);
        ResponseMessage decodedResponse = this.Decode(response);
        this.Post(this.decodedResponse);
    }
    catch (Exception e)
    {
        // Yay, no more maintenance!
        this.Post(this.decodedResponse);
    }
}

Lines of code have decreased. Short term maintenance costs have decreased. This outcome certainly seems like a win in many cases. Unfortunately, it papers over the real issues at hand.

Bad documentation? Sure, that’s not good, but it’s more than that. Weak exception contracts? Yes, this is problematic but not the biggest issue. Too many distinct types of exceptions to handle? A pain for sure. But the foremost problem is tight coupling to external libraries. Solving that problem can limit the damage of bad exception hygiene to a single place in your application, providing an abstraction that the rest of your code can reliably depend on. Via this abstraction, you have the chance to fix a faulty exception contract in terms that make sense in your business domain.

Let’s face it — the types of library dependencies shown here are often spread throughout many places in the codebase. You may even see well-meaning attempts at recognition and remediation of this form:

void DoSafeLibraryCall(Action action, Action<Exception> onError)
{
    try
    {
        action();
    }
    catch (LibraryException1 e)
    {
        onError(e);
    }
    // ...
}

This is improvement of a sort (reduces some repetitive exception handling logic), but doesn’t complete the abstraction.

Consider this instead:

class MessageChannel
{
    public MessageChannel(IRawChannel channel, IEncoder encoder) { /* ... */ }
    public event EventHandler<ResponseEventArgs> ResponseReceived;
    public event EventHandler<ErrorEventArgs> Error;
    public void Send(RequestMessage message) { /* ... */ }
}

interface IRawChannel
{
    // Defined to throw *only* MyApp.ChannelException on recoverable error
    RawMessage Send(RawMessage payload);
}

interface IEncoder
{
    // Both defined to throw *only* MyApp.EncoderException on recoverable error
    TMessage Decode<TMessage>(RawMessage message);
    RawMessage Encode<TMessage>(TMessage message);
}

Now you have code that you can control covering each concern of the messaging protocol. The MessageChannel can be passed around freely and depended upon by any code within your system. All of the third-party library code can now reside in concrete classes implementing IRawChannel and IEncoder; this also becomes the single place where all of the direct error handling would occur, wrapping or suppressing exceptions as needed. Testability is the free bonus that comes with this approach; to verify the MessageChannel itself (e.g. to ensure it properly handles errors and raises the Error event), you simply need to provide stub or simulator implementations of IEncoder et al.

Bad exception contracts? No problem! When you keep external dependencies at a safe distance, behind a well-defined abstraction that you control, it’s all good.