2

I keep getting "Can't set headers after they are sent" building a Node/Express API.

The issue is I am not setting the headers after the response has been sent anywhere. I am always calling res.status(xxx).json({}) to close ever condition.

Route

const router = require('express').Router();
router.get('/password/validate/:hash', PasswordController.validate);
router.post('/password/update', PasswordController.update);

Controller

This is where the error is occurring. I am calling the validate request specifically.

// Import node packages
const mongoose = require('mongoose');
const Password = require('../models/password');
const User = require('../models/user');
const bcrypt = require('bcryptjs');
const moment = require('moment');
const string = require('../middleware/string_functions')

exports.update = (req, res, next) => {
    User.findOne({ email: req.body.email })
        .exec()
        .then(user => {
            if (!user) {
                res.status(401).json({
                    message: 'Cannot retrieve account'
                })
            }
            const expiry = moment().add(30, 'seconds');
            const unique_string = string.generate_random(32);
            const url_hash = string.base64_encode(unique_string +':'+ user._id);
            bcrypt.hash(unique_string, 10, (err, hash) => {
                if (err) {
                    res.status(500).json({ 
                        error: err.message
                    })
                }
                const query = { user_id: user._id }
                const newData = {
                    hash,
                    expiry
                }
                Password.findOneAndUpdate(query, newData, { upsert: true, new: true })
                    .exec()
                    .then(request => {
                        res.status(201).json({
                            message: 'success',
                            url: 'localhost:8081/users/password/validate/' + url_hash,
                            data: request
                        })
                    })
                    .catch(err => {
                        res.status(500).json({
                            error: err.message
                        })
                    })
            })
        })
        .catch(err => {
            res.status(500).json({
                error: err.message
            })
        })
}

exports.validate = (req, res, next) => {
    if (!req.params.hash) {
        res.status(500).json({
            error: 'Missing hash'
        })
    }
    const data = string.base64_decode(req.params.hash).split(':');
    console.log(data)
    Password.findOne({ user_id: data[1] })
        .exec()
        .then(request => {
            if (!request) {
                res.status(404).json({
                    message: 'Change request not found or expired'
                })
            }
            bcrypt.compare( data[0], request.hash, (err, result) => {
                if (err) {
                    res.status(500).json({
                        error: err.message
                    })
                }
                if (result) {
                    if (moment().isAfter(request.expiry)) {
                        res.status(401).json({
                            message: 'Time has expired'
                        })
                    }
                    res.status(200).json({
                        message: 'Hash validation successful'
                    })
                }
                res.status(500).json({
                    error: 'Something went wrong'
                })
            })
        })
        .catch(err => {
            res.status(500).json({
                error: err.message
            })
        })
}

Console Error

_http_outgoing.js:494
    throw new Error('Can\'t set headers after they are sent.');
    ^

Error: Can't set headers after they are sent.
    at validateHeader (_http_outgoing.js:494:11)
    at ServerResponse.setHeader (_http_outgoing.js:501:3)
    at ServerResponse.header (/Users/chrislloyd/Development/Projects/happy-hour-api/node_modules/express/lib/response.js:767:10)
    at ServerResponse.send (/Users/chrislloyd/Development/Projects/happy-hour-api/node_modules/express/lib/response.js:170:12)
    at ServerResponse.json (/Users/chrislloyd/Development/Projects/happy-hour-api/node_modules/express/lib/response.js:267:15)
    at bcrypt.compare (/Users/chrislloyd/Development/Projects/happy-hour-api/api/controllers/passwords.js:83:22)
    at /Users/chrislloyd/Development/Projects/happy-hour-api/node_modules/bcryptjs/dist/bcrypt.js:297:21
    at /Users/chrislloyd/Development/Projects/happy-hour-api/node_modules/bcryptjs/dist/bcrypt.js:1353:21
    at Immediate.next [as _onImmediate] (/Users/chrislloyd/Development/Projects/happy-hour-api/node_modules/bcryptjs/dist/bcrypt.js:1233:21)
    at runCallback (timers.js:789:20)
    at tryOnImmediate (timers.js:751:5)
    at processImmediate [as _immediateCallback] (timers.js:722:5)

Updated Example

