0

I am using Bluebird library with NodeJS (with SailsJS framework)

Promise.all() is not capturing the event when all the promises in promises array are resolved.

What changes should be made in order to solve this problem?

var Promise = require("bluebird");
var request = require('request');
var http = require('http');

function searchMultiple(titles) {

  var results = [];

  return new Promise( function( resolveGlobal, rejectGlobal ){

    var url = "http://xxx.xxx";
    var promises = [];

    titles.forEach(function (title, index) {

      promises[index] = new Promise( function (resolve, reject) {

        var data = {"x":title};

        request({
          uri: url,
          method: "POST",
          body : data
        }, function(error, response, body) {
             return resolve(body)
            }
          }
        },
        function (error, response, body) {
          console.log("error");
          return resolve();
        }
        );
      })
    })

    Promise.all(promises).then(function(combinedResults) {
      console.log("successfully resolved all promises");
      return resolveGlobal(combinedResults);
    }).catch(function (reason) {
      console.log("error");
      return rejectGlobal();
    });

  })
}
Vishwajeet Vatharkar
  • 1,146
  • 4
  • 18
  • 42

2 Answers2

1

There is no need for you to return resolve(value), as it should only resolve the with the given result value.

There's also no reason to create a new promise in your searchMultiple function, since the Promise.all returns a promise. You should just return the promise you already have!

The resolveGlobal() is thus unnecessary and you can just return the result instead, since the then will wrap it as a resolved value.

All of your code can be rewritten as two very simple functions

function searchMultiple(titles) {
  //Empty array of promises
  var promises = [];

  var url = "http://xxx.xxx";

  //Get a promise for each title and push to array
  titles.forEach(function(title){
    promises.push(getData(title, url));
  });

  //Wait for all promises to resolve, and return the result
  return Promise.all(promises)
    .then(function(arrayOfResults){
      //If this only shall return the array, this can be omitted aswell as the catch!
      return arrayOfresults;
    })
    .catch(function(reason){
      //Handle errors
    });
}

function getData(title, url){
  return new Promise(function(resolve, reject){
    var data = {"x":title};
        request({
          uri: url,
          method: "POST",
          body : data
        }, function(error, response, body) {
             resolve(body)
            }
          }
        },
        function (error, response, body) {
          console.log("error");
          //Consider rejecting here instead since you'll get a result that is 'undefined'
          resolve();
        });
  });
}

You should consider rejecting the promise in the error handler instead of resolving it with an undefined value. You might end up with errors further down the road if you get a result array back that have values that are undefined.

Daniel B
  • 8,770
  • 5
  • 43
  • 76
  • You forgot `url` and `title` as parameters to the `getData` function – Bergi Dec 06 '16 at 15:01
  • 1
    Notice that `.then(function(arrayOfResults){ return arrayOfresults; })` is pointless and should be omitted if nothing is done with the array. – Bergi Dec 06 '16 at 15:02
  • 1
    Thanks, missed that! True about the `.then(..)` on the `Promise.all(..)`, I omitted the console printout but should perhaps have included that to make it more clear on why to keep the `.then(..)`. – Daniel B Dec 06 '16 at 15:05
0

Try this:

var Promise = require('bluebird');
var request = require('request');

function searchMultiple(titles) {
  return new Promise(function (resolveGlobal, rejectGlobal) {
    var url = 'http://xxx.xxx';
    var promises = [];

    titles.forEach(function (title) {
      promises
        .push(new Promise(function (resolve, reject) {
          var data = {
            'x': title
          };

          request({
              uri: url,
              method: 'POST',
              body: data
            },
            function (error, response, body) {
              if (!error && response.statusCode == 200) {
                return resolve(body);
              }

              return reject(error);
            });
        }));
    });

    Promise
      .all(promises)
      .then(function (combinedResults) {
        console.log('successfully resolved all promises');

        return resolveGlobal(combinedResults);
      })
      .catch(function (error) {
        console.log('error');

        return rejectGlobal(error);
      });
  });
}

And call function:

searchMultiple([...])
  .then(function (results) {
    console.log(results);
  })
  .catch(function (error) {
    console.log(error);
  });
Niezborala
  • 1,857
  • 1
  • 18
  • 27