How to turn nested callback into promise?

Question

Recently I started using pg-promise with bluebird library. I have always been nesting callback and handling err in each callback. I find that the catch statement in promise looks really neat. I am not sure if it possible to turn this code to promise base?

username = username.toUpperCase();
let text = "SELECT * FROM users WHERE username = $1";
let values = [username];
database.one(text, values).then(function (userObject) {
  // ANY WAY TO TURN this nested bycrypt into promise chain?
  bcrypt.compare(password, userObject.password, function (err, same) {
    if (err) {
      return next(err, null);
    }
    if (!same) {
      return next(new Error("Password mismatched!"), null);
    }
    const serializeObject = {_id: userObject._id};
    return next(null, serializeObject);
  });
}).catch(function (err) {
  return next(err, null);
});

Cadency

bluebird has a promisify function which would promisify bcrypt.compare for you – Adulterant

@JaromandaX yea I was reading into that, so all I have to do is add "Async" keyword to the end of my function? – Cadency

yeah, I guess, sort of, once it's promisified - read the docs :p – Adulterant

I am not sure if it possible to turn this code to promise base any code can be turned into promises, and the promisify solution for your bcrypt module is the easiest one. – Caber

Answer 1

I imagine using bluebirds promisify, you would promisify bcrypt.compare like so (you don't HAVE to use the Async part of the name)

let compareAsync = Promise.promisify(bcrypt.compare);

because userObject in the first .then needs to be used in the second .then, you can't simply chain the .then's by returning compareAsync, because then the next .then wont have access to userObject

one fix, is to use a variable that will be in scope for both .then's (but ugh)

username = username.toUpperCase();
let text = "SELECT * FROM users WHERE username = $1";
let values = [username];
let uo; //hacky the outer scoped variable
database.one(text, values).then(function (userObject) {
    uo = userObject;
    return compareAsync(password, userObject.password);
}).then(function(same) {
    if (!same) {
        throw new Error("Password mismatched!");
    }
    const serializeObject = {_id: uo._id};
    return next(null, serializeObject);
}).catch(function (err) {
    return next(err, null);
});

another (in my opinion cleaner) option is a nested .then

username = username.toUpperCase();
let text = "SELECT * FROM users WHERE username = $1";
let values = [username];
database.one(text, values).then(function (userObject) {
    return compareAsync(password, userObject.password)
    // [optional] following three lines to generate a "nicer" error for compare failure
    .catch(function(err) {
        throw "bcrypt.compare failed";
    })
    // nested .then to pass on the userObject and same at the same time
    .then(function (same) {
        return { same: same, userObject: userObject };
    });
}).then(function (result) {
    let same = result.same,
        userObject = result.userObject;

    if (!same) {
        throw new Error("Password mismatched!");
    }
    let serializeObject = { _id: userObject._id };
    return next(null, serializeObject);
}).catch(function (err) {
    return next(err, null);
});

NOTE: bluebird has a promisifyAll function ... that promisifies functions in an object, and adds (by default) the Async postfix to the function name - I believe you can decide on a different postfix name, but the documentation will tell you more

when promisifying a single function, you declare the name yourself - the above could've easily been

let trumpIsBigly = Promise.promisify(bcrypt.compare);

then you would just use trumpIsBigly where the code has compareAsync

One last possibility

A hand rolled promisified compareAsync (lifted mostly from vitaly-t's answer but with additions)

function compareAsync(password1, password2, inValue) {
    return new Promise(function (resolve, reject) {
        bcrypt.compare(password1, password2, function (err, same) {
            err = err || (!same && new Error("Password mismatched!"));
            if (err) {
                reject(err);
            } else {
                resolve(inValue);
            }
        });

    });
}

Now compareAsync will resolve to the incoming value inValue only if there's no error, AND same is true

username = username.toUpperCase();
let text = "SELECT * FROM users WHERE username = $1";
let values = [username];
database.one(text, values).then(function (userObject) {
    return compareAsync(password, userObject.password, userObject)
}).then(function (userObject) {
    let serializeObject = { _id: userObject._id };
    return next(null, serializeObject);
}).catch(function (err) {
    return next(err, null);
});

Which makes the "chain" very simple!

Adulterant

truly answer most of my doubts! The promisify function seems really complicated. Sorry if these are absurd questions, but what if the library already have Async postfix in it? And how stable is this conversion really? Will it break for some function? – Cadency

The promisify function seems really complicated - really? pass in the function name ...get back a function that returns a promise ... how more simple do you need it? – Adulterant

sorry:P Really new to Promise. Another simple question, for .catch(function(err)) will the err belongs to the first err that occurs? I am currently reading up on promise to understand it more : ) – Cadency

yes (and no) ... in this code, that catch will be hit by the first error, if that is in the database.one, then none of the .then codes will run – Adulterant

The promisify doesn't say anything about passing in a function that is already promised-based. So before I promisify any function, I have to make sure that it is a normal callback function? By reading the source code etc. – Cadency

Bluebird's .promisify() works with any async function that uses the node.js async calling style where there's a callback as the last argument and that callback takes two parameters so it will be called like this callback(err, data). Yes, you have to make sure the function fits that criteria before using it with .promisify(). You shouldn't need to look at the function's code to determine this because it should be the same info that is needed just to use the function yourself so it should be in the doc or comments. – Ferris

@JaromandaX in your nested example, should I provide another catch for that nested promise? This way I can catch the compareAsync error? – Cadency

well, no unless you throw an error at the end of the catch, otherwise the last then will be executed, and you don't want that - the error should be processed by the last catch anyway – Adulterant

I've added a .catch for compareAsync - note WHERE I put it and that it throws so that neither of the following .then will get called – Adulterant

I have trouble visualizing this : ( As far as I know there are two chains? One is the main chain, another is a nested promise chain. The main chain has catch statement. How will the main chain get the error from compareAsync if they are part of different chain? – Cadency

yeah ... think of the nested chain as a promise, that resolves or rejects ... returning that in the "outer" then means the outer then will take on the result of the nested then, be it resolved or rejected ... – Adulterant

Just to clarify, So in the original example without the catch in the nested chain, can the outer catch get the error from compareAsync? right now my understanding is that it cant. – Cadency

as I said, the promise returned in the first .then will "take on: the result of the inner promise, if that is rejected (if compareAsync rejects) then the outer .then will reject with the rejection reason of compareAsync, which will be caught by the final .catch – Adulterant

Promise is so neat but it is so tricky when I have to depend on previous value! It is not "neat" haha – Cadency

there are other ways to handle it ... hang on, I'll add one more solution using a hand written promisified bcrypt.compare function :p – Adulterant

Let us continue this discussion in chat. – Cadency

Answer 2

This is to extend on @Jaromanda's answer, in case you use only that one function, and just want to see how to promisify it manually.

function samePassword(password1, password2) {
    return new Promise(function (resolve, reject) {
        bcrypt.compare(password1, password2, (err, same) => {
            err = err || (!same && new Error("Password mismatched!"));
            if (err) {
                reject(err);
            } else {
                resolve();
            }
        });

    });
}

db.one(text, values)
    .then(userObject => {
        return samePassword(password, userObject.password);
    })
    .catch(error => {
        return next(error, null);
    });

Other than that, the promisify approach is the way to go. But it is always good to understand what it effectively does ;)

Caber

Thank you this give me more insight into promisify. In one of your documentation github.com/vitaly-t/pg-promise/wiki/Common-Mistakes, you mention that we should avoid Promise.all and commented with ` // doesn't settle queries;` Can you provide more explanation on why it doesnt settle? Ultimately isnt query a promise function and should work with Promise.all? – Cadency

It is not about promise function not working, it is about Promise.all not guaranting to settle promises, as per its protocol, i.e. Promise.all only guarantees to settle all promises when the method itself resolves. – Caber

I see, is Promise.all and Promise chaining the same thing? I am going to read Promise in full detail but was hoping for some quick answer : ) – Cadency

Promise.all is a way to "wait" on multiple promises to resolve ... but if just one rejects, Promise.all will reject – Adulterant

@Caber - good job, I was considering showing how to promisify that function too, but my answer was already quite verbose :p – Adulterant