Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

express-session sometimes saves empty sessions (very rarely) #925

Open
bash-tp opened this issue Jan 10, 2023 · 6 comments
Open

express-session sometimes saves empty sessions (very rarely) #925

bash-tp opened this issue Jan 10, 2023 · 6 comments

Comments

@bash-tp
Copy link

bash-tp commented Jan 10, 2023

First off, I tried to create this issue under expressjs/session but the permissions don't allow me to open issues there. Please tell me if you'd like me to move it somewhere else!

We're using express-session 1.17.2 with connect-mongo 4.6.0. Our app creates thousands of new sessions every day and we rarely have any issues. But sometimes I'll start getting this alert:

Error: TypeError: Cannot read property 'expires' of undefined
    at MongoStore.Store.createSession (node_modules/express-session/session/store.js:87:29)
    at inflate (node_modules/express-session/index.js:372:13)
    at node_modules/express-session/index.js:499:11
    at node_modules/connect-mongo/build/main/lib/MongoStore.js:222:17
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)

When I get the session id and look in mongo, the session field contains the string {}. Somehow it saves an empty json object with no cookie key. I need to manually delete the session so the user gets a new one and then it's resolved.

This happens maybe once a month or so. It's such a tiny fraction of all sessions, but it's still frustrating. It's got to be a race condition somewhere but I haven't been able to find anything obvious.

Has behavior like this been reported before? If not, is it possible to add error checking to Store.prototype.createSession so it can handle a missing session.cookie?

@dougwilson dougwilson transferred this issue from expressjs/express Jan 10, 2023
@dougwilson
Copy link
Contributor

Hi @bash-tp that is very strange. Have you opened an issue with your connect-mongo driver? That is what is doing the serialization/deserialization of the session. This module is designed to error out if the desterilized session is not valid, which it sounds like is the case for you, so would seem to be working as intended from the deserialization function. The question, though, is how is it getting serialized like that? An initial look seems they are doing some kind of special handling of the cookie property (https://github.com/jdesboeufs/connect-mongo/blob/v4.6.0/src/lib/MongoStore.ts#L88) during sterilization. I'm not sure what that is doing, exactly, but certainly seems suspect...

@bash-tp
Copy link
Author

bash-tp commented Jan 10, 2023

Thanks for the quick response! I'm not sure which layer the problem is getting introduced in. Also FWIW, it happens with existing sessions, not new... I had a case earlier today where there was a session in use for about an hour with no issues and then all of a sudden this happened. I opened an issue for connect-mongo so we'll see what happens there.

@dougwilson
Copy link
Contributor

Ok. Unfortunately the most I can say is that there is no serialization in this module itself -- that's just done by the store. The session object from this module could in theory be missing the cookie property somehow, but not sure how such a thing would happen off-hand.

I also searched the issues here for prior occurrences of this error and it doesn't seem to be any (just a test broken when V8 changed the wording of the error): https://github.com/expressjs/session/issues?q=%22Cannot+read+property+%27expires%27+of+undefined%22 which is the main reason I was thinking it's possible the issue lies in the store code.

If there was a way to get a reproduction case put together where we could determine the reason for the missing cookie property in the database that would be ideal, of course. I get it of course that is probably not possible due to the complexities involved here.

But, if this were my code base, I would honestly probably patch the mongo-session module to assert if the session object handed to it from express-session has a cookie property on it, as it always should. If I saw that assert in production that would help me narrow down where to move to next to try and determine where the cookie property is going missing...

@tperamaki
Copy link

Any resolution or workaround for this?

I'm seeing the same error, but with connect-memcached.

I keep seeing it while running robot framework tests, which do lots of stuff. It happens on every test run, but in a different test / place on each run.

@bash-tp
Copy link
Author

bash-tp commented Jun 20, 2023

Sorry I didn't make any more progress with troubleshooting. It happens pretty rarely for me now though (last instance was a couple of months ago, and I don't think I made any other changes that could have had an impact) so it hasn't been a priority for me anymore.

@tperamaki
Copy link

Thanks for the response, I set the idle timeout of the connections to memcached to 10ms instead of default 5000ms, and it seems to be a working workaround for my situation.

What that effectively does is that it kills the connections right after use, so the connections won't be re-used, which seems to remove the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants