Some asynchronous JavaScript weirdness

I feel a little silly about the time it took me to find that bug, but it’s a fairly typical one, so it’s worth sharing. While writing a test case for an asynchronous operation, execution of my code would seemingly magically skip over a whole bit of code and jump directly to the assertions at the end of my test case. It looked really bizarre, as if JavaScript didn’t want to run my test code, and I suspected a bug in the debugger for a brief moment. Of course, that was just me not spotting the obvious bug I had written a few minutes earlier. But let me show you some simplified code that demonstrates the issue...

The test code goes something like this:

it('can do something asynchronous', function(done) {
  var payload = {stuff: 'foo', shouldHaveChanged: false};
  doSomethingAsynchronous(payload, function() {
    expect(payload.shouldHaveChanged).to.be.true;
    done();
  });
});

The method being tested looks like this:

/**
 * @description Does something asynchronous.
 * @param {object}   payload The payload for this operation.
 * @param {Function} done    The function to call back when it's done.
 */
function doSomethingAsynchronous(payload, done) {
  async.each(
    ['foo', 'bar', 'baz'],
    function(item, next) {
      doSomethingElseAsynchronous(payload, item, next);
    },
    done()
  );
}

Do you see it? Yes, it’s those parentheses after “done”. The async.each method takes the function to call when all operations are done as its third parameter, but because there are parentheses, this means that the function will be evaluated before async.each gets called with the results of the evaluation. That never happens however, because evaluating the function means executing the callback that’s in the test case and that is asserting the results of an operation that hasn’t happened. So the test fails, before the actual code can run.

The correct method code is:

/**
 * @description Does something asynchronous.
 * @param {object}   payload The payload for this operation.
 * @param {Function} done    The function to call back when it's done.
 */
function doSomethingAsynchronous(payload, done) {
  async.each(
    ['foo', 'bar', 'baz'],
    function(item, next) {
      doSomethingElseAsynchronous(payload, item, next);
    },
    done
  );
}

And that runs as expected.

This may look like a silly mistake, but I actually didn’t write that code, the IDE did. I only typed “d”, and auto-completion completed it into “done()”, and I didn’t notice. It did so because it knew that “done” was a function, from inspecting my JsDoc comments. It did not however have any idea what the expected type was for the third parameter of async.each, because this library does not use JsDoc comments.

In summary, this is a case of half a static analysis resulting in bad assumptions. I make it a point to put JsDoc everywhere, not just as documentation, but also as a way to help my IDE do a good job of statically analyzing the code and give my JavaScript some robustness that is otherwise hard to achieve in dynamic languages.