Terse Exception Messages

Yesterday, whilst putting together a testing tool, I hit one of those generic framework error messages that says exactly what’s wrong but says nothing about where the problem might be. A large part of this diversion was down to being half way through adding support for a new command line verb and then returning to it after a lengthy phone call whilst misremembering that I hadn’t quite finished wiring in the initial NOP implementation.

Everything looked to be in the right place, but when I ran it I got no output at all - not even an error message. Luckily I always test my command line applications with my two-line batch file (See “Windows Batch File Template”) that reports the process exit code to ensure I’m returning the correct code on failure. It showed a “2” which is our generic failure code, so that was ok.

A quick spot of debugging of our console application base class and the problem was easy to see, the code looked like this:

int exitCode = ExitCode.Failure;

foreach (var handler in _handlers)
{
  if (handler.Verb == verb)
  {
    exitCode = handler.Execute(options);
    break;
  }
}

return exitCode;

As you can probably see if there is no handler wired up for the verb then it just falls out the bottom. Luckily we got one thing right which was to adopt the stance of “Assume Failure by Default” so that at least an error was reported, albeit not a particularly helpful one.

A Better Error Message

One could argue at this point that while the code is probably not brilliant, given the domain (a console application), it’s fairly easy to test and fix whatever mistake I made. But, is it worth putting any effort into improving matters and if so how much? Personally I think avoiding fixing this kind of problem is part of what leads to the “Broken Windows” syndrome. It’s only a small piece of code and a sprinkling of LINQ will soon sort it out:

return _handlers.Single(v => v.Name == verb); 
                .Execute(options);

This is much simpler and it generates a bit more noise when it fails, which feels like A Good Thing. When I ran it I got the following error in my console window though:

{System.InvalidOperationException} Sequence contains no matching element

Anyone who has ever done any LINQ will recognise this somewhat terse error message. Of course it’s not the framework’s fault, after all how can they know anything about how the method is used? And it uses generics too so it’s not as if they could easily tell you what the element was that you were looking for because we’ve passed a lambda in.

An Even Better Error Message

At this point we have managed to reduce our code to something that is pretty simple and gets the job done - case closed, right?. The question I asked myself though was whether I should undo some of that simplification in order to produce a better diagnostic message? As I mentioned above the scenario where this code is used means it wouldn’t take long with a debugger to track down the mistake and so is there really any value in going one step further?

I have found in the past that developers often concentrate on the happy path and then generally gloss over the error paths, by which I mean they make sure an error is reported, but don’t put too much effort into ensuring that what is reported would be of use to fellow developers or, more importantly, the support team. I’ve discussed this before in “Diagnostic & Support User Interfaces” but suffice to say that a key part of what makes a complex system supportable is well thought out error handling and clearly written diagnostic messages.

In this instance I decided the LINQ exception was still too terse, and although I wouldn’t be able to pinpoint the source of the error clearly, I felt I could probably do much better with only a little extra code:

var handler = _handlers.SingleOrDefault(v =>
                                    v.Name == verb);

Constraint.MustNotBeNull(handler,
    "No handler configured for verb '{0}'", verbName);

return handler.Execute(options);

This meant that the output from running the console application now was more like this:

{ConstraintViolationException} No handler configured for verb 'new-verb'

By switching from using Single() to SingleOrDefault() I got to catch the missing handler problem myself and effectively report it through an assertion failure [1]. Of course I don’t explicitly catch the case where two handlers with the same name are registered; that fault will still be reported via a terser LINQ exception. However I felt at the time that this was less likely (in retrospect a copy-and-paste error is probably more likely).

The Anguish of Choice

This is the kind of small problem I bump into all the time and I never really know which way to go. While on the one hand I love the simplicity of the code that I could have written, I’ve also worked on too many codebases where problems are tedious to solve because not quite enough thought was put into the likely failure scenarios up front, or refactored when they did show up so that it improves the lives of our successors [2].

 

[1] The Constraint class plays a similar role to Debug.Assert() but is compiled into release builds too. Code contracts could also be used but I wrote this long before working on a C# codebase where the use of real Code Contracts was possible.

[2] A seamless link to another recent post: “Will Your Successor Be a Superstar Programmer”.