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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.