NodeJS callback sequence

Folks, I have the following function, and am wondering whats the correct way to call the callback() only when the database operation completes on all items:

function mapSomething (callback) {
    _.each(someArray, function (item) {
        dao.dosomething(item.foo, function (err, account) {
            item.email = account.email;
        });
    });
    callback();
},

What I need is to iterate over someArray and do a database call for each element. After replacing all items in the array, I need to only then call the callback. Ofcourse the callback is in the incorrect place right now

Thanks!

This will not wait for all your database calls to be done before calling callback(). It will launch all the database calls at once in parallel (I'm assuming that's what dao.dosomething() is). And, then immediately call callback() before any of the database calls have finished.

You have several choices to solve the problem.

  1. You can use promises (by promisifying the database call) and then use Promise.all() to wait for all the database calls to be done.
  2. You can use the async library to manage the coordination for you.
  3. You can keep track of when each one is done yourself and when the last one is done, call your callback.

I would recommend options 1. or 2. Personally, I prefer to use promises and since you're interfacing with a database, this is probably not the only place you're making database calls, so I'd promisify the interface (bluebird will do that for you in one function call) and then use promises.

Here's what a promise solution could look like:

var Promise = require('bluebird');

// make promise version of your DB function
// ideally, you'd promisify the whole DB API with .promisifyAll()
var dosomething = Promise.promisify(dao.dosomething, dao);

function mapSomething (callback, errCallback) {
    Promise.all(_.map(someArray, function(item) {
        return dosomething(item.foo).then(function (account) {
            item.email = account.email;
        });
    }).then(callback, errCallback);
}

This assumes you want to run all the DB calls in parallel and then call the callback when they are all done.

FYI, here's a link to how Bluebird promisify's existing APIs. I use this mechanism for all async file I/O in node and it saves a ton of coding time and makes error handling much more sane. Async callbacks are a nightmare for proper error handling, especially if exceptions can be thrown from async callbacks.


P.S. You may actually want your mapSomething() function to just return a promise itself so the caller is then responsible for specifying their own .then() handler and it allows the caller to use the returned promise for their own synchronization with other things (e.g. it's just more flexible that way).

function mapSomething() {
    return Promise.all(_.map(someArray, function(item) {
        return dosomething(item.foo).then(function (account) {
            item.email = account.email;
        });
    })
}

mapSomething.then(mapSucessHandler, mapErrorHandler);

I haven't tried Bluebird's .map() myself, but once you've promisified the database call, I think it would simplify it a bit more like this:

function mapSomething() {
    return Promise.map(someArray, function(item) {
        return dosomething(item.foo).then(function (account) {
            item.email = account.email;
        });
    })
}

mapSomething.then(mapSucessHandler, mapErrorHandler);

The way you currently have it, callback is executed before any of the (async) tasks finish.

The async module has an each() that allows for a final callback:

var async = require('async');

// ...

function mapSomething (callback) {
  async.each(someArray, function(item, cb) {
    dao.dosomething(item.foo, function(err, account) {
      if (err)
        return cb(err);
      item.email = account.email;
      cb();
    });
  }, callback);
}