Handling multiple nested async operations in Bluebird (promises)

Question

I'm fairly new to JavaScript (Node.js) and to Promises. I'm currently working with AWS Lambda and DynamoDB.

I have a list of items which I fetch from the database asynchronously (have promisified the AWS SDK with bluebird promises API.)

For each of these items, I need to possibly retrieve multiple child items (also asynchronously), and then for each of the child item I have to do another async operation and determine if this async operation succeeded or not.

After all the async operations are done for an item (i.e. all async operations for the child items either succeeds or fails), I need to update the status of the item in the DB (fail/success.)

This (below) is what I have so far. Can you please tell me if I have done this correctly? Are there any logical mistakes in it? Can it be improved?

var doc = require('dynamodb-doc');
var promise = require("bluebird");

var dynamodb = new doc.DynamoDB();

var params = { /* ... */ };

promise.promisifyAll(Object.getPrototypeOf(dynamodb));

dynamodb.queryAsync(params)
    .then(function(data) {
        var items = data.Items;

        var promises = items.map(function(item) {
            params = { /* ...*/ };

            return dynamodb.queryAsync(params)
                .then(function(data2) {
                    var childItems = data2.Items;

                    var morePromises = childItems.map(function(device) {
                        return doAsyncWork()
                            .then(function success() {
                                console.log("Success!");
                            })
                            .catch(function error(err) {
                                console.log("Error!");
                            })
                    });

                    return promise.all(morePromises);
                })
                .then(function() {
                    // Update status for item in DB
                    params = { /* ...*/ };
                    return dynamodb.updateItemAsync(params);
                });
        });

        return promise.all(promises);
    })
    .then(function() {
        var response = { /* ... */ };

        context.succeed(response);
    })
    .catch(function(err) {
        context.fail(err);
    });

Some other things:

For all async operations of an item to complete I'm using Promise.all(), and from the documentation I can see that if even one promise got rejected the subsequent promises will get rejected as well. I don't want this to happen, I want it to continue even if a single promise is rejected.

Likewise for all the items I'm using Promise.all() in the end to wait for all the items to complete their processing. If one failed, the rest won't be processed, right? How can I overcome this?

There is a lot of nesting, how may I improve upon this code?

I need to have a way of consolidating all the results and pass this as a response e.g. something like this:

{
    "Results": [{
        "ItemId": " ... ",
        "Status": "Success",
        "ChildItems": [{
            "ChildItemId": " ... ",
            "Status": "Success"
                /*  ...
                ...
                ...
            */
        }]
    }, {
        "ItemId": " ... ",
        "Status": "Error",
        "ChildItems": [{
            "ChildItemId": " ... ",
            "Status": "Success"
        }, {
            "ChildItemId": " ... ",
            "Status": "Error"
        }]
    }]
}

One solution (may be kind of ugly) that comes to my mind is to have a global object outside and then store in the results within it. Is there any other elegant way to do this?

Thanks.

Answer 1

Can it be improved?

Some things you can improve since you are using Bluebird.

Use Bluebird's promise.map() to save code:

Instead of array.map() followed by promise.all(), you can just use Bluebird's promise.map() by changing code like this:

               var childItems = data2.Items;

                var morePromises = childItems.map(function(device) {
                    return doAsyncWork()
                        .then(function success() {
                            console.log("Success!");
                        })
                        .catch(function error(err) {
                            console.log("Error!");
                        })
                });

                return promise.all(morePromises);

to this:

               return promise.map(data2.Items, function(item) {
                    // do something with item
                    return doAsyncWork();
               });

Be careful about .catch() handlers that just log.

If you handle a promise rejection and don't rethrow or return a rejected promise, the promise state will change from rejected to fulfilled.

So, when you use a .catch() like you have in here:

        return dynamodb.queryAsync(params)
            .then(function(data2) {
                var childItems = data2.Items;

                var morePromises = childItems.map(function(device) {
                    return doAsyncWork()
                        .then(function success() {
                            console.log("Success!");
                        })
                        .catch(function error(err) {
                            console.log("Error!");
                        })
                });

                return promise.all(morePromises);
            })

That will "eat" any error that comes from a rejected promise from doAsyncWork(). Occasionally that's what you want (you want to handle the error and continue as if nothing went wrong), but many times you need to error to get propagated back somehow. You can log it, but let the error propagate by rethrowing it:

        return dynamodb.queryAsync(params)
            .then(function(data2) {
                var childItems = data2.Items;

                var morePromises = childItems.map(function(device) {
                    return doAsyncWork()
                        .then(function success() {
                            console.log("Success!");
                        })
                        .catch(function error(err) {
                            console.log("doAsyncWork Error: ", err);
                            //rethrow so error propagates
                            throw err;
                        })
                });

                return promise.all(morePromises);
            })

For all async operations of an item to complete I'm using Promise.all(), and from the documentation I can see that if even one promise got rejected the subsequent promises will get rejected as well. I don't want this to happen, I want it to continue even if a single promise is rejected.

In Bluebird, if you don't want a Promise.all() to abort when one promise rejects, you can use Promise.settle() instead of Promise.all() if you're using Bluebird 2.x. If you're using Bluebird 3.x, then you use .reflect() when you're returning your promises. How to do this is explained here in the Bluebirds docs. Personally, I liked the way Promise.settle() works, but there must have been some standards-direction reason for changing it.

There is a lot of nesting, how may I improve upon this code?

You can chain some of what you're doing instead of nesting. See How to chain and share prior results with Promises for a variety of ways to sequence multiple operations without so much nesting and accumulate results.