0

I have a function that look like this, for now at least it's working.

exports.changePassword = (req, res) => {
  const { token, password, confirmPassword } = req.body

  User.findOne({resetPasswordToken: token}, (err, user)=>{
    if(!user){
      return res.status(400).send({
        msg: 'Invalid token or token has been used!'
      })
    }

    const hash_password = bcrypt.hashSync(password, 10)
    User.findOneAndUpdate({_id: user._id}, 
      {hash_password}, 
      (err, result)=>{

        if(err){
          return res.status(400).send({
            msg: err
          })
        }

        User.findOneAndUpdate({_id: user._id},
          {resetPasswordToken: ''},
          (err, result)=>{
            if(err){
              return res.status(400).send({
                msg: err
              })
            }

            res.status(200).json({
              status: 1,
              data: 'Your password has been changed.'
            })
          }
        )
      })
  })
}

I just felt bad writing this block of code, because I think it has several problems:

  1. callback hell
  2. duplication of error handling code

For first problem maybe I can use done argument? and do some chaining? And also sometime I doubt I need to handle every single err callback. How would you rewrite above function to become more elegant?

3
  • You should consider using async.js Commented Jan 6, 2018 at 14:38
  • @shawn I use that sometime but that doesn't solve second problem. Commented Jan 6, 2018 at 15:16
  • Use promises!!! Commented Jan 6, 2018 at 15:59

2 Answers 2

2

You can use promises with Mongoose, which will help with your callback hell:

exports.changePassword = (req, res) => {
  const { token, password, confirmPassword } = req.body

  User.findOne({resetPasswordToken: token}).then((user)=>{
    // do stuff with user here 

    const hash_password = bcrypt.hashSync(password, 10)
    // Now chain the next promise by returning it
    return User.findOneAndUpdate({_id: user._id}, {hash_password});
  }).then((result)=>{
    // Now you have the result from the next promise, carry on...
    res.status(200).json({
      status: 1,
      data: 'Your password has been changed.'
    })
  }).catch(err => {
    // Handle any errors caught along the way
  });
}

Since these are promises, you can actually make this even neater by using the ES6 async/await syntax:

// Note this now has the async keyword to make it an async function
exports.changePassword = async (req, res) => {
  const { token, password, confirmPassword } = req.body

  try {
    // Here is the await keyword
    const user = await User.findOne({resetPasswordToken: token});

    // do stuff with user here 

    const hash_password = bcrypt.hashSync(password, 10)

    // Now the next promise can also be awaited
    const result = await User.findOneAndUpdate({_id: user._id}, {hash_password});

    // Finally send the status
    res.status(200).json({
      status: 1,
      data: 'Your password has been changed.'
    });
  } catch (err) {
    // Any promise rejections along the way will be caught here
  });
}
Sign up to request clarification or add additional context in comments.

1 Comment

great catch solved the duplicated error handling problem
0

To avoid this ugly Promise hell we have

ES2017 async/await syntax

You should change your whole code for something like this

exports.changePassword = async function (req, res){
    try {
        const { token, password, confirmPassword } = req.body

        var user = await User.findOne({resetPasswordToken: token}).exec()
        const hash_password = bcrypt.hashSync(password, 10)

        var result = await User.findOneAndUpdate({_id: user._id}, {hash_password}).exec()
        var result2 = await User.findOneAndUpdate({_id: user._id}, {resetPasswordToken: ''}).exec()

        res.status(200).json({status: 1, data: 'Your password has been changed.'})
    } catch (err) {
        res.status(400).send({msg: err }) //If some await reject, you catch it here
    }   
}

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.