CODEWITHSUNDEEP
javascriptnode.jspromiseq
Surya
Surya

Reputation: 1197

Node js not resolving array of promises

I'm trying to execute several async requests and trying to get the output using promises.

If I've multiple requests queued up the Q.all(promises).then () function doesn't seem to be working. For a single request the promises are all resolved. The sample code is here.

var request = require('request');
var Q = require('q');

var sites = ['http://www.google.com', 'http://www.example.com', 'http://www.yahoo.com'];
// var sites = ['http://www.google.com']

var promises = [];

for (site in sites) {
  var deferred = Q.defer();
  promises.push(deferred.promise);
  options = {url: sites[site]};

  request(options, function (error, msg, body) {
    if (error) {
      deferred.reject();
    }

    deferred.resolve();
  });
}

Q.all(promises).then (function () {
  console.log('All Done');
});

What am I doing wrong here ?

Surya

Upvotes: 1

Views: 1686

Answers (3)

Benjamin Gruenbaum
Benjamin Gruenbaum

Reputation: 276306

Here is what I'd do in the scenario, this is the whole code:

var Q = require('q');
var request = Q.nfbind(require('request'));

var sites = ['http://www.google.com', 'http://www.example.com', 'http://www.yahoo.com'];
var requests = sites.map(request); 

Q.all(requests).then(function(results){
   console.log("All done") // you can access the results of the requests here
});

Now for the why:

  • Always promisify at the lowest level possible, promisify the request itself and not the speicific requests. Prefer automatic promisification to manual promisification to not make silly errors.
  • When working on collections - using .map is easier than manually iterating them since we're producing exactly one action per URL here.

This solution is also short and requires minimal nesting.

Upvotes: 3

Leonid Beschastny
Leonid Beschastny

Reputation: 51470

The problem is that you're changing the value of deferred on every tick of your for loop. So, the only promise which is actually resolved in your example is the last one.

To fix it you should store the value of deferred in some context. The easiest way to do so is to use Array.prototype.forEach() method instead of for loop:

sites.forEach(function (site){
  var deferred = Q.defer();
  var options = {url: sites[site]};
  promises.push(deferred.promise);

  request(options, function (error, msg, body) {
    if (error) {
      deferred.reject();
    }

    deferred.resolve();
  });
})

And you also missed var declaring options variable. In JavaScript it means declaration of a global variable (or module-wide variable in node.js).

Upvotes: 0

Explosion Pills
Explosion Pills

Reputation: 191749

Don't use for..in to iterate over arrays. What this actually does is set site to 0, 1, 2 and this just doesn't work very well. Use some other form of iteration like a regular for loop, or Array.prototype.forEach

sites.forEach(function (site) {                                                 
  var deferred = Q.defer();                                                     
  promises.push(deferred.promise);                                               
  options = {url: site};

Upvotes: 0

Related Questions