Click here to Skip to main content
15,887,585 members
Articles / General Programming / Exceptions

Logging is the new Exception Swallowing

Rate me:
Please Sign up or sign in to vote.
4.00/5 (2 votes)
4 May 2011CPOL1 min read 9.1K   1   1
Logging is the new Exception Swallowing

For a long time now, I've been stamping down hard on empty catch blocks in code, for obvious reasons. When I can dictate coding standards, that's pretty much top the list.

  1. Every 'catch' block must (at a minimum) log or throw.

    I now realize I made a mistake with this rule...it should be:

  2. Every 'catch' block must (at a minimum) throw or log.

    It's a subtle (the wording – not the formatting) difference in the way we need to think about this rule but it's one I think we don't think about enough.

For example, this is a common pattern of code that I'm seeing during Code Review sessions:

C#
public bool PerformCriticalTask()
{
    try
    {
        CriticalFunctionalityTaskA();
        CriticalFunctionalityTaskB();
        CriticalFunctionalityTaskC();
        return true;
    }
    catch(Exception ex)
    {
        Logger.Log(ex);
    }
    return false;
}
  
public CriticalFunctionalityTaskA()
{
    try
    {
        //Do Important Stuff Here
    }
    catch(Exception ex)
    {
        Logger.Log(ex);
    }
}
  
public CriticalFunctionalityTaskB()
{
    try
    {
        //Do More Important Stuff Here
    }
    catch(Exception ex)
    {
        Logger.Log(ex);
    }
}
  
public CriticalFunctionalityTaskC()
{
    try
    {
        //Do The Most Important Stuff
    }
    catch(Exception ex)
    {
        Logger.Log(ex);
    }
}

It's pretty clear that this Catch/Log pattern has become a developer's default exception handling boiler plate code. It's adding no value and actually making the troubleshooting processes more complicated and time consuming. 

My response to this is simple… remove all exception handling!

In the vast majority of cases, if an exception is thrown, the application is already broken and letting it limp along is an act of cruelty. The application should handle these critical failures as exceptional and handle all of these events the same way in an application scoped way (Custom Error Page for Web Applications, etc.). By all means, log the exception at the application scope level, it will have actual diagnostic value there (complete stack trace, etc.).

Of course, there are exceptions to this policy such as when you can legitimately recover from the fault. But these cases are few and far between.

License

This article, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)


Written By
Software Developer (Senior) Freestyle Interactive Ltd
United Kingdom United Kingdom
I'm a lead developer for Freestyle Interactive Ltd where we create many wonderful websites built on Microsofts ASP.Net and Ektron CMS.

I've been developing .Net applications (both Windows and Web) since 2002.