exports.update = (req, res, next) => {
    // Check if hash value exists
    if (!req.params.hash) {
        res.status(500).json({
            error: 'Missing hash value'
        });
        return;
    }
    // Check if password and confirmation are the same
    if (req.body.password != req.body.passwordConfirmation) {
        res.status(401).json({
            message: 'Password confirmation does not match'
        });
        return;
    }
    // Decode and split hash and user id into array
    const data = string.base64_decode(req.params.hash).split(':');
    // Find record that contains user id
    Password.findOne({ user_id: data[1] })
        .exec()
        .then(request => {
            console.log(request)
            // Throw 404 error if record is not found
            if (!request) {
                return res.status(404).json({
                    message: 'Password change request doest not exist or timed out'
                });
            }
            // Check if change request has expired
            if (moment().isAfter(request.expiry)) {
                res.status(401).json({
                    message: 'Password change request expired',
                    request: {
                        request: 'http://localhost:3001/users/password/request'
                    }
                });
                // Delete expired record
                Password.remove({ _id: request._id })
                    .exec()
                    .catch(err => {
                        res.status(500).json({
                            error: err.message
                        });
                    });
                return;
            }
            // Compare hash value from encoded string to encrypted hash value in database
            console.log(mongoose.Types.ObjectId(request.user_id))
            bcrypt.compare( data[0], request.hash, (err, result) => {
                // Bcrypt error performing comparison
                if (err) {
                    res.status(500).json({
                        error: err.message
                    });
                    return;
                }
                // Check if result is true
                if (result) {
                    // Find user record matching request.user_id and update password
                    User.findOneAndUpdate({ _id: mongoose.Types.ObjectId(request.user_id) }, {$set: { password: req.body.password  }}, {new: true}, (err, user) => {
                        console.log(user)
                        // Error finding and updating user record
                        if (err) {
                            res.status(500).json({
                                error: err.message
                            });
                            return;
                        }
                        // If returned user account is not null
                        if (user) {
                            res.status(200).json({
                                message: 'Password updated',
                                user
                            });
                            return;
                        }
                        // Could not find user record
                        res.status(404).json({
                            message: 'Could not find user account to update'
                        });
                        return;
                    })
                }
                // Catch all error
                res.status(500).json({
                    error: 'Something went wrong'
                });
                return;
            })
        })
        .catch(err => {
            res.status(500).json({
                error: err.message
            });
            return;
        });
}
Just A Guy
  • 149
  • 5
  • 18
  • the error is after `bcrypt.compare` in validate function... if `error` if `result` and then again `res.json`!!! – Ashh May 16 '18 at 17:46

2 Answers2

7

That particular error is caused when you send multiple responses to the same request.

You see to be thinking that as soon as you do res.status(...).json(...) that your function returns and stops executing. It does not. res.json() is just a regular function call. It doesn't change the control flow in your function at all (unless it throws an exception). A successful call to res.json() executes and then your function just keeps right on executing the lines of code that follow.

What you need is a return statement after each time you send a response (if there is any other code in your function that could execute and send another response) so that your function doesn't continue to execute and send another response or you could bracket your responses in if/else statements so you don't execute the sending of more than one response.

Here's a fixed version with 5 added return statements to keep the rest of your code from executing after you've sent a response and to keep you from sending multiple responses to the same request. Each addition is commented with ==> added:

// Import node packages
const mongoose = require('mongoose');
const Password = require('../models/password');
const User = require('../models/user');
const bcrypt = require('bcryptjs');
const moment = require('moment');
const string = require('../middleware/string_functions')

exports.update = (req, res, next) => {
    User.findOne({ email: req.body.email })
        .exec()
        .then(user => {
            if (!user) {
                res.status(401).json({
                    message: 'Cannot retrieve account'
                })
                return;            // <== added
            }
            const expiry = moment().add(30, 'seconds');
            const unique_string = string.generate_random(32);
            const url_hash = string.base64_encode(unique_string +':'+ user._id);
            bcrypt.hash(unique_string, 10, (err, hash) => {
                if (err) {
                    res.status(500).json({ 
                        error: err.message
                    })
                    return;            // <== added
                }
                const query = { user_id: user._id }
                const newData = {
                    hash,
                    expiry
                }
                Password.findOneAndUpdate(query, newData, { upsert: true, new: true })
                    .exec()
                    .then(request => {
                        res.status(201).json({
                            message: 'success',
                            url: 'localhost:8081/users/password/validate/' + url_hash,
                            data: request
                        })
                    })
                    .catch(err => {
                        res.status(500).json({
                            error: err.message
                        })
                    })
            })
        })
        .catch(err => {
            res.status(500).json({
                error: err.message
            })
        })
}

