bluebird promise each not return the last result

Question

I am using promise.each to loop through 2 db query using bookshelfjs, but roles is not giving me the result of this -> roles[resource.get('name')] = role.get('name');, but it give me the object from slect statement:

var promise = new Promise(
        function resolver(resolve, reject) {   
            var roles = {};
            Promise.each(user.relations.relates.models, function(relation){
                return Resource.forge({resourceId: relation.get('resourceId')}).fetch().then(function(resource){
                    return Role.forge({roleId: relation.get('roleId')}).fetch().then(function(role){
                        roles[resource.get('name')] = role.get('name');
                        return roles;
                    }).catch(function(err){
                        reject({"status":"error", "data": err});
                    });
                }).catch(function(err){
                    reject({"status":"error", "data": err});
                });
            }).then(function(roles){
              resolve(roles);
            });
        }
    );

    return promise;

Answer 1

From the Bluebird docs for Promise.each():

Resolves to the original array unmodified, this method is meant to be used for side effects.

So, that means that when you do:

Promise.each(array).then(function(val) {
   // iteration finished here
   // val is the original array
});

That val will be the original array you passed in, not your roles object.

Because your roles object is in a higher scope, you can remove the parameter declared as roles in your .then() handler and just directly refer to the higher scoped roles variable.

You should also avoid the promise constructor anti-pattern here and not create a new promise, but just return the one you already have.

You can do that like this:

var roles = {};
return Promise.each(user.relations.relates.models, function (relation) {
    return Resource.forge({resourceId: relation.get('resourceId')}).fetch().then(function (resource) {
        return Role.forge({roleId: relation.get('roleId')}).fetch().then(function (role) {
            roles[resource.get('name')] = role.get('name');
        });
    }).catch(function (err) {
        // repackage how the error is presented into our own object
        throw ({"status": "error", "data": err});
    });
}).then(function () {
    // make the resolved value of the final promise be the roles object
    return roles;
});

The resolved value of this returned promise will be your roles object.

Summary of Changes:

1. Return the promise from Promise.each() rather than creating a new promise (no need to create a new promise here).
2. Let rejects propagate back up, no need to manually reject a higher level promise.
3. In the .then() handler for the Promise.each(), remove the declared parameter named roles since that conflicts with your higher scoped roles object.
4. Return roles in that .then() handler so it becomes the resolve value of the promise you are returning.
5. Remove the inner .catch() since the outer .catch() can do it's work also.
6. Change the .catch() to throw the repackaged error value.
7. Remove the inner return roles; since it is not needed. The roles object is constantly available at a higher scope so there is no need to make it a resolved value of these inner promises (in fact, that may just create confusion to do so).

---

Possible performance optimization. Since none of your results depend upon the prior results, it appears that maybe you could run all your async operations in parallel rather than in series in which case you could replace Promise.each() with Promise.map().

Answer 2

Yes, sorry, we changed our mind.

In 3.x we were supposed to make .each return the results rather than the original values but that broke the code for a lot of people so we changed it back and added a mapSeries.

Basically, use .mapSeries whenever you'd use each and care about the results, it does exactly what you'd expect. If you can run the items concurrently use .map though as it'll likely perform much better.