Ashish Mehra
Ashish Mehra

Reputation: 77

How to make it work in order?

I'm little bit confused here in this register route.As you can see I'm performing some validation checks before sending data to db.

First, I'm checking for empty fields in a form and after that I'm checking for username and email existence in records and In last password confirmation. All these thing are working fine but due to the async behaviour of DB query app crashed everytime.

/* POST Register User  */
router.post('/register',function(req,res,next){

let user = req.body;
//checking for empty field in a form
for(let key in user){
    if(user[key] === ""){
        return next(mid.error("All fields are required to fill"));
    }
}

User.findOne({username:user.username})
    .exec(function(err,user){
        if(err){
            return next(mid.error("Something went wrong"));
        }
        if(user){
            return next(mid.error("Username already exist"));
        }
});

User.findOne({email:user.email})
    .exec(function(err,user){
        if(err){
            return next(mid.error("Something went wrong"));
        }
        if(user){
            return next(mid.error("Email already exist"));
        }
});

//matching password 
if(user.password !== user.confirm){
    return next(mid.error("Password not matched.Try again !"));
}

//save data in object
let userData = {
    username : user.username,
    email    : user.email,
    password : user.password
};

//save data in database

User.create(userData,function(err,user){
    if(err){
        return next(mid.error("Something went wrong.Try again !!!"));
    } else {
        req.session.userID = user._id;
        return res.redirect('/home');
    } 

});

});

Is there is any way to get rid from this ?

Upvotes: 0

Views: 34

Answers (1)

Joschua Schneider
Joschua Schneider

Reputation: 4093

As you mentioned, the queries are Asynchronous. In this case, both your findOne calls are being processed because the findOne calls itself are Synchronous, leading to your error when one of the query completes first and executes the callback. Your callback then returns and passes everything on to the next middleware.

You sould put everything you want to happen after your first Query finished into the callback. In this case:

User.findOne({username:user.username})
.exec(function(err, foundUser){
    if(err){
        return next(mid.error("Something went wrong"));
    }
    if(foundUser){
        return next(mid.error("Username already exist"));
    }
    User.findOne({email:user.email})
     .exec(function(err,foundUser){
       if(err){
           return next(mid.error("Something went wrong"));
       }
       if(foundUser){
           return next(mid.error("Email already exist"));
       }
       //HERE GOES ALL OTHER CODE THAT SHOULD HAPPEN AFTER THE QUERIES
   });
});

Note that I renamed the user variable inside the callbacks so that it does not conflict with your user object used for the queries!

In the future, or for a more complicated usecase, I would recommend using Promises to avoid deeply nested callbacks like this.

Upvotes: 1

Related Questions