0

The then() method on promises returns a promise. A promise can be in one of three states, pending, fulfilled, or rejected. However when creating a promise you have a resolved and rejected method to call when you want your promise to be fulfilled or rejected. I can not find out how to use these methods on functions in the then() method. This has caused me problems when I am trying to use normal asynchronous functions with a callback in the middle of a then() chain. Take this code for example.

        User.findOne({
            where: {
                email: email
            }
        }).then(function(user){
            if(user){
                return done(null, false, req.flash('signupMessage', 'That email is already taken.'));
            }else{
                var user = User.build({ email: email });
                console.log("ONE");
                User.generateHash(password, function(err, hash){
                    console.log("TWO");
                    if(err){
                        return done(err);
                    }else{
                        user.password = hash;
                        var newUser = user.save();
                        return newUser;
                    }
                })
            }
        }, function(err){
            console.log("ERROR: ", err);
        }).then(function(newUser){
            console.log("THREE");
            return done(null, newUser);
        }, function(err){
            console.log("USER NOT CREATED: ", err);
        });

In this case User.findOne() returns a promise. Now the console for this would print ONE THREE TWO. The second then statement is called as soon as the asynchronous is called. Is there a way to not have the second then statement called until I notify it as resolved.

(I am aware this would be easy code to fix several different ways, I am more curious about why and when the then promise returned by the first statement becomes fulfilled.)

AC74
  • 3
  • 2
  • _"Is there a way to not have the second then statement called until I notify it as resolved"_ Which portion of `js` referencing ? , initial `.then()` ?, where `done()` called ? _"can not find out how to use these methods on functions in the then() method"_ The promise returned from initial call to `User.findOne` already fulfilled or rejected ; could create a new `Promise` inside of initial `.then()` – guest271314 Nov 07 '15 at 16:13

3 Answers3

2

So what happens is, findUser gets fulfilled (and not rejected), so the callback in the first argument to .then() invokes (and not the second). In the .then() clause, You console.log("ONE") and you don't return anything (because you fall into the else { clause, and you only return in the callback to createHash which doesn't mean much in the context of the promise.), so it actually returns undefined.

Then the next .then() invokes, and you get console.log("THREE").

Then sometime in the future, you get the callback from createHash invoked, and you get console.log("TWO").

The correct way would have been to write User.generateHash so that it too returns a promise, then you could chain it with the rest of the promise chain, and not have to deal with asynchronous stuff within your promise callback.

Madara's Ghost
  • 172,118
  • 50
  • 264
  • 308
  • Ok so the then() promise is fulfilled when function returns something. If nothing is returned then once its run all of its code its fulfilled. Thanks – AC74 Nov 07 '15 at 17:10
1

When creating a promise you have a resolved and rejected method to call when you want your promise to be fulfilled or rejected. I can not find out how to use these methods on functions in the then() method

When using the then method on a promise, you don't have access to the resolve/reject callbacks - then manages all this for you. It does return the new promise which will resolve with the result of your callback, that is, its return value. This return value can be a promise that will be assimilated automatically (when you resolve with a promise it will automatically fulfill/reject with the result of that promise).

So this is what you need to do here. When you want to call a callback method within the then callback, you will have to create a promise for it, which you then can return. The easiest and best way would be to promisify the callback function beforehand, but you can use the Promise constructor as well (where in you will have access to resolve/reject).

    User.findOne({
        where: {
            email: email
        }
    }).then(function(user){
        if (user){
            throw new Error('That email is already taken.');
        } else {
            var user = User.build({ email: email });
            console.log("ONE");
            var promise = new Promise(function(resolve, reject) {
//                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                User.generateHash(password, function(err, hash){
                if (err) reject(err);
                else     resolve(hash);
            });
            return promise.then(function(hash) {
                console.log("TWO");
                user.password = hash;
                var newUser = user.save();
                return newUser;
            });
        }
    }).then(function(newUser){
        console.log("THREE");
        return done(null, newUser);
    }, function(err){
        console.log("ERROR: ", err);
        return done(null, false, req.flash('signupMessage', err.message));
    });
Community
  • 1
  • 1
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • No nested `.then()`s please. Just return the promise. If you desperately need scope stuff and cannot use a library like `bluebird` to do it sanely, just `resolve({hash, user})` or something like that. – Madara's Ghost Nov 07 '15 at 16:45
  • @Madara Ah well, I guess you can trivially [unnest the `then`](http://stackoverflow.com/a/22000931/1048572) (more or less, maybe create the `user` elsewhere), but I wanted to keep the OP's code structure, and explicitly wanted to show that only the callback promisification should go into the promise constructor and everything else in a separate `then` callback. – Bergi Nov 07 '15 at 16:56
  • Your code has two Promise bad practices though (the nesting of `.then()`s and the use of a Promise constructor inside a `.then()`), I understand that you wanted to keep the code similar, but these *are* bad practices, and should be in example code. – Madara's Ghost Nov 07 '15 at 16:57
  • @MadaraUchiha: Nope. Nesting `then` might be a smell when it's not necessary, but not really a bad practise. And there's absolutely nothing wrong with having a `Promise` constructor inside a `then` - having it *around* a `then` is the [common antipattern](http://stackoverflow.com/q/23803743/1048572) – Bergi Nov 07 '15 at 17:02
0

if done() and User.generateHash() do not return promises, then there is no reason for the 2nd level of this program to wait before going onto the next then() block.

To fix this, or to see your print statements print out ONE TWO THREE, i would do this

User.findOne({
    where: {
        email: email
    }
}).then(function(user){
    var promise;
    if(user){
        promise = done(null, false, req.flash('signupMessage', 'That email is already taken.'));
    }else{
        var user = User.build({ email: email });
        console.log("ONE");
        promise = User.generateHash(password, function(err, hash){
            return new Promise(function(resolve, reject){
                console.log("TWO");
                if(err){
                    reject(done(err));
                }else{
                    user.password = hash;
                    var newUser = user.save();
                    resolve(newUser);
                }
            });
        });
    }

    return promise;
}, function(err){
    console.log("ERROR: ", err);
}).then(function(newUser){
    console.log("THREE");
    return done(null, newUser);
}, function(err){
    console.log("USER NOT CREATED: ", err);
});

This way the first then will wait for promise to be resolved before going to the next then block, this means User.generateHash will have to resolve before going to the next then block because the variable promise depends on it.