How I should use promises for db + http calls in node js

Question

I need to implement system which

Get data from parent collection.
Check if particular key found in redis
If no then do a http call and get json data then set cache
if yes then get data from cache
save data into child collection for parent id.

I have working solution using callback something like that.

MongoClient.connect(dsn).then(function(db) {
    parentcollection.findOne({"_id" : new ObjectId(pid)}, function(err, data) {

    var redis = require("redis"),
    client = redis.createClient();

    client.on("error", function (err) {
        console.log("Error " + err);
    });

    // If not set

    client.get(cacheKey, function(err, data) {
        // data is null if the key doesn't exist
        if(err || data === null) {
            var options = {
                host: HOST,
                port: 80,
                path: URI
            };

            var req = http.get(options, function(res) {
                res.setEncoding('utf8');
                res.on('data', function (chunk) {
                    body += chunk;
                    //console.log('CHUNK: ' + chunk);
                });
                res.on('end', function () {
                    data = JSON.parse(body);


                    // Get childdata After process of data

                    childcollection.save(childdata, {w:1}, function(cerr, inserted) {
                        db.close();

                    });
                });
            }); 
        } else {
            // Get childdata from cache data
            childcollection.save(childdata, {w:1}, function(cerr, inserted) {
                db.close();
            });
        }
    });
});

I want to use promise (native one, not external ones like bluebird / request ) instead of callbacks. I checkout manuals and thinking if I need to implement like that

var promise1 = new Promise((resolve, reject) => {

    MongoClient.connect(dsn).then(function(db) {
        parentcollection.findOne({"_id" : new ObjectId(pid)}, function(err, data) {


    });

}}.then(function(data){
   var promise2 = new Promise((resolve, reject) => {
     var redis = require("redis"),
        client = redis.createClient();

        client.on("error", function (err) {
            console.log("Error " + err);
        });

        // If not set

        client.get(cacheKey, function(err, data) {
            // data is null if the key doesn't exist
            if(err || data === null) {
                var options = {
                    host: HOST,
                    port: 80,
                    path: URI
                };

                var promise3 = new Promise((resolve, reject) => {
                        var req = http.get(options, function(res) {
                            res.setEncoding('utf8');
                            res.on('data', function (chunk) {
                                body += chunk;
                                //console.log('CHUNK: ' + chunk);
                            });
                            res.on('end', function () {
                                data = JSON.parse(body);


                            // Get childdata After process of data


                        });
                    })
                }).then(function(data){
                    childcollection.save(childdata, {w:1}, function(cerr, inserted) {
                                db.close();

                            });
                });
            } else {
                // Get childdata from cache data
                childcollection.save(childdata, {w:1}, function(cerr, inserted) {
                    db.close();
                });
            }
        });

}}.then(function(data){
});
});

Which look like as dirty as callback hell or any better approach which not used promises like above ?

Factor out the `new Promise` calls into extra promise-returning functions. — Bergi, Apr 19 '17 at 13:12

score 1 · Accepted Answer · answered Apr 19 '17 at 15:12

One issue is that you never call the resolve functions provided to the promise constructor callbacks. Without calling them, promises never resolve.

I would suggest creating those new promises in separate, reusable functions. On the other hand, some MongoDb methods return promises already when you don't provide the callback argument.

You could do it like below.

// Two promisifying functions:
function promiseClientData(client, key) {
    return new Promise(function (resolve, reject) {
        return client.get(key, function (err, data) {
            return err ? reject(err) : resolve(data); // fulfull the promise
        });
    });
}

function promiseHttpData(options) {
    return new Promise(function (resolve, reject) {
        return http.get(options, function(res) {
            var body = ''; // You need to initialise this...
            res.setEncoding('utf8');
            res.on('data', function (chunk) {
                body += chunk;
                //console.log('CHUNK: ' + chunk);
            });
            res.on('end', function () {
                data = JSON.parse(body);
                resolve(data);  // fulfull the promise
            });
        );
    });
}

// Declare the db variable outside of the promise chain to avoid 
// having to pass it through
var db;

// The actual promise chain:
MongoClient.connect(dsn).then(function (dbArg) {
    db = dbArg;
    return parentcollection.findOne({"_id" : new ObjectId(pid)}); // returns a promise
}).then(function (data) {
    var redis = require("redis"),
    client = redis.createClient();
    client.on("error", function (err) {
        console.log("Error " + err);
    });
    // Get somehow cacheKey...
    // ...
    return promiseClientData(client, cacheKey);
}).then(function (data) {
    // If not set: data is null if the key doesn't exist
    // Throwing an error will trigger the next `catch` callback
    if(data === null) throw "key does not exist"; 
    return data;
}).catch(function (err) {
    var options = {
        host: HOST,
        port: 80,
        path: URI
    };
    return promiseHttpData(options);
}).then(function (data) {
    // Get childdata by processing data (in either case)
    // ....
    // ....
    return childcollection.save(childdata, {w:1}); // returns a promise
}).then(function () {
    db.close();
});

I assume that the promises returned by MongoDb are fully compliant. In doubt, you can turn them into native JavaScript promises by calling Promise.resolve() on them, for instance like this:

return Promise.resolve(parentcollection.findOne({"_id" : new ObjectId(pid)}));

or:

return Promise.resolve(childcollection.save(childdata, {w:1}));

Thanks your example code help me lot to write code through promises, however I do not use throw/catch for conditional flow for redis, but use functional flow similar to http://stackoverflow.com/a/33260670/1230744 — kuldeep.kamboj, Apr 20 '17 at 13:07
Sure, that would work. Make sure to avoid code duplication though: `childcollection.save` should better occur only once. Also, if you don't use `catch` be aware that if an exception occurs, the chain is broken and the error cannot be trapped anymore. Throw/catch works really well in a promise chain. — trincot, Apr 20 '17 at 13:17
Yes I am using catch for all generic failure in promise chain, that is primary reason I hesitated for using catch for redis key miss which is normal event. — kuldeep.kamboj, Apr 20 '17 at 13:43
I see, it just seemed handy here, as you want to do the same thing in case of an error and in case of no key match (cf. `if(err || data === null)` in your original code). — trincot, Apr 20 '17 at 14:24
okay, I just copy & pasted code from redis sample examples, I did not realize that same line handle both fatal error & empty cache case. — kuldeep.kamboj, Apr 21 '17 at 04:32

How I should use promises for db + http calls in node js

1 Answers1