Skip to content

💾 feat: Production-ready Memory Store for express-session #5212

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

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

lkiesow
Copy link
Contributor

@lkiesow lkiesow commented Jan 8, 2025

The express-session library comes with a session storage meant for testing by default. That is why you get a message like this when you start up LibreChat with OIDC enabled:

Warning: connect.session() MemoryStore is not
designed for a production environment, as it will leak
memory, and will not scale past a single process.

LibreChat can already use Redis as a session storage, although Redis support is still marked as experimental. It also makes the set-up more complex, since you will need to configure and run yet another service.

This pull request provides a simple alternative by using a in-memory session store marked as a production-ready alternative by the guys from express-session¹. You can still configure Redis, but this provides a simple, good default for everyone else.

See also #1014

¹⁾ https://github.com/expressjs/session?tab=readme-ov-file#compatible-session-stores

Change Type

Please delete any irrelevant options.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Translation update

Testing

  • Configure OIDC
  • Running without this patch: You should see the memry leak warning from above
  • Running with this patch applied: No warning should show up
  • In any case, the login via OIDC should still work

The `express-session` library comes with a session storage meant for
testing by default. That is why you get a message like this when you
start up LibreChat with OIDC enabled:

    Warning: connect.session() MemoryStore is not
    designed for a production environment, as it will leak
    memory, and will not scale past a single process.

LibreChat can already use Redis as a session storage, although Redis support
is still marked as experimental. It also makes the set-up more complex, since
you will need to configure and run yet another service.

This pull request provides a simple alternative by using a in-memory session
store marked as a production-ready alternative by the guys from
`express-session`¹. You can still configure Redis, but this provides a simple,
good default for everyone else.

See also danny-avila#1014

¹⁾ https://github.com/expressjs/session?tab=readme-ov-file#compatible-session-stores
@dennis531
Copy link
Contributor

We have the problem that the session is undefined sometimes. I found a similar problem on https://stackoverflow.com/questions/63259184/node-with-express-session-issue but there is no working solution. It seems to be a race condition. I hope the new memory store solves this problem.

Logs

LibreChat | Error: did not find expected authorization request details in session, req.session["oidc:studip.de"] is undefined
LibreChat | at /app/node_modules/openid-client/lib/passport_strategy.js:132:13
LibreChat | at OpenIDConnectStrategy.authenticate (/app/node_modules/openid-client/lib/passport_strategy.js:191:5)
LibreChat | at attempt (/app/node_modules/passport/lib/middleware/authenticate.js:369:16)
LibreChat | at authenticate (/app/node_modules/passport/lib/middleware/authenticate.js:370:7)
LibreChat | at Layer.handle [as handle_request] (/app/node_modules/express/lib/router/layer.js:95:5)
LibreChat | at next (/app/node_modules/express/lib/router/route.js:149:13)
LibreChat | at Route.dispatch (/app/node_modules/express/lib/router/route.js:119:3)
LibreChat | at Layer.handle [as handle_request] (/app/node_modules/express/lib/router/layer.js:95:5)
LibreChat | at /app/node_modules/express/lib/router/index.js:284:15
LibreChat | at Function.process_params (/app/node_modules/express/lib/router/index.js:346:12)

@danny-avila danny-avila changed the title Provide production-ready memory store for eypress-session 💾 feat: Production-ready Memory Store for express-session Jan 8, 2025
@danny-avila danny-avila merged commit dd92758 into danny-avila:main Jan 9, 2025
1 check passed
danny-avila added a commit that referenced this pull request Jan 10, 2025
danny-avila added a commit that referenced this pull request Jan 10, 2025
* ✨ feat: Add OpenWeather Tool for Weather Data Retrieval 🌤️

* chore: linting

* chore: move test files

* fix: tool icon, allow user-provided keys, conform to app key assignment pattern

* chore: linting not included in #5212

---------

Co-authored-by: Jonathan Addington <[email protected]>
aleibovici pushed a commit to ASISolutions/LibreChat that referenced this pull request Jan 12, 2025
…a#5212)

The `express-session` library comes with a session storage meant for
testing by default. That is why you get a message like this when you
start up LibreChat with OIDC enabled:

    Warning: connect.session() MemoryStore is not
    designed for a production environment, as it will leak
    memory, and will not scale past a single process.

LibreChat can already use Redis as a session storage, although Redis support
is still marked as experimental. It also makes the set-up more complex, since
you will need to configure and run yet another service.

This pull request provides a simple alternative by using a in-memory session
store marked as a production-ready alternative by the guys from
`express-session`¹. You can still configure Redis, but this provides a simple,
good default for everyone else.

See also danny-avila#1014

¹⁾ https://github.com/expressjs/session?tab=readme-ov-file#compatible-session-stores
aleibovici pushed a commit to ASISolutions/LibreChat that referenced this pull request Jan 12, 2025
…#5246)

* ✨ feat: Add OpenWeather Tool for Weather Data Retrieval 🌤️

* chore: linting

* chore: move test files

* fix: tool icon, allow user-provided keys, conform to app key assignment pattern

* chore: linting not included in danny-avila#5212

---------

Co-authored-by: Jonathan Addington <[email protected]>
owengo pushed a commit to openwengo/LibreChat that referenced this pull request Jan 21, 2025
…a#5212)

The `express-session` library comes with a session storage meant for
testing by default. That is why you get a message like this when you
start up LibreChat with OIDC enabled:

    Warning: connect.session() MemoryStore is not
    designed for a production environment, as it will leak
    memory, and will not scale past a single process.

LibreChat can already use Redis as a session storage, although Redis support
is still marked as experimental. It also makes the set-up more complex, since
you will need to configure and run yet another service.

