What is the proper response to lousy error message?

Question

I've just come across (for the 47 millionth time) some code that looks like this:

except IOError, e:
  print "Problems reading file: %s." % filename
  sys.exit( 1 )

My first reaction is very visceral: the person who coded this is a complete idiot. How hard is it to print error messages to stderr and to include the system error message in the string? It certainly used to be the case that, for simple programs (command line filters), best practice was to print error messages to stderr and (when appropriate) to include the system error in the message. A very large percentage of developers no longer follow those rules. Many questions of the form "what is best practice for error messages" include answers about massive logging frameworks and exception handling mechanisms that do not apply to the simple filter. Has best practice changed? or is failure to include system error messages and printing error messages to the wrong stream the mark of a novice?

Answer 1

Either fix it yourself or report a bug to the programmer responsible for this to fix it.

Additionally there are two code smells in this:

• No logging framework.
• Code should not just abruptly exit.

It is important to have the exception bubble to the main method, have it logged properly and THEN exit. This allows upstream code to properly wrap things up - close files, empty buffers, release system resources, etc.

Answer 2

While it's hard to excuse error handling with this little information in, the reality is that scoping error handling is actually pretty tricky.

Too little and you end up unable to troubleshoot even the most straight forward of problems, too much and you end up with logs that cycle before you can get to them, fill servers and where writing the error handling takes as long as writing the code without having any idea whether it's even going to be problematic (seriously, I've seen code which dumps every variable out on every function call but which has never gone majorly wrong).

In this example it might be worth asking how long has this code been around and what does it do. If it's been around for 10 years and it's still doing this it's probably a sign that the code is actually trouble free and that any error handling is unnecessary.

And depending on what it's doing it may be that either you don't need to know about the nature of the error (because all you care about is that it failed and it's irrelevant why) or that the code does such as specific thing that anyone who understands it will be able to guess what's happened 99% of the time with no additional information.

Ultimately you're right, it's basically no more work to put in the extra information and it should happen but you're harsh saying you have no respect for them.

It's probably a case of inexperience or time pressure, or writing something quick and dirty which they thought would run once and somehow ended up in still production a decade later. Those are all a bit rubbish but we've all been there and let the programmer who has never turned out less than perfect code cast the first stone.

In terms of specifics I'd fix it and maybe politely mail the team saying that you've noticed this and wouldn't it be better and no more work to include the standard message (including a nice code sample so they don't even need to look it up).

But on a wider note getting error handling right is harder than it seems.

Answer 3

One potential healthy reaction after you get through the disgust (and its normal to feel disgust):

Could it have been that the programmer who did that was under such a severe time crunch that he/she was grateful just to have the time to be able to report any error string at all? The programmer him or herself may wish they had the time do polish everything, but did the best they could under very difficult circumstances.

I've seen a lot of code where I work that's terrible and ugly but was written by very smart people trying to get the company through the next pay cycle. I'm just grateful it was written because them getting that project done has enabled me to have a job where I can complain about how bad the original code is :)

Answer 4

I think you're being too rude here. Especially since your suggestion (including system error message) can be criticized. I don't know how does it happen in Python, but in .NET Framework, exception messages are not expected to be displayed to the end user. They are written for developers, and only for them. There are at least three reasons for that:

• The language of .NET Framework may be different from the UI language (including when the UI language is configurable by the user).
• The exception messages are sometimes scary or not helpful for an end user (Would you like to show to your user something like: "Object reference not set to an instance of an object"?).
• Giving an exact exception message is not very secure.

In his answer, Thorbjørn Ravn Andersen points the two most important problems with the code: no logging and abrupt exit. But what about the error message itself?

I would say it's not so bad compared to others. I see a lot of strange, wrong, unhelpful messages even in commercial popular products. Examples:

• When dealing with the names of files which exceed 259 characters, Microsoft Word tells us a very helpful story that our floppy drive is too small (on a machine where there is no floppy drive at all) and suggests, guess what?... that we update our antivirus.

• When something wrong happens in Visual Studio, sometimes it tells that, well, something wrong happened, but it doesn't bother to tell us what, and doesn't even want to tell where log files are (are those errors logged, by the way?).

• On my old phone, when trying to send mails, it tells me something like "Operation failed". Oh, thanks for such details, now I know precisely what was wrong!

• etc.

DailyWTF has also lots of examples of such error messages. Including pessimistic messages like "Error: success".

In many cases, you can see something like:

try
{
    // Thousands of lines here (hopefully not in the same method).
}
catch (Exception) // After all, who cares about *what* is exactly the exception?
{
    MessageBox.Show(@"Something went wrong. Please restart your application.
If you see this message again, contact your administrator.");

    // And yes, we don't want to be polite, don't want to tell what's wrong,
    // but want to suck administrators by telling them that they must contact
    // themselves.
}

If you put a error message, if it's not correctly written (because you don't have time, or whatsoever), at least:

• Don't put stupid suggestions, like "contact your administrator",
• Make the error searchable on internet (including some error number can help), or at least make sure that there is only one error in a specific context.
• Ensure that the user can report you the error. Can you solve quickly the bug report if it says:

Bug 1024: I think I've found a bug. When working with your great and user friendly application, sometimes, it randomly displays "The application encountered a fatal error." message. Can you solve it, please?

Answer 5

I think my number is around the same (47 million, give or take) of running into somewhat equivalent C# code:

try
{
    // Tricksy code be here.
    return true;
}
catch (Exception ex)
{
    Logger.Error("This error occurred: " + ex.Message);
    return false;
}

So, in our log, we wind up with a number of oh-so-useful "Object reference not set to an instance of an object" just sitting there with no other context. How hard is it to actually type LESS and have it be:

    Logger.Error("This error occurred: " + ex);

Much more useful information for the developers.

Now as to your second question: is the developer a novice? Maybe, maybe not. But certainly the developer didn't think ahead to what might be needed to do forensic reconstruction of an error.