exports.validate = (req, res, next) => {
    if (!req.params.hash) {
        res.status(500).json({
            error: 'Missing hash'
        })
    }
    const data = string.base64_decode(req.params.hash).split(':');
    console.log(data)
    Password.findOne({ user_id: data[1] })
        .exec()
        .then(request => {
            if (!request) {
                res.status(404).json({
                    message: 'Change request not found or expired'
                })
                return;            // <== added
            }
            bcrypt.compare( data[0], request.hash, (err, result) => {
                if (err) {
                    res.status(500).json({
                        error: err.message
                    })
                    return;            // <== added
                }
                if (result) {
                    if (moment().isAfter(request.expiry)) {
                        res.status(401).json({
                            message: 'Time has expired'
                        })
                    }
                    res.status(200).json({
                        message: 'Hash validation successful'
                    })
                    return;            // <== added
                }
                res.status(500).json({
                    error: 'Something went wrong'
                })
            })
        })
        .catch(err => {
            res.status(500).json({
                error: err.message
            })
        })
}
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • Actually `res.json` is the return statement for route... did not return the code execution? – Ashh May 16 '18 at 17:57
  • @AshishChoudhary - `res.json()` is JUST a regular function call. It doesn't cause your function to return or stop executing any more than any other function call such as `console.log("hi")`. It does cause the response to be sent, but your function continues executing just fine after calling that function. If you want your function to return, you need a `return` statement or you need to bracket all your `res.xxx()` calls in `if/else` so you can never execute more than one of them. – jfriend00 May 16 '18 at 18:07
  • @AshishChoudhary - Added further clarification on this point to my answer. – jfriend00 May 16 '18 at 18:14
  • This is great! Thank you so much for that explaination. It lines up with another error I was getting which was unhandled promises. By not returning the flow continued triggering and not handling additional promises in the code. (I am guessing). – Just A Guy May 16 '18 at 18:21
  • @jfriend00 I am running into this headers issue again :P.. This time I am following your example and ensuring I am returning all of my res statements. I added another code example above, are you able to take a quick peek? – Just A Guy May 17 '18 at 00:24
  • @JustAGuy - There's no `return` after the `res.json()` that comes right after this: `if (moment().isAfter(request.expiry)) {`. – jfriend00 May 17 '18 at 00:27
  • @jfriend00 - Is it bad practice to delete a record using a short statement like this, 'Password.remove({ _id: request._id }).exec()' in mongoose? – Just A Guy May 17 '18 at 00:31
  • @JustAGuy - I'm not a mongoose guy. Don't know. It really isn't stack overflow's system to keep asking more and more questions here. If you really get stuck on something else, ask a new question. – jfriend00 May 17 '18 at 00:40
3

The res object by itself does not stop the execution of your program. You must use return if you prefer to use Guard Clauses instead of Nested Conditions

Replace statements like this:

if (err) {
  res.status(500).json({
    error: err.message
  })
}

With this:

if (err) {
  res.status(500).json({
    error: err.message
  });
  return; // return statement added
}
Iván E. Sánchez
  • 1,183
  • 15
  • 28
  • Actually res.json is the return statement for route... did not return the code execution? – Ashh May 16 '18 at 17:58
  • you are just telling node that I must response that, but without an explicit return statement, your code will still run, so you could do whatever you want after responding to a user except from responding again. – Iván E. Sánchez May 16 '18 at 18:02
  • you can also check this answer for more information https://stackoverflow.com/a/16180550/6111342 – Iván E. Sánchez May 16 '18 at 18:05
  • your welcome. if this response was helpful please mark it as the solution, so other users could easily be helped as well – Iván E. Sánchez May 16 '18 at 18:13
  • You and jfriend00 both had the right answer. I gave the solution to jfriend00 because they were the first to answer. Thank you for the additional link! – Just A Guy May 16 '18 at 18:32