February 21, 2013
Honour thy async signature

When writing asynchronous JavaScript code in node.js there is a common pattern that nearly everyone follows of providing a callback to the async function which is to be called once the function completes, the function takes an error parameter as the first argument and the results as the second. It is common to see code like:

function doWork() {
    getUserAsync(‘bob’, function(error, user) {
        if (error) {
            console.log('Whoops’);
            return;
        }
        console.log('fetched user’);
    });
}
doWork();

Callbacks are how node currently handles asyncronous callbacks, but it is important to note “callbacks != async” you can also have synchronous callbacks e.g.

function printUsers(users, print) {
    for(var i=0; i<users.length; ++i) {
        print(users[i]);
    }
}
printUsers(['bob’, 'joe’], function(username) { console.log(username); });

Here you can see we have a callback function but it is called synchronously by the printUsers function.

So where am I going with this. Well in node, if your function has an async signature you should always make sure it is async, you should always honour the asynchronous expectation a caller may have assumed is always implicit.

Let’s pretend that you are writing some code that connects to Amazon S3 then uploads a file to S3. The api connects and the callback is fired when the connection succeeds or fails:

var s3Client = S3.connect(function(error) {
    if (error) {
        console.log('Failed to connect’);
        return;
    }
    console.log('Successfully connected’);

    // do some work
    s3Client.upload('foo.txt’, 'this is the contents of the file’);
});

This looks innocuous enough, if the client connects successfully we upload some content, however if the author of the S3 library did something like the following:

var S3 = {
    connectionOpen: false,

    connect: function(callback) {
        if (this.connectionOpen) {
            // If the connection is already open we can callback immediately
            callback();
        }
        else {
            // open a connection, simulate with a timeout for purposes
            // of the example an async network callback
            var self = this;
            setTimeout(function() {
                self.connectionOpen = true;
                callback();
            }, 1000);
        }
        return this;
    }
};

They have written an optimization, if the underlying connection is already open (maybe they are using connection pooling etc.), they simply callback synchronously, otherwise they do the full async connection. The problem is in our example code, if the connect callback is called synchronously you can see the s3Client variable hasn’t been set yet and so the code will crash with a null reference error!

Obviously as an SDK writer you wouldn’t want this kind of signature in the first place, you would want a constructor that you can call to create the client then probably a “connect” method, so you don’t run into this pattern, but I hope this example gives you the idea.

So, to work around this, always make sure you call any callback to your methods in an async manner, this is simple to do using process.nextTick, for example we could have written the above connect method as:

if(connectionOpen) {
    process.nextTick(callback);
}

This ensures the callback is not called immediately and is added to the event queue to be processed after the calling code has finished executing.