then() for Q.all() not being called

Question

In Node.js, I am waiting for several requests to finish before I make a function call as follows:

function loadQuotes(symbols){
    var promises = [];
    for(var s in symbols){
        var deferred = Q.defer();
        var url = rootURL;
        request(url, function (error, response, body) {
            if (!error && response.statusCode == 200) {
                var info = JSON.parse(body);
                    deferred.resolve();
                    console.log("resolved");
            }
        });
        promises.push(deferred.promise);
    }
    return Q.all(promises);
}

I am waiting for all the requests as follows

loadQuotes(symbols.slice(0,3)).then(function(data){
    console.log("done");
    httpServer();
}).done();

All of the requests are made, but I do not see "done" printed to the console. The promises variable is indeed a list of promises, and they have all been "resolved", yet the then function is not called. Any ideas?

Answer 1

You have a scoping problem. Your variable deferred is local to your whole function, not just to the for loop so you are overwriting that variable in your loop before you use it in the callback and thus you don't resolve all the deferred objects you created. Remember the callback happens sometime later after the for loop has fully run its course.

A simple fix would be to change to use let instead of var. That will make your deferred variable local to the for loop scope rather than the whole function scope.

But, my preferred fix would be to make a promisified version of request() that returns a promise and use that.

function rp(url) {
    return new Promise(function(resolve, reject) {
        request(url, function(err, response, body) {
            if (err) return reject(err);
            if (response.statusCode !== 200) reject(new Error(response.statusCode));
            resolve(body);
        });
    });
}

Then, you can just use that:

function loadQuotes(symbols){
    var promises = [];
    for(var s in symbols){
        var url = rootURL;
        promises.push(rp(url));
    }
    return Promise.all(promises);
}

Or, if you really need to use the Q library still, you could write the above code using that too.

FYI, there is a request-promise library that returns a promise that is a substitute for the request library. You could use that too.

---

Note, assuming symbols is an array and you want to stick with an ES5 solution, you could also just switch your for loop to use .forEach() and that would create a new function scope for each invocation of the loop, also solving your issue.

function loadQuotes(symbols){
    var promises = [];
    symbols.forEach(function(s) {
        var deferred = Q.defer();
        var url = rootURL;
        request(url, function (error, response, body) {
            if (!error && response.statusCode == 200) {
                var info = JSON.parse(body);
                deferred.resolve(info);
                console.log("resolved");
            }
        });
        promises.push(deferred.promise);
    });
    return Q.all(promises);
}

---

EDIT Jan, 2020 - request() module in maintenance mode

FYI, the request module and its derivatives like request-promise are now in maintenance mode and will not be actively developed to add new features. You can read more about the reasoning here. There is a list of alternatives in this table with some discussion of each one. I have been using got() myself and it's built from the beginning to use promises and is simple to use.