This pull request provides a simple alternative by using a in-memory session
store marked as a production-ready alternative by the guys from
`express-session`¹. You can still configure Redis, but this provides a simple,
good default for everyone else.

See also danny-avila#1014

¹⁾ https://github.com/expressjs/session?tab=readme-ov-file#compatible-session-stores
owengo pushed a commit to openwengo/LibreChat that referenced this pull request Jan 21, 2025
…#5246)

* ✨ feat: Add OpenWeather Tool for Weather Data Retrieval 🌤️

* chore: linting

* chore: move test files

* fix: tool icon, allow user-provided keys, conform to app key assignment pattern

* chore: linting not included in danny-avila#5212

---------

Co-authored-by: Jonathan Addington <[email protected]>
justinmdickey pushed a commit to e-gineering/LibreChat that referenced this pull request Jan 30, 2025
…a#5212)

The `express-session` library comes with a session storage meant for
testing by default. That is why you get a message like this when you
start up LibreChat with OIDC enabled:

    Warning: connect.session() MemoryStore is not
    designed for a production environment, as it will leak
    memory, and will not scale past a single process.

LibreChat can already use Redis as a session storage, although Redis support
is still marked as experimental. It also makes the set-up more complex, since
you will need to configure and run yet another service.

This pull request provides a simple alternative by using a in-memory session
store marked as a production-ready alternative by the guys from
`express-session`¹. You can still configure Redis, but this provides a simple,
good default for everyone else.

See also danny-avila#1014

¹⁾ https://github.com/expressjs/session?tab=readme-ov-file#compatible-session-stores
justinmdickey pushed a commit to e-gineering/LibreChat that referenced this pull request Jan 30, 2025
…#5246)

* ✨ feat: Add OpenWeather Tool for Weather Data Retrieval 🌤️

* chore: linting

* chore: move test files

* fix: tool icon, allow user-provided keys, conform to app key assignment pattern

* chore: linting not included in danny-avila#5212

---------

Co-authored-by: Jonathan Addington <[email protected]>
justinmdickey pushed a commit to e-gineering/LibreChat that referenced this pull request Feb 7, 2025
…a#5212)

The `express-session` library comes with a session storage meant for
testing by default. That is why you get a message like this when you
start up LibreChat with OIDC enabled:

    Warning: connect.session() MemoryStore is not
    designed for a production environment, as it will leak
    memory, and will not scale past a single process.

LibreChat can already use Redis as a session storage, although Redis support
is still marked as experimental. It also makes the set-up more complex, since
you will need to configure and run yet another service.

This pull request provides a simple alternative by using a in-memory session
store marked as a production-ready alternative by the guys from
`express-session`¹. You can still configure Redis, but this provides a simple,
good default for everyone else.

See also danny-avila#1014

¹⁾ https://github.com/expressjs/session?tab=readme-ov-file#compatible-session-stores
justinmdickey pushed a commit to e-gineering/LibreChat that referenced this pull request Feb 7, 2025
…#5246)

* ✨ feat: Add OpenWeather Tool for Weather Data Retrieval 🌤️

* chore: linting

* chore: move test files

* fix: tool icon, allow user-provided keys, conform to app key assignment pattern

* chore: linting not included in danny-avila#5212

---------

Co-authored-by: Jonathan Addington <[email protected]>
danny-avila pushed a commit that referenced this pull request Mar 27, 2025
The `express-session` library comes with a session storage meant for
testing by default. That is why you get a message like this when you
start up LibreChat with OIDC enabled:

    Warning: connect.session() MemoryStore is not
    designed for a production environment, as it will leak
    memory, and will not scale past a single process.

LibreChat can already use Redis as a session storage, although Redis support
is still marked as experimental. It also makes the set-up more complex, since
you will need to configure and run yet another service.

This pull request provides a simple alternative by using a in-memory session
store marked as a production-ready alternative by the guys from
`express-session`¹. You can still configure Redis, but this provides a simple,
good default for everyone else.

See also #1014

¹⁾ https://github.com/expressjs/session?tab=readme-ov-file#compatible-session-stores
danny-avila added a commit that referenced this pull request Mar 27, 2025
* ✨ feat: Add OpenWeather Tool for Weather Data Retrieval 🌤️

* chore: linting

* chore: move test files

* fix: tool icon, allow user-provided keys, conform to app key assignment pattern

* chore: linting not included in #5212

---------

Co-authored-by: Jonathan Addington <[email protected]>
MichielMAnalytics pushed a commit to MichielMAnalytics/ProAI that referenced this pull request Jun 6, 2025
…a#5212)

The `express-session` library comes with a session storage meant for
testing by default. That is why you get a message like this when you
start up LibreChat with OIDC enabled:

    Warning: connect.session() MemoryStore is not
    designed for a production environment, as it will leak
    memory, and will not scale past a single process.

LibreChat can already use Redis as a session storage, although Redis support
is still marked as experimental. It also makes the set-up more complex, since
you will need to configure and run yet another service.

This pull request provides a simple alternative by using a in-memory session
store marked as a production-ready alternative by the guys from
`express-session`¹. You can still configure Redis, but this provides a simple,
good default for everyone else.

See also danny-avila#1014

¹⁾ https://github.com/expressjs/session?tab=readme-ov-file#compatible-session-stores
MichielMAnalytics pushed a commit to MichielMAnalytics/ProAI that referenced this pull request Jun 6, 2025
…#5246)

* ✨ feat: Add OpenWeather Tool for Weather Data Retrieval 🌤️

* chore: linting

* chore: move test files

* fix: tool icon, allow user-provided keys, conform to app key assignment pattern

* chore: linting not included in danny-avila#5212

---------

Co-authored-by: Jonathan Addington <[email protected]>
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

Successfully merging this pull request may close these issues.

3 participants