Can't set headers after they are sent when logging a user in

Question

I'm scratching my head here, I'm using bcrypt to try and log in a user but I'm getting the following error:

Error: Can't set headers after they are sent.

Here's the code for that route:

router.post('/login', (req, res, next) => {
  User.find({ email: req.body.email })
    .exec()
    .then(user => {
      if (user.length < 1) {
        res.status(401).json({
          message: 'Auth failed'
        });
      }
      bcrypt.compare(req.body.password, user[0].password, (err, result) => {
        if (err) {
          res.status(401).json({
            message: 'Auth failed'
          });
        }
        if (result) {
          res.json(200).json({
            message: 'Auth successful'
          });
        }
        return res.status(401).json({
          message: 'Auth failed 3'
        });
      });
    })
    .catch(err => {
      res.status(500).json({
        error: err
      });
    });
});

I thought the error might've come from the if else statements and trying to send a header twice, but I'm ending them before moving on the next conditional no? Am I miss-reading something here?

Answer 1

The error you see is caused when your code tries to send more than one response for the same request. The typical cause for this error is a coding mistake in how you handle asynchronous operations. In your code you show, I can see the following mistakes that can cause this error:

1. If user.length < 1, then you do res.status(401).json(...), but then you let the code continue to run where you then send other responses.

2. If you get an error from bcrypt.compare(), you send an error response and then let the code continue to run and send other responses.

3. If bcrypt.compare() succeeds, you send res.json(200).json(...) which is just wrong. You probably meant res.status(200).json(...).

4. If bcrypt.compare() succeeds and you have a result, you send two responses.

In looking at your code, it appears that you think that as soon as you do res.json() that the function returns and no other code executes. That is not the case. Until you hit a return, the rest of the code in that function continues to execute.

Here's one way to fix it (adding a few return statements and one else):

router.post('/login', (req, res, next) => {
    User.find({ email: req.body.email }).exec().then(user => {
        if (user.length < 1) {
          res.status(401).json({message: 'Auth failed'});
          return;
        }
        bcrypt.compare(req.body.password, user[0].password, (err, result) => {
          if (err) {
            res.status(401).json({message: 'Auth failed'});
            return;
          }
          if (result) {
            res.json({message: 'Auth successful'});
          } else {
            res.status(401).json({message: 'Auth failed 3'});
          }
        });
    }).catch(err => {
        res.status(500).json({error: err});
    });
});

Or, to do this in a little cleaner fashion where all responses with the same status are combined and all flow control is done with promises, you can do something like this:

const util = require('util');
bcyrpt.compareAsync = util.promisify(bcrypt.compare);

router.post('/login', (req, res, next) => {
    User.find({ email: req.body.email }).exec().then(user => {
        if (user.length < 1) {
            throw new Error('No user match');
        }
        return bcrypt.compareAsync(req.body.password, user[0].password).then(result => 
            if (!result) {
                throw new Error('Auth failed 2');
            }
            res.json({message: 'Auth successful'});
          }
        }).catch(err => {
            res.json(401).json({message: err.message});
        });
    }).catch(err => {
        res.status(500).json({error: err});
    });
});