Comments and Discussions

 
RantSwallowing, logging, rethrowing, and ignoring Pin
Seth Morris10-May-11 10:36
Seth Morris10-May-11 10:36 
(This response is also posted to my blog at http://s4sd.wordpress.com/2011/05/10/swallows-logs-throws-and-ignores-exceptions/[^]

I am always glad to see someone pointing out that exception handling is horribly handled!

First, a disclaimer. I'm a bit old school. I think exceptions are generally bad things that have led to worse code and worse error handling. I recognize that I am in the minority and that the ship has sailed, though.
Second, another disclaimer: I have a love of bullet lists that may seem in need of treatment. I apologize.
Third, yet another disclaimer: I have no idea why CodeProject formatting seems so horribly inconsistent when I'm editing a post. I hope this comes out readable on your end.


You're raising a point which goes to the heart of the problem. We write code that swallows (or logs unimportant exceptions because we get yelled at for swallowing) because our libraries use exceptions poorly and because our development culture uses exceptions even more poorly. Exceptions, no matter what academic purists and book authors intend, are used in to mean any of several conditions in the real world, and they're used with the same syntax despite the semantics.

In the wild (including in the .NET library), an exception could mean any combination of:
- The function couldn't perform its contract (the classic definition of an exception).
- The function is informing you of a side effect ("Hi. I've consumed all your disk space. Have a nice day."). This may not even be an error.
- Some internal object threw an exception. I hope the library correctly listed every exception that type it can throw AND the functions it calls can throw. And that they correctly listed this.
- There is an informational message you should log, but you can continue.
- We're done with a loop. The exception indicates the end of some condition (reading a file, network access, etc.)

And we may want to respond wildly differently:
- The system state is inconsistent, exit the app ASAP. This is often the correct answer in client apps.
- Try a different function.
- Move on. For example, some type conversion failed and the original value is as good as it gets.

We get in this situation because we use exceptions poorly, but the response can't be "Use exceptions better." We have to deal with libraries that we can't change, we have to deal with legacy code that we shouldn't change, our team members have--and will continue to hire--people of varying skill, and writing exception handling isn't the reason we're writing software. We're writing software to meet some business need (or some general need, if you aren't comfortable with the word "business"). Exceptions take up way too much of our time (as does anything that isn't about making our users and customers happier and more successful).

For a fabulous example of the current situation, consider the joys of int.Parse() and the need for int.TryParse(), which has a horrible legacy syntax and obfuscates the logic of the code.

So we're left with a mix of reasonable responses to exceptions:
- Catch the exceptions we can do something useful with, even if that is crashing, and let some unknown error handler above us catch the others. This is the most common suggested solution.
- Catch the exceptions we have some alternate response to and swallow others with some default action.
- Swallow everything because failing what we're doing is not critical. Some comment to this effect makes code reviewers happy (and is generally good for other reasons!), but the same boilerplate comment in many places actually decreases readability. Differentiate Decision from Idiom.
- Log the exception so we don't get dinged in a code review for ignoring exceptions. Off-site code reviews by outside consultants love to make long lists of correctly swallowed exceptions and present them managers who are less-technical or who don't have a familiarity with the code or platform and can't identify which are well-considered and which are just lazy. [In software, about half the time "Lazy Programmer" is used as a code phrase for "pressured to meet deadlines," "not given sufficient training," or "no longer working here and easy to blame."]

This post calls out a good response that is often left implicit, because it also gets dinged in code reviews and it is a little scary:
- Let any unknown upstream called catch exceptions, if they can, and hope they can handle it

Here's what I'd like to add: It's a good response when a few conditions are met. In other conditions, other responses make sense (of course). Code should express its intent and assumptions to other programmers who will have to maintain it for years to come; it needs to be readable as to which conditions the programmer assumes are present.
- All exceptions are handled here. When the libraries change, this code needs to be re-evaluated for exception handling.
- Known exceptions are handled, but bailing out to upstream handlers is acceptable. Finally blocks are used correctly, for example.
- Exceptions are informational, but shouldn't stop execution.
- Downstream code throws exceptions when this code doesn't care and downstream functions use the "pass exceptions along and hope they're handled" philosophy. We just want them to go away.

Also, note that there is a significant different between handling exceptions around one statement, such as int.Parse(), and a statement block which may throw exceptions from any of several calls.

I augment coding standards with something less draconian: a "stop" list (http://s4sd.wordpress.com/2008/09/17/development-red-lights/[^]). This is a list of statement, idioms, and techniques which require stopping, stepping back, and explaining to another (senior) developer. If a second set of eyes agrees that it's reasonable in this context, you go forward (and say who agreed in the checkin comment!). Swallowing exceptions is a good Stop item.

Make a considered and deliberate choice in how to handle exceptions. Then make it clear. Code that swallows (or logs) any exception indiscriminately won't go away. It's impossible to handle exceptions better than your downstream calls throw them and real, reasonable conditions arise where exceptions in downstream code just don't matter. As an obvious example, an exception from an error logging call (say, network or disk unavailability is preventing logging) shouldn't be logged.

Just for fun, here's a cute way to document swallowing errors. "Cute trick" is usually a code phrase for "bad for maintenance," but maybe this one is reasonable.

using System;
namespace ExceptionSwallowing
{
    class Program
    {
        delegate void statementblock();
        static void SWALLOW_EXCEPTIONS(statementblock fn) {try{fn();}catch(Exception){;}}

        static void Main(string[] args)
        {
            SWALLOW_EXCEPTIONS(()=>
            {
                Console.WriteLine("Example of executing a Statement Block");
            });
            Console.WriteLine();

            try
            {
                SWALLOW_EXCEPTIONS(() =>
                {
                    Console.WriteLine("Throwing an exception that should be swallowed");
                    throw (new Exception("ExceptionToBeIgnored"));
                });
                Console.WriteLine("  Success: the exception was correctly ignored");
            }
            catch (Exception e)
            {
                Console.WriteLine(  "Error: the exception was caught but should have been swallowed: " + e.Message);
            }
        
            Console.WriteLine();
            Console.WriteLine("Press any key to exit");
            Console.ReadKey();
        }
    }
}

General General    News News    Suggestion Suggestion    Question Question    Bug Bug    Answer Answer    Joke Joke    Praise Praise    Rant Rant    Admin Admin   

Use Ctrl+Left/Right to switch messages, Ctrl+Up/Down to switch threads, Ctrl+Shift+Left/Right to switch pages.