Strategies for Successful Development

Process and technology for small teams building real software

Archive for May, 2011

Swallows, logs, throws, and ignores (exceptions)

Posted by S4SD (Seth Morris) on 2011/05/10

This post is in response to Logging is the New Exception Swallowing by Martin Jarvis.

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.

Mr. Jarvis is 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 exceptions) 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 claim, 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 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 lazy1.

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 (https://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.

   1: using System;

   2:  

   3: namespace ExceptionSwallowing

   4: {

   5:     class Program

   6:     {

   7:         delegate void statementblock();

   8:         static void SWALLOW_EXCEPTIONS(statementblock fn) {try{fn();}catch(Exception){;}}

   9:  

  10:         static void Main(string[] args)

  11:         {

  12:             SWALLOW_EXCEPTIONS(()=>

  13:             {

  14:                 Console.WriteLine("Example of executing a Statement Block");

  15:             });

  16:             Console.WriteLine();

  17:  

  18:             try

  19:             {

  20:                 Console.WriteLine("Throwing an exception that should be swallowed");

  21:                 SWALLOW_EXCEPTIONS(() =>

  22:                 {

  23:                     Console.WriteLine("Throwing an exception that should be swallowed");

  24:                     throw (new Exception("ExceptionToBeIgnored"));

  25:                 });

  26:                 Console.WriteLine("  Success: the exception was correctly ignored");

  27:             }

  28:             catch (Exception e)

  29:             {

  30:                 Console.WriteLine(  "Error: the exception was caught but should have been swallowed: " + e.Message);

  31:             }

  32:         

  33:             Console.WriteLine();

  34:             Console.WriteLine("Press any key to exit");

  35:             Console.ReadKey();

  36:         }

  37:     }

  38: }



Listening to: Lou Reed – New York – Romeo Had Juliette

1) 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.”

Advertisements

Posted in Dev Leads, Tips, Tools, and Tricks | Tagged: , , , | Leave a Comment »