2
\$\begingroup\$

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?

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

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.

\$\endgroup\$
3
  • \$\begingroup\$ Okay, it seems the .finally() issue is something with the console.log messages being out of chronological order in Nodejitsu's jitsu logs tail feature. That is certainly confusing for someone already out of their usual toolset already. :-/ Still confused about the .done()... some people use it, some don't. What's the point? Is it okay that my last .then clause returns nothing, and if so what makes that okay? \$\endgroup\$ Mar 10, 2014 at 3:10
  • \$\begingroup\$ Good old console.log() - it's great having such an easy logger always available, but it occasionally has me chasing non-errors because of the timing of messages (and the fact that it seems if I crash out on some code, the not-yet-emitted messages are lost). As for .done(), I tend not to use it and often see it omitted, but it appears I should, having read the API doc \$\endgroup\$ Mar 10, 2014 at 3:28
  • \$\begingroup\$ Things have been going along pretty well; I'd say the promise library was a good choice. Thanks for the help and the original suggestion! Feel free to test blackhighlighter, if it happens to be up and working. :-) \$\endgroup\$ Mar 17, 2014 at 2:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.