How to resolve a variable number of promises in node.js

Question

I'm working on a function (called by an express.js route) to merge event info in a database with its Facebook counterpart and return it as an array of event objects.

I am having trouble with the asynchronous nature of node.js and resolving a variable number of promises within a foreach loop before returning the whole object. I've tried numerous different methods of rearranging my code (callbacks, counters, promises, etc.), but I have not been successful in solving this problem, and I would really like to know why. I suspect it has to do with variables being overwritten in the foreach loop, but I'm not sure how to solve that.

I am looking for three things:

What am I not grasping conceptually that is needed to solve this problem?
How would I figure this out or debug this in the future?
How do I fix my code to make it work?

Here is my function:

function mergeEvents(req, res, next, events){

console.log("Merge Events");

var dfd = q.defer();

ensureAuthenticated(req, res, next).then(function(auth){
    var iEvent, event;
    var promises = [];

    if (auth){
        console.log("authenticated!");
        console.log("auth token: " + ACCESS_TOKEN);

        for (iEvent in events){
            event = events[iEvent];

            var promise = q.defer();
            promises.push(promise);

            https.get('https://graph.facebook.com/' + event.fb_id + '?access_token=' + ACCESS_TOKEN, function(response) {
                var str = '';
                response.on('data', function(chunk){
                    str += chunk;
                });

                response.on('end', function(){
                    var fb_event = JSON.parse(str);
                    event.dataValues.fb = fb_event;
                    promise.resolve(event);
                });
            });

            if (promises.length == events.length){
                console.log("last run through");
                q.all(promises).then(function(results){
                    console.log("all promises completed?");
                    console.log(results[0]); //OUTPUT BELOW
                    //more code in here... but promises haven't resolved
                    //...
                    dfd.resolve(events);
                });
            }
        }
    }else{
        console.log("Not authenticated. Redirecting to main page.");
        dfd.resolve(events);
    }
});

return dfd.promise;

}

While I am trying to get a JSON object, it returns an unresolved promise on console.log(results[0]):

{ promise: [object Object],
  resolve: [Function],
  fulfill: [Function],
  reject: [Function],
  notify: [Function] }

Code references I have viewed:

Oh, and here's my function for a single event fb/db merge that works, so you can compare:

function mergeEvent(req, res, next, event){
console.log("Merge Event");

var dfd = q.defer();

ensureAuthenticated(req, res, next).then(function(auth){
    if (auth){
        console.log("authenticated!");
        console.log("auth token: " + ACCESS_TOKEN);
        https.get('https://graph.facebook.com/' + event.fb_id + '?access_token=' + ACCESS_TOKEN, function(response) {
            var str = '';
            response.on('data', function(chunk){
                str += chunk;
            });

            response.on('end', function(){
                var fb_event = JSON.parse(str);
                event.dataValues.fb = fb_event;
                dfd.resolve(event);
            });
        });
    }else{
        console.log("not authenticated. redirecting to main page");
        dfd.resolve(event);
    }
});

return dfd.promise;
}

score 6 · Accepted Answer · edited May 23 '17 at 12:08

Your main problem is here:

var promise = q.defer();
promises.push(promise);

q.defer() does not return a promise. It returns a deferred.

var result = q.defer();
promises.push(result.promise);

Naming variables correctly is important, you didn't see the mistake because you chose improper variable names.

That being said...

Avoid for .. in. Arrays have .forEach() or .map().
Instead of checking if (promises.length == events.length), move that part out of the loop.
Your function is pretty long and could use a little refactoring.
And of course, don't call your deferred objects "deferred" or your promise objects "promise". That's not descriptive.
Read through What is the explicit promise construction antipattern and how do I avoid it? (let it sink in, it takes some time)

Here's what I would use.

var q = require('q');
var qHttp = require("q-io/http"); // -> https://github.com/kriskowal/q-io

var FB = {
    // collect other FB API methods here, maybe transform into module
    graph: function (id) {
        var url = 'https://graph.facebook.com/' + id + '?access_token=' + ACCESS_TOKEN;
        return qHttp.read(url).then(function (data) {
            return JSON.parse(data.toString());
        });
    }
};

function mergeEvents(req, res, next, events) {
    return ensureAuthenticated(req, res, next).then(function (auth) {
        if (!auth) return q.reject("Not authenticated.");

        return q.all(events.map(function (event) {
            return FB.graph(event.fb_id).then(function (data) {
                event.dataValues.fb = data;
                return event;
            });
        }).then(function (results) {
            //more code in here...
        }));
    });
}

Note: If you wrote ensureAuthenticated, modify it to reject directly and on its own instead of resolving with a falsy auth value that you need to check every time you use it. The line if (!auth) ... could be removed after that.

Also, the //more code in here... part that deals with the "enhanced" events should probably live outside of mergeEvents.

Thank you! As a beginner with promises, this was very helpful, as were the resources :) — smileham, Apr 26 '15 at 22:16

How to resolve a variable number of promises in node.js

1 Answers1