Reputation: 121
I'm a beginner trying to understand node.js. I tried to convert a callback into promise in my passport local strategy, after transforming into promise I'm having an error when I try to log in that says "user is not defined". I am sure I have done something not right in transforming my callback function, would you please have a look on my code and explain me where have i done wrong.
here's the previous callback
exports.newLocalStrategy= new localStrategy(
(username,password,done)=>{
User.find({username: username},(err, user)=>{
if (err) throw err;
if(user.length == 0){
console.log("Unknown User");
return done(null,false,{message: 'unknown User'});
}
comparePassword(password,user[0].password, (err,isMatch)=>{
if (err) throw err;
if (isMatch){
return done(null, user);
return res.send("Loggedin");
}else{
console.log('invalid password');
return done(null, false, {message:"Invalid password"});
}
})
});
});
const comparePassword = (candidatePassword, hash, callback)=>{
bcrypt.compare(candidatePassword, hash, (err, isMatch)=>{
if (err) return callback(err);
callback(null, isMatch);
});
}
and the code after I made it into promise:
exports.newLocalStrategy= new localStrategy(
(username,password,done)=>{
//promise
User.find({username: username})
.then(user =>{
if(user.length ==0){
return done(null, false,{message: 'unknown user'})
}
})
.catch(err => {return done(null,err)})
comparePassword(password,user[0].password)
.then(isMatch => {
if (isMatch) return (done,null);
})
.catch(err=>{return done(null,err)})
});
const comparePassword = (candidatePassword, hash, callback)=>{
return new Promise((resolve,reject)=>{
bcrypt.compare(candidatePassword, hash, (err, isMatch)=>{
if (err) return reject(err);
resolve(null, isMatch);
});
})
}
I am getting the error at comparePassword(password,user[0].password)
this line
Upvotes: 0
Views: 157
Reputation: 51936
There are two main issues here. Since it seems like you want to continue making the newLocalStrategy
accept a done
callback, I'll assume you don't want it to return a promise, but rather just use promises internally.
exports.newLocalStrategy = new localStrategy(
(username, password, done) => {
User.find({
username: username
})
.then(user => {
if (user.length == 0) {
return done(null, false, { message: 'unknown user' })
}
})
.catch(err => {
return done(null, err)
})
// this is in the wrong scope, `user` is not defined here
comparePassword(password, user[0].password)
.then(isMatch => {
if (isMatch) return (done, null);
})
.catch(err => {
return done(null, err)
})
});
const comparePassword = (candidatePassword, hash, callback) => {
return new Promise((resolve, reject) => {
bcrypt.compare(candidatePassword, hash, (err, isMatch) => {
if (err) return reject(err);
// resolve takes only one parameter
resolve(null, isMatch);
});
})
}
After making the corrections, it should look like this:
exports.newLocalStrategy = new localStrategy((username, password, done) => {
User.find({ username }).then(users => {
if (users.length === 0) {
throw new Error('unknown user');
} else {
return Promise.all([users, comparePassword(password, users[0].password)]);
}
}).then(([users, isMatch]) => {
if (isMatch) done(null, users);
else throw new Error('invalid password');
}).catch(err => {
done(null, false, err)
});
});
const comparePassword = (candidatePassword, hash) => {
return new Promise((resolve, reject) => {
bcrypt.compare(candidatePassword, hash, (err, isMatch) => {
if (err) reject(err);
else resolve(isMatch);
});
});
};
The most confusing part of this is probably the line
return Promise.all([users, comparePassword(password, users[0].password)]);
This could have been simplified to
return comparePassword(password, users[0].password);
if the users
didn't have to be passed to the next .then()
, since it's being passed to done()
if there's a match. Promise.all()
accepts an array of promises and resolves them before invoking the next .then()
callback. users
is not a promise but it is implicitly converted to one with Promise.resolve()
internally, and passed along with the value resolved from comparePassword()
.
For future reference, you could have defined comparePassword()
using util.promisify()
:
const comparePassword = require('util').promisify(bcrypt.compare);
Upvotes: 1