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

No context available when using express-session with Memory store #6

Closed
azatoth opened this issue Aug 17, 2016 · 12 comments
Closed

No context available when using express-session with Memory store #6

azatoth opened this issue Aug 17, 2016 · 12 comments

Comments

@azatoth
Copy link
Contributor

azatoth commented Aug 17, 2016

If using session and token middleware, trying to access the explorer crashes the server with a "Error: No context available. ns.run() or ns.bind() must be called first."

see also strongloop/loopback#2636 and https://github.com/azatoth/loopback-sandbox for a testcase (is also reproducible if using plain loopback-context 1.0.0 without the cls-hooked patch)

@bajtos

@bajtos bajtos self-assigned this Aug 17, 2016
@bajtos bajtos added the bug label Aug 17, 2016
@azatoth
Copy link
Contributor Author

azatoth commented Aug 17, 2016

As loopback is not using getCurrentContext() anymore but is accessing the raw loopbackContext object, there is no check if the context is active or not.

@bajtos
Copy link
Member

bajtos commented Aug 17, 2016

So apparently the getCurrentContext checks for ns.active too. The patch strongloop/loopback#2604 which inlined getCurrentContext did not include this check.

@bajtos bajtos added the #review label Aug 17, 2016
@bajtos bajtos changed the title No context available when accessing explorer No context available when using express-session with Memory store Aug 17, 2016
@bajtos
Copy link
Member

bajtos commented Aug 17, 2016

The problem is caused by the memory store provided by express-session, which calls setImmediate to defer callbacks - see session/memory.js#L25-L27. Apparently setImmediate is not supported by node-continuation-local-storage.

I agree loopback apps should not crash in such case, strongloop/loopback#2649 will fix that.

However, fixing express-session is up to somebody else to do.

@bajtos
Copy link
Member

bajtos commented Aug 17, 2016

The problem is caused by the memory store provided by express-session, which calls setImmediate to defer callbacks - see session/memory.js#L25-L27. Apparently setImmediate is not supported by node-continuation-local-storage.

Let me clarify this. When I changed session/memory to use process.nextTick instead of setImmediate, then the app started to work. There may be something else in play, because node-continuation-local-storage claims to support setImmediate according to their docs (https://github.com/othiym23/node-continuation-local-storage#continuation-local-storage).

@Jeff-Lewis
Copy link

@bajtos have you had a chance to try cls-hooked? I'm curious if it helps with some of these known cls issues. The latest cls-hooked version supports node 4.5.0.

@azatoth
Copy link
Contributor Author

azatoth commented Aug 19, 2016

@Jeff-Lewis initially I was testing with cls-hooked enabled (see https://github.com/azatoth/loopback-sandbox)

@bajtos
Copy link
Member

bajtos commented Aug 24, 2016

@Jeff-Lewis there is a pending pull request reworking loopback-context to use cls-hooked, see #2

@josieusa
Copy link
Collaborator

cls-hooked depends on async-hook, which may supportsetImmediate, or at least so it seems from the source code. See https://github.com/AndreasMadsen/async-hook/blob/9c2d1ee927233ec1ad31cbf286029c1c98bd9885/patches/timers.js#L16

@josieusa
Copy link
Collaborator

Link to PR #11

@josieusa
Copy link
Collaborator

josieusa commented Jan 3, 2017

express-session is known to have problems with cls-hooked, see othiym23/node-continuation-local-storage#29. Maybe I could add tests for express-session to PR #19 to see if it solves the issue.

@bajtos bajtos removed their assignment Apr 28, 2017
@stale
Copy link

stale bot commented Feb 11, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 11, 2020
@stale
Copy link

stale bot commented Feb 27, 2020

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.

@stale stale bot closed this as completed Feb 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants