Bluebird Promise.all doesn't get called

Question

I have a url which returns a list of other urls For each url, I want to do stuff and then use Promise.all to tell me that it finished.

For some reason, it does process all urls, but the Promise.all doesn't seem to be called (Bluebird)

What am I doing wrong?

var rp = require("request-promise");
var Promise = require("bluebird");

var promrequests = [];
rp(
 {
  url: url_of_list_of_urls,
  json: true,
 },
 function(error, response, body) {
  if (!error && response.statusCode === 200) {
   let urls = [];
   for (var i in body) {
    urls.push(body[i]);
   }

   for (let j in urls) {
    let url = urls[j];

    promrequests.push(
     rp(
      { url: url, followAllRedirects: true },
      function(error, response, body) {
       console.log("working on " + url);
       // do stuff
      }
     )
    );
   }

   Promise.all(promrequests).then(function() {
    console.log("finished all");
   });
  }
 }
);

Do you really mean that `Promise.all` doesn't get called, or that it does get called but the `then` callback doesn't run? — Bergi, Apr 02 '17 at 19:34
[Don't use `for…in` enumerations on arrays!](https://stackoverflow.com/q/500504/1048572) Both for `body` and `urls` — Bergi, Apr 02 '17 at 19:35
For goodness sakes, stop using `for/in` to iterate an array. Either use a traditional `for` loop, `.forEach()` or `for/of` all of which are built for arrays. What you are doing is built for iterating properties of an object which will include array entries, but can also include other properties. — jfriend00, Apr 02 '17 at 23:52
I thought the whole point of using `rp` was that you use `.then()` with it and DO NOT pass it a callback! You're trying to somehow use a hybrid of a little of each. That's bad. request-promise does NOT accept a callback as an argument. It JUST returns a promise. — jfriend00, Apr 02 '17 at 23:54

score 1 · Accepted Answer · answered Apr 02 '17 at 22:07

There's no need to use callbacks with the request-promise library, the example code is a mixture of the original request callback API and the request-promise API.

Bluebird also has a .map helper to make working with arrays easier.

Dropping all the callbacks and returning promises throughout the chain gives you something like (untested):

requestOptionsUrls = {
  url: url_of_list_of_urls,
  json: true,
}
rp(requestOptionsUrls).then(function(urls){
  return Promise.map(urls, function(url){
    return rp({ url: url, followAllRedirects: true })
      .then(function(body){
        console.log("working on " + url);
        // work
      })
  })
})
.then(function(){
  console.log("finished all");
})
.catch(function(error){
  console.error(error)
})

request-promise will do 2XX checking for you unless you set the simple request option to false.

It also only resolves the body of the request unless you set the resolveWithFullResponse to true, say if you wanted to do more complex checks on the status code of the response.

The code generally works, yet for some reason, the "finished all" part runs/prints before everything else :/ — Nick Ginanto, Apr 03 '17 at 03:40
seems that the finised all part needs to be on the `Promise.map` part. So I'll mark this as solved. Thanks! — Nick Ginanto, Apr 03 '17 at 04:13
That's odd, attaching the `.then()` directly to `Promise.map` should be the same as attaching the `.then()` to the returned value from `Promise.map`. They should both wait for the resolution of all the values inside. — Matt, Apr 03 '17 at 09:45

Bluebird Promise.all doesn't get called

1 Answers1