How to resolve variable number of promises in node.js
I am working on a function (called express.js cursor) to combine event information in the database with my Facebook copy and return it as an array of event objects.
I am having problems with the asynchronous nature of node.js and resolving a variable number of promises in a foreach loop before returning the whole object. I've tried many different ways to reorder my code (callbacks, counters, promises, etc.), but I haven't been able to solve this problem and I really would like to know why. I suspect it has something to do with swapping variables in the foreach loop, but I'm not sure how to solve this.
I am looking for three things:
- What I don't understand conceptually what is needed to solve this problem?
- How can 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 to console.log (results [0]):
{ promise: [object Object],
resolve: [Function],
fulfill: [Function],
reject: [Function],
notify: [Function] }
The code links I looked at:
- https://github.com/kriskowal/q
- https://github.com/kriskowal/q/wiki/API-Reference
- https://strongloop.com/strongblog/how-to-compose-node-js-promises-with-q/
- http://thejsguy.com/javascript/node.js/2014/06/27/JavaScript-Flow-Control.html
Oh, and here's my function for single fb / db event pooling 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;
}
source to share
Your main problem is here:
var promise = q.defer();
promises.push(promise);
q.defer()
does not return a promise. It returns deferred .
var result = q.defer();
promises.push(result.promise);
Correctly naming variables is important, you don't notice the error because you chose the wrong variable names.
It 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 quite long and can use a little refactoring.
- And of course, don't call your deferred objects "deferred" or your promises "promise." It's not descriptive.
- Read What Is The Explicit Promise About Building Antipatterns And How To Avoid It? (let it sink, it takes a while)
This is what I will be using.
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
, change it to reject directly and on your own, instead of resolving with the falsy value auth
, which needs to be checked every time you use it. After that, you can delete the line if (!auth) ...
.
Also, the part //more code in here...
that deals with "extended" events should probably live out mergeEvents
.
source to share