Session: Errors in store actions always result in an "Error: Can't set headers after they are sent." error

[darrenscerri]

Currently an error in a store action such as reading or writing automatically ends the response using the writeend function. Therefore, any custom error middleware handlers cannot send any headers, and would emit an "Error: Can't set headers after they are sent." error.

The following code block replicates the issue:

var express = require('express');
var http = require('http');
var session = require('express-session');

var app = express();

var MongoStore = require('connect-mongo')(session);

var mongoStore = new MongoStore({
  url: 'mongodb://localhost:27017/test',
  collection: 'sessions'
});

app.use(session({
  secret: 'secret',
  saveUninitialized: true,
  resave: true,
  store: mongoStore
}));

app.get('/', function(req, res, next) {
  res.json({ ok: true });
});

app.use(function(err, req, res, next) {
  res.sendStatus(500);
});

http.createServer(app).listen(5000);

This requires a working MongoDB server running on port 27017. If the MongoDB server is terminated and a GET request is sent to the Express server on '/', this would trigger this error:

Error: Can't set headers after they are sent.
    at ServerResponse.OutgoingMessage.setHeader (_http_outgoing.js:344:11)
    at ServerResponse.header (~/node_modules/express/lib/response.js:718:10)
    at ServerResponse.contentType (~/node_modules/express/lib/response.js:551:15)
    at ServerResponse.sendStatus (~/node_modules/express/lib/response.js:339:8)
    at ~/index.js:26:7
    at Layer.handle_error (~/node_modules/express/lib/router/layer.js:71:5)
    at trim_prefix (~/node_modules/express/lib/router/index.js:310:13)
    at ~/node_modules/express/lib/router/index.js:280:7
    at Function.process_params (~/node_modules/express/lib/router/index.js:330:12)
    at Immediate.next (~/node_modules/express/lib/router/index.js:271:10)
    at Immediate.immediate._onImmediate (timers.js:435:18)

[joewagner]

This is potentially a duplicate of issue https://github.com/expressjs/session/issues/4

[dougwilson]

This is actually an issue in the user's code posted above, specifically the error middleware. You can see the issue highlighted in the stack trace provided.

The error handler never checks if a response has actually been written before it tries to write one itself, thus ending with that error. An error can occurs at any point in a request/response cycle, even if a response has been half-written.

I would suggest just dropping that middleware, because it pretty much does nothing. Otherwise, I suggest checking the res.headersSent (https://nodejs.org/dist/latest-v4.x/docs/api/http.html#http_response_headerssent) flag before you try to write an error response and either do nothing, or keep passing the error to the next handler:

app.use(function(err, req, res, next) {
  if (res.headersSent) return next(err);
  res.sendStatus(500);
});

[panManfredini]

Hi,
I also bumped in this issue that seem to have been already discussed a few times #499 #52 #225, but it doesn't look to have been solved, and I'd like to share my thoughts on it.
The issue is that whenever a sessionStore.set() method returns an error, the error is catch and handled by the session-middleware however this only happens after headers are sent, causing the error "Can't set headers after they are sent". Now as @dougwilson says just above, this is not an issue and one should handle this in the error handler. What I think is an issue is that the user in this case will get a page with status code 200 and a cookie, which will be invalid since is not stored. You can test this by simply doing:

app.use( 
    session({
        secret: 'MyVerySecretSectret',
        resave: false,
        saveUninitialized: false,
        cookie: { secure: false, httpOnly: true, sameSite: true }
    })
);


app.get('/', function(req,res, next){

  req.sessionStore.set = (sessionId, session, fn) => { 
      fn(new Error("Always throwing....")); 
  };

  req.session.modifyme = "hey";
  res.render('index');

});

Probably there aren't many cases in which this would happen (I encounter it because was writing invalid data to a table), but in general would be nice to not have the header sent before confirmation from the store. This can be done (as was briefly discussed in #52) by giving the option of force saving, or even recommending it. This is already possible by calling the method req.session.save(callback) so one can handle the error as he please (one must call also session.desrtroy() in the error handler otherwise the cookie will be attached anyway). Although is working well for the case of a store error, when everything is ok (no error) it is saving twice, up to now I couldn't understand why.

Well, that's my understanding... what do you guys think? Am I doing some stupid mistake somewhere?