Search code examples
node.jsexpressasync.jskeystonejs

async.series() continues to execute next functions despite errors


I'm using the async.series() control flow from caolan's async module. U nlike the doc's explanation that it should excute all functions in sequence, one after the other and stop when one calls its callback with an error; mine actually calls the main callback if one of the functions gets an erro but then happily continues to execute the rest of the functions in sequence.

async.series([

    function (cb) {

        if (!req.body.join_firstname || !req.body.join_lastname || !req.body.join_email || !req.body.join_password) {
            req.flash('error', 'Please enter a name, email and password.');
            cb(true);
        }

        if (req.body.join_password !== req.body.join_passwordConfirm) {
            req.flash('error', 'Passwords must match.');
            cb(true);
        }

        if (req.body.join_email !== req.body.join_emailConfirm) {
            req.flash('error', 'Emails must match.');
            cb(true);
        }

        cb(null);

    },

    function (cb) {

        keystone.list('User').model.findOne({
            email: req.body.join_email
        }, function (err, user) {

            if (err || user) {
                req.flash('error', 'User already exists with that email address.');
                cb(true);
            }

            cb(null);

        });

    },

    function (cb) {

        var userData = {
            name: {
                first: req.body.join_firstname,
                last: req.body.join_lastname
            },
            email: req.body.join_email,
            password: req.body.join_password
        };

        var User = keystone.list('User').model,
            newUser = new User(userData);

        newUser.save(function (err) {
            if (err) {
                //if there's an error, don't send activation mail
                cb(err);
            } else {
                newUser.activationEmail(function (err) {
                    if (err) {
                        //if we can't send activation email,
                        //delete user from db to prevent re-registration failing because of non-unique email
                        keystone.list('User').model.findOne({
                            email: req.body.join_email
                        }).remove(function (err) {
                            req.flash('error', "Couldn't send an activation email. Contact support if this problem persists.");
                            cb(true);
                        });
                    } else {
                        cb(err);
                    }
                });
            }
        });
    }
], function (err) {
    if (err) return next();
    req.flash('success', "Hi, " + req.body.join_firstname + "! We've sent you an activation email. Please check your inbox and spam folder.");
    return res.redirect('/');
});

For example, when I purposely enter the wrong password confirm value, it throws an error, executes the callback and return next(); and then just continues, even saving the user in the db. Obviously that was not the intended result.

Anyone got an idea what I'm doing wrong here?


Solution

  • If you want execution of a current function to stop, it's not enough to call a callback. For instance:

    function(cb) {
    
        if (!req.body.join_firstname || !req.body.join_lastname || !req.body.join_email || !req.body.join_password) {
            req.flash('error', 'Please enter a name, email and password.');
            cb(true);  // will add this callback to the stack
        }
        // continuing here
        // ...
    }
    

    Either change your if-then-construct:

    function(cb) {
    
        if (!req.body.join_firstname || !req.body.join_lastname || !req.body.join_email || !req.body.join_password) {
            req.flash('error', 'Please enter a name, email and password.');
            cb(true);  // will add this callback to the stack
        } else if (req.body.join_password !== req.body.join_passwordConfirm) {
            //...
        }
        // no more code here
    }
    

    or return:

    function(cb) {
    
        if (!req.body.join_firstname || !req.body.join_lastname || !req.body.join_email || !req.body.join_password) {
            req.flash('error', 'Please enter a name, email and password.');
            return cb(true);  // will add this callback to the stack and return
        }
        // will only be executed if the if is false
        // ...
    }