node-continuation-local-storage
node-continuation-local-storage copied to clipboard
Context gets lost in Express after using session middleware
Hi Forrest,
First of all, in my view cls is a wonderful peace of software. I just started using it and it solves lots of issues for me. Thank you for creating it and sharing it.
This is actually a questing rather than an issue. I've successfully integrated cls in an Express 4 application but after adding session middleware I started experiencing issues. To be more specific, the context gets lost. The code I've got is very similar to the following:
var express = require('express');
var session = require('express-session');
var cls = require('continuation-local-storage');
var ns = cls.createNamespace('contextNamespace');
var app = express();
app.use(function(req, res, next) {
ns.bindEmitter(req);
ns.bindEmitter(res);
ns.run(function() {
next();
});
});
app.use(function(req, res, next ) {
// context is available
var namespace = cls.getNamespace('contextNamespace');
next();
});
// this is the critical session middleware
app.use(session({ resave: true, saveUninitialized: true, secret: 'someSecret', store: sessionStore }));
app.use(function(req, res, next ) {
// context is lost
var namespace = cls.getNamespace('contextNamespace');
next();
});
As per the comments in the code above, the content gets lost for all middleware after app.use(session());
I went through all open and closed issues and I gather that in this kind of situations ns.bind() can be used but I don't quite understand how. So, I was wandering if you could give me some directions as to how to approach and solve this issue?
Thank you in advance.
I've got one other minor note. In the very first code example in your README.md, you have session.set('user', user);
which doesn't seem to be enclosed in a session.run()
. Is this OK or my understanding of cls is still too shallow?
Cheers,
Nasko.
In the very first code example in your README.md, you have
session.set('user', user);
which doesn't seem to be enclosed in asession.run()
.
The first 2 examples are probably a little oversimplified, but what they're intended to convey is the idea that somewhere else something is calling ns.run()
, and the set and get calls don't have to care, because they do their lookup from the CLS context dynamically. The first example should probably be more complete, and a patch to that effect would be welcome.
So, I was wandering if you could give me some directions as to how to approach and solve this issue?
I would have to dig into the source of express-session
and see what it's doing that's interrupting the continuation chain (there are a lot of candidates, from a superficial skimming of the source – there's its custom defer
function, which does some very clever, potentially CLS-breaking things under Node 0.8 and earlier; there's the monkeypatching it does of res.end
and other methods). Something you could try is changing the session setup to
app.use(
ns.bind(
session({
resave : true,
saveUninitialized: true,
secret: 'someSecret',
store: sessionStore
})
)
);
That may not work (see above for the cleverness going on inside express-session
), in which case either you can dig in to the source yourself and try to figure out what's going on, or you can ping me, and when I have a few spare moments (probably not too soon) I can dig into it.
@wraithan or @hayes, have either of you run into this with Express 4? Might you have any pointers here?
Thanks for the prompt reply, Forrest. Much appreciated.
Unfortunately, the suggested app.use(ns.session())}
didn't work. In my case the quick and dirty workaround is to have the cls middleware declared after the session one.
In regards to:
The first example should probably be more complete, and a patch to that effect would be welcome.
I'll see if I can come up with a suitable patch.
Cheers.
Maybe not a real help but at least a comment:
I'm using express4 but not with the regular express-session
module but with client-session
.
That's working like a charm with the usual (IMHO this should be added to the README.md
)
app.use(function initializeContinuationLocalStorage(req, res, next) {
ns.bindEmitter(req);
ns.bindEmitter(res);
ns.run(function() {
next();
});
});
Therefore express itself seems to be fine and it's express-session
itself causing the issue.
@nasooz not sure if you typo-ed there, but wanted to highlight the differences in what you indicated didn't work and what was suggested.
@othiym23 indicated you use the ns.bind() wrapper around the already existing session function. ns.bind(session)
In your reply where you said it didnt work you wrote ns.session() , which does not use the bind method defined in the source https://github.com/othiym23/node-continuation-local-storage/blob/master/context.js
I believe ns.session() would cause an uncaught exception because session is not a method on the ns object
@othiym23 this looks like a great module, thanks for all your work! I completely agree with you, that node needs some sort of capability for thread-local storage and that passing request/response objects through your entire application can't be the solution...
@nasooz, like you I am using this module in combination with both express and express-session and ran into the same problems with the CLS middleware not carrying through the entire request. You mentioned your workaround with adding the CLS middleware after the session middleware worked for you. Would you be willing to share a bit more of the code that worked for you? I am basically using with the same setup as the code in your first comment, except my CLS middleware is added after the session middleware. I then have another middleware that adds some property from the request object onto the active namespace (ns.set('key', value)
, this works) and am trying to read this property somewhere deep down the callback/promise stack (where I don't have access to the request/response objects), but the active namespace has already been removed/exited and the property is not available anymore.
Any suggestions would be greatly appreciated. Thanks!
This is an abbreviated chunk of a larger solution (e.g. you need to set up domains earlier on in execution) -- but I thought I would share nonetheless. Pretty sure we've nailed the issue at least for our case. Will possibly add the other code needed later. Also see my comment here: https://github.com/nodejs/node/issues/66
var domain = require("domain")
var session = require('express-session');
var sessionMiddleware = exports.sessionMiddleware = session({
secret: secret,
store: sessionMongoStore,
rolling:true
})
function domainBind(fn)
{
if( process.domain && typeof fn === 'function'){
fn = process.domain.bind(fn)
}
return fn
}
app.use(function(req, res, next)
{
var myNext = domainBind(function(){
next();
})
sessionMiddleWare(req,res, myNext)
});
Just to say that I don't think this is a bug with CLS. Just express-session
is losing CLS context due to using queues internally, as many/some other libraries do. Really it needs someone to create cls-express-session
module.
But I think the following should work...
Replace:
app.use( session( { /* options */ } ) );
with:
var sessionMiddleware = session( { /* options */ } );
app.use( function(req, res, next) {
return sessionMiddleware.call( this, req, res, ns.bind(next) );
} );
A function which patches an express middleware to make it retain CLS context:
function clsifyMiddleware(fn, ns) {
return function(req, res, next) {
return fn.call(this, req, res, ns.bind(next));
};
}
Then you do:
app.use( clsifyMiddleware( session( { /* options */ } ), ns ) );
NB the above doesn't re-bind res
and req
. But I think that isn't necessary. Maybe someone else can comment on that.