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

need to check if the context is active #2636

Closed
wants to merge 1 commit into from
Closed

Conversation

azatoth
Copy link
Contributor

@azatoth azatoth commented Aug 16, 2016

if you for example load the explorer, you don't have any active context,
and it will fail here then with the message:
"Error: No context available. ns.run() or ns.bind() must be called
first."

Fix strongloop/loopback-context#6

if you for example load the explorer, you don't have any active context,
and it will fail here then with the message:
  "Error: No context available. ns.run() or ns.bind() must be called
  first."
@slnode
Copy link

slnode commented Aug 16, 2016

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@slnode
Copy link

slnode commented Aug 16, 2016

Can one of the admins verify this patch?

3 similar comments
@slnode
Copy link

slnode commented Aug 16, 2016

Can one of the admins verify this patch?

@slnode
Copy link

slnode commented Aug 16, 2016

Can one of the admins verify this patch?

@slnode
Copy link

slnode commented Aug 16, 2016

Can one of the admins verify this patch?

@bajtos
Copy link
Member

bajtos commented Aug 16, 2016

Thank you @azatoth for the pull request. As I mentioned in Gitter, I think this is fixing a wrong place. req.loopbackContext should be set only when there is an active context configured for the request. The problem you are observing is probably a bug in loopback-context. See https://github.com/strongloop/loopback-context/blob/268716f69eec246b4ae0f7e402147b6fcbc924cf/server/middleware/per-request.js#L36-L59.

@bajtos
Copy link
Member

bajtos commented Aug 16, 2016

@azatoth please reproduce the problem using loopback-context only, add a failing test to https://github.com/strongloop/loopback-context/blob/268716f69eec246b4ae0f7e402147b6fcbc924cf/test/main.test.js and then try to fix the problem directly in loopback-context.

@bajtos
Copy link
Member

bajtos commented Aug 17, 2016

Reopening.

So apparently getCurrentContext checks for ns.active too, see https://github.com/strongloop/loopback-context/blob/268716f69eec246b4ae0f7e402147b6fcbc924cf/server/current-context.js#L92. The patch #2604 which inlined getCurrentContext did not include this check.

@bajtos bajtos reopened this Aug 17, 2016
@slnode
Copy link

slnode commented Aug 17, 2016

Can one of the admins verify this patch?

3 similar comments
@slnode
Copy link

slnode commented Aug 17, 2016

Can one of the admins verify this patch?

@slnode
Copy link

slnode commented Aug 17, 2016

Can one of the admins verify this patch?

@slnode
Copy link

slnode commented Aug 17, 2016

Can one of the admins verify this patch?

@slnode
Copy link

slnode commented Aug 17, 2016

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@bajtos bajtos added the #review label Aug 17, 2016
@bajtos
Copy link
Member

bajtos commented Aug 17, 2016

@slnode ok to test

@bajtos
Copy link
Member

bajtos commented Aug 17, 2016

Closing in favour of #2649

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.

4 participants