Exceptions are not exceptional

This post was originally published on November 3rd, 2012 on my personal blog.

Earlier today, I decided to fix a small JSHint issue that’d been bothering me for quite some time. You can see the issue in this snippet, where I’ve commented out all of JSHint’s error detection code and left just the shell that wraps that:

try {
  advance();
  /* Almost complete JSHint parsing logic */
} catch (err) {
  if (err) {
    var nt = state.tokens.next || {};
    JSHINT.errors.push({
      scope: “(main)”,
      raw: err.raw,
      reason: err.message,
      line: err.line || nt.line,
      character: err.character || nt.from
    }, null);
  }
}

As you can see, JSHint (and JSLint for that matter) is almost entirely wrapped in one giant try/catch block. All errors go directly to the JSHINT.errors array without ever propagating to the outer scope. This behavior bit me a few times before so I finally decided to change the catch block:

try {
  advance();
  /* Almost complete JSHint parsing logic */
} catch (err) {
  if (err && err.name === “JSHintError”) {
    var nt = state.tokens.next || {};
    JSHINT.errors.push({
      scope : “(main)”,
      raw : err.raw,
      reason : err.message,
      line : err.line || nt.line,
      character : err.character || nt.from
    }, null);
  } else {
    throw err;
  }
}

Boom! This seemingly minor change suddenly opened a pretty big box full of nasty worms. Because, apparently, some parts of JSHint/JSLint work only if they blow up in the middle of execution. Consider the following code:

prefix(“!”, function () {
  this.right = expression(150);
  this.arity = “unary”;
  if (bang[this.right.id] === true) {
    warning(“W018", this, “!”);
  }
  return this;
});

What happens when our input is a string with only one character “!” in it? In this case, a call to expression returns undefined and the line with this.right.id blows up. The catch block I demonstrated above catches that exception, puts it into the JSHINT.errors array and halts execution. And since this exception didn’t have all required fields (like err.raw) the method responsible for generating reports will simply ignore it. In the end, this means that JSHint will appear to be working as expected.

This is horrible. Not only does it make debugging extremely annoying, it also makes sure that our tests don’t catch all corner cases.

So I changed this instance and a few others to explicitly check whether there are more tokens in the input stream and if there are none, give up and quit. But now I can’t really ship JSHint like this because, even though our test coverage is pretty good, I can’t guarantee that it won’t blow up somewhere else. This means I will have to ship it with a switch:

// Propagate exceptions, but only in debug mode. 
if (debugMode) {
  throw err;
}

The moral of this story is that there is absolutely nothing exceptional in invalid input strings and your error handling code, in most cases, is better when it’s explicit and local.

So now, after spending hours on this and similar bugs, I really see the point behind error handling in Go.