Thoughts on this conversion of code from Step.js to Q promise library?

Question

I have some code that I'd written using this pattern with Step.js. In this case, talking to MongoDB in Node:

Step(
    function connectToDatabaseWithAuthorization() {
        mongodb.connect(mongoConnectURI, this);
    },

    function getCommitCollection(err, conn) {
        if (err) handleResErr(res, err);

        conn.collection('commits', this);
    },

    function addCommitToCollection(err, coll) {
        if (err) handleResErr(res, err);

        var commit = JSON.parse(req.param('commit', null));
        commit.commit_date = requestTime;
        commit.commit_id = common.makeIdFromCommit(commit);
        coll.insert(commit, {safe: true}, this);
    },

    function respondWithShowAndVerifyUrlsInJson(err, records) {
        if (err) handleResErr(res, err);

        res.json({
            commit_date: records[0].commit_date
        });
    }
)

When I went to update my Node libraries, I noticed Step wasn't getting much attention or undergoing much development. And I wasn't happy about the error handling being spread into each function in the steps.

So I went to research modern alternatives, and Async.js seemed more popular. But when I asked about how to convert it, someone suggested I look into Q Promises instead.

I came up with this transformation, which seems to work:

Q.try(function() {

    return Q.ninvoke(mongodb, 'connect', mongoConnectURI);

}).then(function (conn) {

    return Q.ninvoke(conn, 'collection', 'commits');

}).then(function (coll) {

    var commit = JSON.parse(req.param('commit', null));
    commit.commit_date = requestTime;
    commit.commit_id = common.makeIdFromCommit(commit);
    return Q.ninvoke(coll, 'insert', commit, {safe: true});

}).then(function (records) {

    res.json({
        commit_date: records[0].commit_date
    });

}).catch(function (err) {

    handleResErr(res, err);

});

Not a large piece of code, but I could use a sanity check. I was having problems with figuring out if I should be using .done() in there somewhere, and also I couldn't get .finally to work. If I put it on the end of the chain, it ran after the first promise and not after all of them. :-/

While I mostly think the generality makes it an improvement, it does really obscure the method calls. Q.ninvoke(mongodb, 'connect' mongoConnectURI) is quite a bit uglier than mongodb.connect(mongoConnectURI). Have people made some better way to do mongodb calls with promises than this?

Answer 1

Interesting that you're getting .finally() after your first method rather than at the end. Should I read that to mean that your code executed as: connect-->finally-->[DONE] or did control flow somewhere else in the promise chain subsequent to finally?

Regarding the aesthetics - I couldn't agree with your more; ninvoke and its relatives are not the easiest to read. Two main options. One, you can look for a promise-enabled library for this or that. This is probably not my first choice unless the library would be giving you something more than just a promises wrapper. Two, you can get yourself some nicer looking functions by using the denodeify function. Consider if you wrote a module like this (you could of course inline this code as well which I often do for both initial development as well as if I find I really don't need more than a function or two):

var Q = require('q');
var lib = require('someNodeModule');

module.exports = {
   connect: Q.denodeify(lib.connect),
   read: Q.denodeify(lib.read),
   write: Q.denodeify(lib.write)
}

At this point, you have a new module with each of the above functions being promise-returning ones rather than callback-invoking ones. If you require the above as, say libp, you could then replace your various return Q.ninvoke(...) calls with much more clear calls such as a simple return libp.connect(...). The signature of the function stays the same (other than the obvious loss of the final callback).

I will try to return to this later this evening as well regarding the odd .finally() behavior. In the meantime, you might find this Q&A relevant to an issue you did not raise, but may encounter. More generally, the individual providing the answer, Bergi, appears to be quite expert with promises (from other answers I have seen), so you might check out some other answers or even solicit input on this. I am a journeyman, at best, with promises. I think I have a reasonable understanding of the concepts and library, but have not yet accumulated all the idioms that would be helpful.