1

My server crashed and I found the following error in my logs:

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

and I could not reproduce the error. I think I found a fix, but I want to ensure that I understand the problem, which is outlined in the code below. Based on this post, I believe it is coming from the multiple res.send(..) calls in my Express router:

router.post('/adduser', function(req, res) {
    var msg = '(message okay)'
    var db = req.db;    
    db.collection('userlist').find({
        username: req.body.username;
    }).toArray(function(err, items) {

        // verify the user, set session fields, etc...

        console.log("message after user verification is: ");
        console.log(msg);

        res.send({
            msg: msg
        });

    });

    // Insert the request's data into the 'userlist' collection.
    db.collection('userlist').insert(body, function(err, result) {
        console.log("inserting into userlist...");
        res.send(
            (err === null) ? {
                msg: msg
            } : {
                msg: err
            }
        );
    });
    console.log("message after user verification is: ");
    console.log(msg);
}

The last few lines of my log are:

message after user verification is: 
(message okay)
POST /login/adduser 200 7ms - 10b
inserting into userlist...

/home/lucas/mynode/node_modules/mongoskin/node_modules/mongodb/lib/mongodb/c
onnection/base.js:245
        throw message;      
              ^

Error: Can't set headers after they are sent.
 at ServerResponse.OutgoingMessage.setHeader (http.js:689:11)
 at ServerResponse.header (/home/lucas/mynode/node_modules/express/lib/response.js:717:10)
 at ServerResponse.send 

I suspect that I am calling the res.send(..) twice, so I believe that I fixed this by removing the first res.send(..), but is this the correct fix? Also, if calling res.send(..) twice causes the error, then why can't I reproduce this bug? I never got the error, but another user did.

Can anyone provide an explanation? Feedback on my solution would also be very helpful.

Community
  • 1
  • 1
modulitos
  • 14,737
  • 16
  • 67
  • 110
  • 2
    Yes, it's due to the multiple uses of `res.send()`. Express' `res.send()` finalizes the response to the web client via [`res.end()`](http://nodejs.org/api/http.html#http_response_end_data_encoding). Once the response has been ended, no further output should/can be written until a new `req` arrives with its own `res`. – Jonathan Lonowski Sep 18 '14 at 20:50
  • 1
    Note, it looks like you may also have a race condition between two async operations and there is no guarantee of order. If you want to make sure the second db operation happens after the first one, then you need to manually sequence them by not starting the second one until the first one is done or by not inserting the result from the second one until the first one is done. – jfriend00 Sep 18 '14 at 20:58
  • Recently I was also having the same problem. I had used res.set("Connection", "close"); after res.json() ... and that was the problem. I removed the res.set and wolla, its working great. – Abhishek Deb Dec 12 '14 at 07:20

2 Answers2

3

you should restructure your code, atm you have got 2 async calls in your code

db.collection('userlist').find is called and will return later
db.collection('userlist').insert and will also return later

so you call res.send 2 times

something like this and you won't call res.send twice

router.post('/adduser', function(req, res) {
  var msg = '(message okay)'
  var db = req.db;

  db.collection('userlist').find({username: req.body.username}).toArray(function(err, items) {
    // verify the user, set session fields, etc...
    // if error return(send) error
    if(err) return res.send({err:"errormessage"})
    // if everything is ok - Insert the request's data into the 'userlist' collection.
    db.collection('userlist').insert(body, function(err, result) {
      //if error return(send)  error
      if(err) return res.send({err:err})
      //everything was ok send msg
      res.send({msg:msg})
    })
  })
James
  • 80,725
  • 18
  • 167
  • 237
supernova
  • 3,814
  • 3
  • 22
  • 37
2

You are correct about the cause of the problem, however, there isn't enough information here to determine whether the solution is the correct one.

For example, there is no obvious relationship from the example between the find / insert calls - are these supposed to be called in series? Does insert require anything from the find? For example, if you need to check a record exists before inserting then the correct solution would look something like

db.collection('userlist').findOne({
    username: req.body.username
}, function(err, result) {
    if (result !== null) {
        db.collection('userlist').update(result, body, function(err, updateCount) {
            if (err !== null) {
                console.log('Updated existing record');
            }
        });
    } else {
        db.collection('userlist').insert(body, function(err, result) {
            if (err !== null) {
                console.log('Created new record');
            }
        });
    }
});

Notice the difference here? Remember, find and insert are non-blocking calls so in your example you can't guarentee the order of which both callbacks are run. However, by moving the insert call into the find callback we are guarenteeing that insert will always be called after the find.

James
  • 80,725
  • 18
  • 167
  • 237