Promise.all() behaving unexpectedly, where is my lack of understanding?

Question

I am trying to write a script which uses the GitHub API. I have a function that accepts a list of usernames. For each username a call is made to the API to get that users starred repos. For each users starred repo i'd like to capture the name of the repo and the number of stars, eventually I'd like to do more work with this data.

I'm trying to implement this with just native promises and avoid Q, Bluebird etc. Here's what I have that is not working.

function getNameAndStarInfo(repo){
  return new Promise(function(resolve, reject){
    //i'd like this to return an object but it has to be an iterable??
    resolve([{[repo.full_name] : repo.stargazers_count}]); 
  });
};

function getStarredRepos(usernames){
  var promises =[];
  for (var user in usernames){
    //build the header for API request

    var name = usernames[user];
    var url = 'https://api.github.com/users/' + name + '/starred';
    var header = {url: url, headers: {'User-Agent': 'username', 'Authorization': 'token blahblabhlabh'}, json: true };

    //for the current user make a request and get their starred repos
    request(header, function(err, res, usersStarredRepos){
        for (var repo in usersStarredRepos){
            promises.push(getNameAndStarInfo(usersStarredRepos[repo]));

        };
    });
   };
   Promise.all(promises)
      .then(function(promises){
        //dont need to log, would like to do stuff with this data later
        console.log(promises);
    });

};

Essentially, I'm expecting to see a all of the {name: stargazers_count} data when I log the array after in the .then() method. However an empty array is output every time. Could someone explain to me what I'm missing here?

you called Promise.all on an empty array. Somewhat of a duplicate of: http://stackoverflow.com/questions/14220321/how-do-i-return-the-response-from-an-asynchronous-call — Kevin B, Sep 15 '16 at 19:32

score 1 · Accepted Answer · edited Sep 15 '16 at 20:10

You are pushing into promises only when a request is already done. So when Promise.all is being executed, the array is still empty because none of them has finished yet.

Try with this way:

function getNameAndStarInfo(repo) {
    return new Promise(function(resolve, reject) {
        //i'd like this to return an object but it has to be an iterable??
        resolve([{
            [repo.full_name]: repo.stargazers_count
        }]);
    });
}

function getUserStars(username) {
    return new Promise(function(resolve, reject) {
        var name = username.name;
        var url = 'https://api.github.com/users/' + name + '/starred';
        var header = { url: url, headers: { 'User-Agent': 'username', 'Authorization': 'token blahblabhlabh' }, json: true };
        //for the current user make a request and get their starred repos
        request(header, function(err, res, usersStarredRepos) {
            var promises = [];
            for (var repo in usersStarredRepos) {
                promises.push(getNameAndStarInfo(usersStarredRepos[repo]));
            };
            Promise.all(promises).then(resolve).catch(reject);
        });
    });
}

function getStarredRepos(usernames) {
    var promises = [];
    for (var user in usernames) {
        promises.push(getUserStars(usernames[user]));
    }
    Promise.all(promises).then(console.log).catch(console.log);
}

But to avoid creating Promises manually to handle requests, i would suggest you to use request-promise:

https://www.npmjs.com/package/request-promise

Avoid the [`Promise` constructor antipattern](http://stackoverflow.com/q/23803743/1048572)! You should promisify `request` only, and chain to it. — Bergi, Sep 15 '16 at 20:07
@Bergi the question is strictly about Promise.all and not on Promises patterns. He asked why the Promise.all wasn't working as he expected. I told him the cause, and then, just wrote a code (following his patterns) as an example. About the number of the requests being fired, i'm not seeing that happening. I was firing many request, one for each user. The code wasn't even needed. He just asked for an explanation. — Diego ZoracKy, Sep 15 '16 at 20:13
@DiegoZoracKy Yes, the answer regarding `Promise.all` is correct, but the code you wrote is a bad example. Please fix it, this pattern should not be used. — Bergi, Sep 15 '16 at 20:18
@DiegoZoracKy Regarding my deleted comment about the number of requests, I was confused by the function naming; I've suggested an edit. — Bergi, Sep 15 '16 at 20:19

Promise.all() behaving unexpectedly, where is my lack of understanding?

1 Answers1