node.js ~ constructing chained sequence of Promise resolves

Question

Can anyone suggest a better way to structure this use of Promises? I'm newish to Promises and am wondering if I'm missing something about how to construct this a chain of events.

Note: It IS my intention to NOT use the rej[ect] here. What you see guatanrees only the res[olve] returns. Meaning the code this returns to only needs one path with which to handle the returned value. Thereby the code returned to is simplier in it's flow.

It might help to know, if you don't recognize it, this is taken from a module I created. Think of it as a Dao.

module.exports = {

    dbConnection: function () {
        return { user: 'sa', password: 'mypassword', server: 'localhost', database: 'mydb' };
    },


    CanIConnectToTheDB: function () {
        return new Promise(function (res, rej) {
            var sql = require('mssql');
            var myDao = require('./myDao');
            var cn = new sql.ConnectionPool(myDao.dbConnection());

            cn.connect().then(function () {
                var req = new sql.Request(cn);
                var qry = 'select serverproperty(\'productversion\') as \'rs\'';
                req.query(qry)
                    .then(function (rs) {
                        qry = 'select isnull(object_id(\'SomeObjectIKnowExists\'), -1)';
                        req.query(qry)
                            .then(function (rss) {
                                res(' CONNECTED// MASTER DB SUCCESS// MY DB SUCCESS');
                            })
                            .catch(function (err) {
                                res(' CONNECTED// MASTER DB SUCCESS// ISSUE QUERYING MY DB //' + err + '//');
                            });
                    })
                    .catch(function (er) {
                        res(' CONNECTED// COULD NOT QUERY MASTER DB //' + er + '//');
                    });
            })
            .catch(function () {
                res(' CAN NOT CONNECT');
            });
        });
    }
};

Having a failure make your promise resolve successfully seems to defeat much of the point of promises. Nesting promises also defeats the point. — lonesomeday, Apr 05 '17 at 17:41
Like I said. I'm new-ish. can you demonstrate better? Although I do insist only ONE return value is received by the caller. This one return incorporate both successes and failures — Steve, Apr 05 '17 at 17:43
There's nothing wrong with catching errors in order to provide readable error messages. It's actually good practice to do so when error messages are to be displayed to users. But it's better to allow errors to continue down the error path (by rethrowing), and only success down the success path, and to return an "uncaught" promise. This standard strategy leaves an async function's caller(s) free to choose how success and failure are to be treated. — Roamer-1888, Apr 06 '17 at 00:44
@Roamer-1888 I totally get that. It is just in this case for me it is as you stated. I just want this thing to report string. The string can say what the string has to say. And the use can choose what they want to do with what the read. :-) So I guess in a way I'm even re-throwing the exception up to a layer even more superior than that of the code of the application. — Steve, Apr 06 '17 at 02:02
I'm just trying to encourage you, more than the selected answer, that you can achieve your objective of composing a report of progress through the promise chain, and still have separate success and error paths. Remember that Errors have a ` message` property. — Roamer-1888, Apr 06 '17 at 09:02

score 4 · Answer 1 · edited May 23 '17 at 12:09

