Swallowing exceptions is hiding bugs

The whole idea of using exceptions is to catch and handle unusual situations that could crash your program or corrupt state such that the results your program produces are incorrect. In modern programming languages, exceptions have been perverted and are used in place of normal everyday error handling. But that’s a rant for another time.

One of the basic tenants of exception handling is catch only what you know how to handle. For example, in .NET the File.Open method can throw any one of several different exceptions.  When writing code that opens a file, you have to handle those exceptions. So, for example, you’d write:

FileStream f;
try
{
    f = File.Open("filename", FileMode.Read);
}
catch (FileNotFoundException)
{
    // code to handle file not found
}
catch (DirectoryNotFoundException)
{
    // code to handle directory not found
}
// other catch statements for other exceptions

You do not want to write this:

try
{
    f = File.Open("filename", FileMode.Read);
}
catch (Exception ex)
{
    // handle all exceptions
}

Why? Because there may be exceptions there that you don’t know how to handle. Sure, you can check the type of exception and re-throw those you don’t know how to handle, but then you’re defeating much of the purpose of exception handling and you’re also losing information.

Failing to handle exceptions is bad, but it’s not the worst thing. The worst thing you can do is catch an exception you don’t know how to handle, and fail to pass it on. Doing that masks bugs. For example, imagine your program does some calculations using a shared variable:

public int NumberOfItems;

private double GetAverage()
{
    double total = GetTotal(); // gets the total of something
    double avg = total / NumberOfItems;
    return avg;
}

If NumberOfItems is zero, the code above would throw DivideByZeroException. That’s right and proper, since the result of dividing by zero is undefined, and you don’t want your program producing the wrong answer. That could be disastrous. It’s better that the exception escape and potentially crash your program, the idea being that no answer is better than an incorrect answer.

Now, suppose somebody comes along much later and decides to “fix” that crashing program. He writes:

// For some reason this code sometimes crashes the program.
// This will fix it.
try
{
    CallMethodThatCallsGetAverage();
}
catch
{
    // Squash that pesky exception
}

That very effectively prevents the GetAverage method from crashing the program. It also very effectively prevents any other catchable exception from crashing the program. It “fixes” the symptom (the program no longer crashes), but it doesn’t fix the problem. The programmer who writes that should be … “counseled.”

And don’t think I’m making this up. I’ve seen such code.

It’s one thing for a programmer to make a “fix” like that. It’s another thing entirely to create a Framework component that works that way. A case in point is the .NET System.Timers.Timer component. The documentation for the Elapsed event says:

The Timer component catches and suppresses all exceptions thrown by event handlers for the Elapsed event. This behavior is subject to change in future releases of the .NET Framework.

This is terrible! Imagine that your Elapsed event handler encounters corrupt data and wants to throw an exception so that the program will crash. It can’t! Try this:

static void MyTimer_Elapsed(object sender, System.Timers.ElapsedEventArgs e)
{
    Console.WriteLine("Badness! Throwing exception");
    throw new ApplicationException("something bad happened");
}

The exception will never escape to the main program. The timer handler in .NET does essentially this:

try
{
    MyTimer_Elapsed(sender, args);
}
catch
{
    // Why would anybody want to know that something bad happened?
}

System.Timers.Timer is a bug hider, and for that reason shouldn’t be used.

Unfortunately, that’s not the only component in the Framework that squashes exceptions. It seems like any event handler that executes on a non-UI thread will squash exceptions. I don’t know if that’s globally true, but it sure seems that way. I always check the documentation these days, just to be sure.

 

Comments are closed.