Instead of something like this:

                .then(function (rs) {
                    qry = '...';
                    req.query(qry)
                        .then(function (rss) {

you can use something like this:

                .then(function (rs) {
                    qry = '...';
                    return req.query(qry);
                }).then(function (rss) {

I.e. you can return a promise in one then callback and get the resolved value of that promise in the next then callback, so your indentation keeps constant.

Simpler example - instead of this:

a().then(va => {
    b(va).then(vb => {
        c(vb).then(vc => {
            // you can use vc here
        });
    });
});

you can do:

a().then(va => {
    return b(va);
}).then(vb => {
    return c(vb);
}).then(vc => {
    // you can use vc here
});

Or, even simpler if you use async and await:

va = await a();
vb = await b(va);
vc = await c(vb);
// you can use vc here

Note that you can only use await inside of a function created with the async keyword. In places where you don't have native support for async and await you can use Babel or with a slightly different syntax a generator based approach like in co or Bluebird coroutines. For more info and support in browsers and Node, see this answer:

Is there a better way to run CLI commands with Node.js?

Update

This is not tested, but this is more or less how I would write it:

module.exports = {

    dbConnection: function () {
        return { user: 'sa', password: 'mypassword', server: 'localhost', database: 'mydb' };
    },

    CanIConnectToTheDB: function () {
        var sql = require('mssql');
        var myDao = require('./myDao');
        var cn = new sql.ConnectionPool(myDao.dbConnection());
        var req;

        return cn.connect()
        .catch(err => Promise.reject('Error 1: ' + err))
        .then(() => {
            req = new sql.Request(cn);
            var qry = 'select serverproperty(\'productversion\') as \'rs\'';
            return req.query(qry)
                .catch(err => Promise.reject('Error 2: ' + err));
        }).then(rs => {
            var qry = 'select isnull(object_id(\'SomeObjectIKnowExists\'), -1)';
            return req.query(qry)
                .catch(err => Promise.reject('Error 3: ' + err));
        }).then(function (rss) {
            return 'CONNECTED// MASTER DB SUCCESS// MY DB SUCCESS';
        }).catch(err => {
            // if you want it always resolved:
            return 'CAN NOT CONNECT: ' + err;
        });
    }
};

Of course I would keep the final promise returned by that function to be rejected on errors and resolved only on success, but since you explicitly included that strange requirement in your question then I wrote it how you wanted.

But if it rejected the promise on any error then it would be much easier to use, especially if all you care is the answer to the question in the name of the function - Can I Connect To The DB:

CanIConnectToTheDB()
    .then(() => console.log("Yes I can"))
    .catch(() => console.log("No I can't"));

More info:

For more info see those answers:

How to convert Waterfall method to promise
jQuery: Return data after ajax call success (see explanation in updates to this answer)

I guess the use of return in `return req.query(qry);` what I haven't picked up on yet. I'll play with that — Steve, Apr 05 '17 at 17:44
This is the answer I was expecting. But in this construction I don't see how to fit in the three _different_ queries. can you elaborate? — Steve, Apr 05 '17 at 17:48
I also wanted to make it more vertical, but try applying that pattern to the OP's actual code, and it gets tricky. I'd like to see your version. — T.J. Crowder, Apr 05 '17 at 18:00
@T.J.Crowder Challenge taken ;) See my updated answer. This is still a mess but this is how I would do it if I wanted to always get a resolved promise out of the function but with different error messages for errors on different stages. — rsp, Apr 05 '17 at 18:21
You'll get a `ReferenceError` using the code you've added: `req` is out of scope where you're doing the second query. Which is why I didn't unnest that part. :-) — T.J. Crowder, Apr 05 '17 at 18:21
@T.J.Crowder Good point. Now, this is why I would use async/await (or some generator based coroutines) because that way I would keep everything resolved in the past in scope. Here I would have to either pass the req along along the way, make a new one, or move the variable declaration to the outer scope - which I think I'm going to do here because it will be simplest. — rsp, Apr 05 '17 at 18:24
Small issue I know but just to be clear. `Error 1` is `CAN NOT CONNECT`, correct? — Steve, Apr 05 '17 at 18:30
@rsp Rafal. Thanks for the hard work. Well done. But I am enjoying T.J.'s `returns` bubbling up more for it's clarity. But I am learning from you example 1) how the flattening of `.then()s ` and 2) how I can nest implementing a returned a caught `Promise.reject()`. All in all though I'm having a hard time clearly seeing it. — Steve, Apr 05 '17 at 18:40

T.J. Crowder · Accepted Answer · 2021-03-11T09:57:05.257

The key thing to remember about promises is that then returns a new promise (as does catch). How that new promise is settled depends on what you return from the handler: If you return a promise (or more generally, a thenable), the new promise from then/catch is resolved to the one that you returned (it will be fulfilled or rejected based on what that promise does); if you return a value, the new promise is fulfilled with that value. (FWIW, I go into promise terminology — "fulfill" vs. "resolve," etc. — in this post on my blog.)

So you can chain them together. Think of the then and catch handlers as transforming filters the ultimate result flows through.

Note also that if you have a starting point that gives you a promise (cn.connect()), you don't need new Promise: Just use then and catch to transform what passes through the chain by returning the (new) resolution value.

Another key thing to remember is that if a catch handler returns a value, it converts a rejection into a resolution. To continue down the rejection path, a catch handler must either throw an exception or return a promise that is/will be rejected.

Finally: require calls should always be at the beginning of the module.

So, without removing your conversion of rejections to resolutions (more on that in a moment):

var sql = require('mssql');
var myDao = require('./myDao');

module.exports = {

    dbConnection: function () {
        return { user: 'sa', password: 'mypassword', server: 'localhost', database: 'mydb' };
    },


    CanIConnectToTheDB: function () {
        var cn = new sql.ConnectionPool(myDao.dbConnection());
        return cn.connect()
            .then(function () {
                var req = new sql.Request(cn);
                var qry = 'select serverproperty(\'productversion\') as \'rs\'';
                return req.query(qry)
                    .then(function (rs) {
                        qry = 'select isnull(object_id(\'SomeObjectIKnowExists\'), -1)';
                        return req.query(qry)
                            .then(function (rss) { // Note you're not using rss anywhere
                                return ' CONNECTED// MASTER DB SUCCESS// MY DB SUCCESS';
                            })
                            .catch(function (err) {
                                return ' CONNECTED// MASTER DB SUCCESS// ISSUE QUERYING MY DB //' + err + '//';
                            });
                    })
                    .catch(function (er) {
                        return ' CONNECTED// COULD NOT QUERY MASTER DB //' + er + '//';
                    });
            })
            .catch(function() {
                return ' CAN NOT CONNECT';
            });
    }
};

Note: It IS my intention to NOT use the rej[ect] here. What you see guatanrees only the res[olve] returns. Meaning the code this returns to only needs one path with which to handle the returned value. Thereby the code returned to is simplier in it's flow.

Rejections follow a separate path from resolutions for a reason. It doesn't make things more complicated, it makes things simpler. Don't convert rejections into resolutions unless you're done an error-recovery of some kind and can continue on as though the rejection hadn't happened.

Here's that code where rejections are allowed to be rejections:

var sql = require('mssql');
var myDao = require('./myDao');

module.exports = {

    dbConnection: function () {
        return { user: 'sa', password: 'mypassword', server: 'localhost', database: 'mydb' };
    },

    CanIConnectToTheDB: function () {
        var cn = new sql.ConnectionPool(myDao.dbConnection());
        return cn.connect()
            .then(function () {
                var req = new sql.Request(cn);
                var qry = 'select serverproperty(\'productversion\') as \'rs\'';
                return req.query(qry)
                    .then(function (rs) {
                        qry = 'select isnull(object_id(\'SomeObjectIKnowExists\'), -1)';
                        return req.query(qry)
                            .then(function (rss) { // Note you're not using rss anywhere
                                return ' CONNECTED// MASTER DB SUCCESS// MY DB SUCCESS';
                            });
                    });
            });
    }
};

Using it:

theModule.CanIConnectToTheDB()
    .then(function() {
        // Yes, let's do something
    })
    .catch(function() {
        // No, report the problem, etc.
    });

I'd also probably abstract out the bit I assume you'll end up doing over and over: Establishing a connection and getting a request object from it:

var sql = require('mssql');
var myDao = require('./myDao');

function getRequest() {
    var cn = new sql.ConnectionPool(myDao.dbConnection());
    return cn.connect().then(function() {
        return new sql.Request(cn);
    });
}

module.exports = {

    dbConnection: function () {
        return { user: 'sa', password: 'mypassword', server: 'localhost', database: 'mydb' };
    },

    CanIConnectToTheDB: function () {
        return getRequest().then(function(req) {
            var qry = 'select serverproperty(\'productversion\') as \'rs\'';
            return req.query(qry)
                .then(function (rs) {
                    qry = 'select isnull(object_id(\'SomeObjectIKnowExists\'), -1)';
                    return req.query(qry)
                        .then(function (rss) { // Note you're not using rss anywhere
                            return ' CONNECTED// MASTER DB SUCCESS// MY DB SUCCESS';
                        });
                });
        });
    }
};

but "report the problem" is exactly the same, error or no-error. the return string gets rendered the web page. Since .then() & .catch() do the same thing with the return I don't see this as being DRY. But I'll play with it some. Thanks. 528k rep! dang. — Steve, Apr 05 '17 at 17:53
i like removing the catches form the module. But do you see how that is mostly just _moving_ code from the back to the front? — Steve, Apr 05 '17 at 17:54
Would you say by converting the error to a report, the same report as a success, is what I was doing in terms of `convert rejections into resolutions for my error-recovery` But you are correct. Else where I would not disband the reject as I am. — Steve, Apr 05 '17 at 17:57
On the requires() ~ oddly, and I couldn't figure this out, `CanIConnectToTheDB()` couldn't see `dbConnection` unless the requires was inside `CanIConnectToTheDB()`. No idea why not. — Steve, Apr 05 '17 at 17:59
@user2367083: I'm saying best practice is let the caller handle rejections, just like best practice for exception handling is let the caller handle exceptions. Not only does it give you simpler code, it means the consumer isn't surprised by getting a promise that resolves instead of rejecting when an error occurs, which defeats the expectation and contract of promises. Now, sometimes to try to protect our abstractions (although abstractions *always* leak), we may convert one rejection into another, but converting a rejection to a resolution is not a good idea. — T.J. Crowder, Apr 05 '17 at 17:59
I'm seeing how those `returns` bubble up. That's stylish enough. — Steve, Apr 05 '17 at 18:06
@user2367083: Yeah, it's really powerful. I find the pipeline/transform metaphor a good way to think of it. — T.J. Crowder, Apr 05 '17 at 18:07
I'm going to guess the returns marginally perform better than the resolves. Or perhaps give or take given the number of obejcts at play or the type of item being returned. e.g. a returned complex object might perform better with resolves but my simple string might perform better with `return`. But then the difference might be _very_ marginal — Steve, Apr 05 '17 at 18:13

node.js ~ constructing chained sequence of Promise resolves

2 Answers2

Update

More info:

Linked