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

MongoDB - loopback.getCurrentContext() return null #21

Closed
bajtos opened this issue Jan 5, 2017 · 15 comments
Closed

MongoDB - loopback.getCurrentContext() return null #21

bajtos opened this issue Jan 5, 2017 · 15 comments
Labels

Comments

@bajtos
Copy link
Member

bajtos commented Jan 5, 2017

@dongmai commented on Wed Jul 20 2016

Hi everyone ,

Sorry for duplication, but I'm facing it again on nearly the latest version of loopback.
Here is my pacakge.json

{
  "name": "test",
  "version": "1.0.0",
  "main": "server/server.js",
  "scripts": {
    "start": "node .",
    "pretest": "eslint .",
    "posttest": "nsp check"
  },
  "dependencies": {
    "async": "^2.0.0",
    "compression": "^1.0.3",
    "cors": "^2.5.2",
    "helmet": "^1.3.0",
    "loopback": "^2.22.0",
    "loopback-boot": "^2.6.5",
    "loopback-component-explorer": "^2.4.0",
    "loopback-connector-mongodb": "^1.15.2",
    "loopback-datasource-juggler": "^2.39.0",
    "loopback-testing": "^1.2.0",
    "serve-favicon": "^2.0.1"
  },
  "devDependencies": {
    "eslint": "^2.5.3",
    "nsp": "^2.1.0"
  },
  "repository": {
    "type": "",
    "url": ""
  },
  "license": "UNLICENSED",
  "description": "test"
}

I've used a middleware to save current logged user data into context but it (getCurrentContext) always returned null when working with MongoDB (memory db works just fine.)

Bellow is my code,

app.use(loopback.token()); 
app.use(loopback.context());

app.use(function (req, res, next) {
    if (!req.accessToken) return next();

    app.models.User.findById(req.accessToken.userId, function(err, user) {
        if (err)   return next(err);
        if (!user) return next(new Error('No user with this access token was found.'));
        res.locals.currentUser = user;

        var loopbackContext = loopback.getCurrentContext();
        console.log('SERVER CTX?' +loopbackContext);
        if (loopbackContext) {
            loopbackContext.set('currentUser', user);
        }else{
            throw new Error('Current context is null.');
        }
        next();
    });
});

Can anyone please have a look ? Have I done something wrong ?

Thanks.


@0candy commented on Wed Jul 20 2016

Thank you for pointing out this issue. Due to the instability, there are plans to deprecate this method.
See strongloop/loopback#1676


@syuhei176 commented on Sun Jul 24 2016

I had same problem, so I use [email protected] . It work fine.


@beornharris commented on Tue Aug 02 2016

Hey, I've run across this issue as well and just wanted to add some extra info (hope it helps somewhere). I had quick look through #1676 and didnt see anything that might help me.

I initially had an earlier version of loopback running, where I had no problems calling loopback.getCurrentContext() from within a remote method. I inadvertently updated to the current version, and started seeing issues.

I have created a demo repo (from scratch and using slc commands + customisation) to help explain.

https://github.com/beornharris/authtest

The issue I am experiencing is directly related to MongoDB and access tokens, as far as I can tell. Using the repo, the steps to replicate (in api explorer) are as follows (note: requires mongo instance) :

  1. go to endpoint /Networks/{id}/contacts, enter an ID (value irrelevant) and empty object {} and submit. The console shows a context received at the endpoint (from common/models/network.js - Network.addContact)
CONTEXT:  Namespace {
  name: 'loopback',
  active: { accessToken: undefined },
  _set: [ null ],
  id:
   AsyncListener {
     create: [Function],
     flags: 15,
     before: [Function],
     after: [Function],
     error: [Function],
     uid: 2,
     data: null } }
  1. Login using /Accounts/login

`{ "email": "[email protected]", "password": "user1"}

and set the Access Token.

  1. Repeat step 1 - the console shows

CONTEXT: null

Given this, I have a few other observations :

  1. Issue does not occur when using memory DB connector
  2. CONTEXT always shows null when ACL or AccessToken are set to mongo (this may well be by design, but I dont recall reading in the docs that these should ALWAYS be in-memory)
  3. Error also occurs when using the built-in User model, and not just the derived (Account) model.

EDIT: @syuhei176 - I can confirm that the issue disappears with [email protected]


@dongmai commented on Wed Aug 03 2016

Just try to use [email protected] And it isn't working. :(


@beornharris commented on Wed Aug 03 2016

@dongmai - just to confirm, to downgrade the mongo connector you did something like the following...

  1. change package.json from "^1.15.2" to "1.15.1"
  2. npm install

so it is using 1.15.1, and not minimum 1.15.1 ?


@josieusa commented on Wed Jan 04 2017

I had the same issue using the same connector.
I solved it using my own PR and Node 6.3.1 locally:
#2
#11
So far, so good.
Of course, it's going to need more tests.
I'd also mention that it solved other similar issues with getCurrentContext() for me (see #1495 for examples).
Your mileage may vary


@dongmai commented on Thu Aug 04 2016

@beornharris Yes, I've downgraded the version number in package json and re-run npm install of course.

Before downgrading it was working on local but not on remote server, and its the same after downgrading.


@evseevnn commented on Tue Oct 11 2016

Strange, after downgrading version i try use Model.observe('before save', ...)
and i get context === null inside.

@josieusa @bajtos
P.S. when i use mongo not work before save because by default for mongo version >=3 in connector no findOrCreate method. So, i improve dao.js for fire this event and get this problem. Maybe this depend on use notifyObserversOf? because after use this function i lose context. i tried to use commit of @josieusa but this not helped


@josieusa commented on Wed Jan 04 2017

@beornharris I reproduced the issue using your authtest repo.
I'll verify my hypothesis that the issue could be caused by the es6-promise dependency of mongodb which is imported here:
https://github.com/mongodb/node-mongodb-native/blob/V2.2.19/package.json#L16
This hypothesis comes from my experience. In fact, in my PR 19 of loopback-context I just made some experiments, and it seems like it could fix problems with other Promise libraries, but only if these are used inside user code, and not some 3rd party library.
Sadly, I just tested that it didn't fix your issue, because that es6-promise is used in a 3rd party module (mongodb), and as I just said, my PR doesn't cover this case. (Also, I don't know yet if this is the root of this issue).
I'm not sure if this can be fixed by user code alone.
I know that you fixed your problem by changing versions, but other people may still be looking for proper solutions.


@Jeff-Lewis commented on Wed Jan 04 2017

@josieusa try telling MongoDB to use cls-bluebird instead and then use cls-hooked instead of cls when initializing cls-bluebird.

In MongoClient.connect we can pass in the promiseLibrary to use. Now I'm not sure if there is an easy way to tell loopback-connector-mongodb to use our instance of cls-bluebird without forking it.

@dehypnosis
Copy link

Same problem again in release 3.0.0 with mysql connector.

in Model.prototype.someRemoteMethod
and in Model.observe('before save', ...);

i will report some details later.

@bajtos
Copy link
Member Author

bajtos commented Jan 9, 2017

FWIW, loopbackio/loopback-connector-mongodb#275 shows how to patch the MongoDB connector to restore current context.

@bajtos
Copy link
Member Author

bajtos commented Jan 9, 2017

@dehypnosis

Same problem again in release 3.0.0 with mysql connector.

in Model.prototype.someRemoteMethod
and in Model.observe('before save', ...);
i will report some details later.

Please open a new issue to keep track of the problem with MySQL connector.

@michaelfreund
Copy link

michaelfreund commented Jan 25, 2017

Downgraded loopback-connector-mongodb to 1.15.1 but context is still null when passing access token. I can confirm that patch loopbackio/loopback-connector-mongodb#275 is fixing the issue but agree with @bajtos that we should not monkey-patch loopback code. However everyone of us is fighting with project deadlines and we should put together a working solution for this asap.

Regarding @bajtos comment strongloop/loopback#1495 (comment) I was able to get context in middleware from req.loopbackContext with the following setup (loopback 2.38.0 and loopback-connector-mongodb 1.18.0)

server/middleware.config

{
  "initial": {
    "loopback-context#per-request": {},
  },
  "auth": {
    "loopback#token": {}
  },
  "auth:after": {
    "./middleware/context": {}
  }
}

server/middleware/context.js

'use strict';

const App = require('../server.js');

module.exports = function (options) {
	return function context (req, res, next) {
		if (
			!req.accessToken ||
			!req.accessToken.userId ||
			!req.loopbackContext
		) {
			return next();
		}

		var currentContext = req.loopbackContext;

		// ...
	};
};

@bajtos could you please explain the difference in this scenario? Why is req.loopbackContext returning context object and LoopBackContext.getCurrentContext null?

The whole context thing is a big issue and related information is spread across github, loopback docs, stackoverflow, etc making it difficult for users to understand what's going on behind the scenes and what are the risks of using current context.

@bajtos maybe the context documentation can be improved and things put together?

UPDATE

Although context is available with req.loopbackContext in middleware you can not use currentContext.set() since continuation-local-storage is reporting No context available. ns.run() or ns.bind() must be called first..

@bajtos
Copy link
Member Author

bajtos commented Feb 7, 2017

Why is req.loopbackContext returning context object and LoopBackContext.getCurrentContext null?

IIRC, req.loopbackContext points to CLS namespace object that may or may not be active. getCurrentContext returns null when the namespace is not active. req.loopbackContext methods throw an error when you try to get/set values and the namespace is not active.

smartmouse added a commit to loopbackio/loopback.io that referenced this issue Feb 7, 2017
connect to strongloop/loopback-context#21

fix the wrong url link of loopback-connector-db2
smartmouse added a commit to loopbackio/loopback.io that referenced this issue Feb 7, 2017
connect to strongloop/loopback-context#21

replace loopback-connector-db2z to loopback-connector-db2 in db2 table
smartmouse added a commit to loopbackio/loopback.io that referenced this issue Feb 7, 2017
connect to strongloop/loopback-context#21

fix loopback-connector-db2 link
@smartmouse smartmouse self-assigned this Feb 7, 2017
@smartmouse smartmouse added the #wip label Feb 7, 2017
@smartmouse smartmouse removed their assignment Feb 9, 2017
@smartmouse smartmouse removed the #wip label Feb 9, 2017
@josieusa
Copy link
Collaborator

josieusa commented Feb 23, 2017

@Jeff-Lewis Unfortunately, your solution with cls-bluebird and cls-hooked does not work for me.
As I said, I already had reproduced the issue using authtest repo from @beornharris.
Also, I just noticed that I was using the native Promise (not es6-promise).
If it doesn't work with the native Promise, then I think there is no Promise implementation that could make it work with this approach. (Neither cls-bluebird).
I could invent another approach, if only I found the point(s) where the context gets lost in the mongo (?) package... but still no luck.
EDIT In strongloop/loopback-connector-mongodb PR 275 someone investigated another approach, but it wasn't considered to be a proper solution.
I'm going to take inspiration from it.

@josieusa
Copy link
Collaborator

josieusa commented Feb 23, 2017

I think that the PR loopback-connector-mongodb 275 is, by chance, a similar idea to the one demoed in this snippet (which is for mongoose instead).
othiym23/node-continuation-local-storage#58 (comment)
Both, in fact, use Namespace.prototype.bind() function from cls API.
And I could find even more examples of this kind of approach in the issues in the cls repo.
Having solved (?) issue #17 right now thanks to this "magic" function 😄 I think that, in order to workaround this issue, bind() should be called somewhere (I bet there is more than one place where it could be put), and there's no other approach that comes to my mind.
So, I'm going to think about what could be the most elegant (or less ugly?) way and the most reasonable place to call bind() to fix this issue... an idea for a new PR.
I'm not sure that it'll end differently than PR 275 (which was rejected)...

@josieusa
Copy link
Collaborator

josieusa commented Mar 7, 2017

I came up with the following idea.
According to this repo, the fix is either this or it quite looks like this.
And this is the easy part.
The hard part is about where to put the fix.
The most logical place would be inside the loopback connector code.
But I have another idea.
We can monkey-patch Array.prototype.push() (and/or maybe other array functions?).
If the argument to push()(or whatever) is a function, and if the function has a property with a unique name like __loopbackContext, of type object, containing the context, then we could re-bind the function inside our new push() function, and then call the old push function.
This is slow, of course. 😞
Also, it is a bit radical. But the modification to the behavior of push() is really minimal.
Lastly, it doesn't work if a custom implementation of push() is used somewhere. An example of this is the deque package, which could be a sub-dependency of another dependency.
(The presence of deque is easy to test, anyway...)
But first of all I have to answer the following question: does this trick really work?

@mrfelton
Copy link

Same issue for us. Until this is fixed we are using the patch from loopbackio/loopback-connector-mongodb#275 which does resolve the issues.

I have created a new branch of the mongo connector based on the latest code from master with this patch applied, incase anyone else needs it in their projects: https://github.com/fullcube/loopback-connector-mongodb/tree/fc/bind-context-275

@mrfelton
Copy link

In my case, I tracked an issue with the context being lost when used with the mongo connector down to the https://github.com/fullcube/loopback-component-mq package. This package uses https://github.com/arobson/rabbot which in turn uses https://github.com/cujojs/when - which causes the context to become lost across promise chains.

The issue with https://github.com/cujojs/when is documented on the loopback-context readme.

I have patched Rabbot so that it uses bluebird instead of when.js. see arobson/rabbot#94

Additionally I have created a new release of loopback-component-mq that uses this updated version of Rabbot. see https://github.com/fullcube/loopback-component-mq/releases/tag/v2.2.4

This resolves many of the lost context issues that I was experiencing when used in conjunction with the mongo connector.

I'd suggest that people experience this issue check to see if any of your dependencies are using when.js under the hood.

@bajtos
Copy link
Member Author

bajtos commented Dec 14, 2017

Cross-posting @matt-landers comment from strongloop/loopback#1495 (comment) as it may be useful to other people too:

On node 8.9.1 with the MongoDB connector, getCurrentContext returns null if it is not set outside of the call to a model like:

req.app.models.User.findById(req.accessToken.userId, function (err: Error, user: any) {
    var lbc = loopbackContext.getCurrentContext({ bind: true });
    if (lbc) {
        lbc.set('currentUser', user);
    }
    next();
});

However, it does work when you move it outside:

var lbc = loopbackContext.getCurrentContext({ bind: true });
req.app.models.User.findById(req.accessToken.userId, function (err: Error, user: any) {
    if (lbc) {
        lbc.set('currentUser', user);
    }
    next();
});

You also must use the bind option or it will return null as well. The documentation on loopback-context will have you pulling your hair out because it will not work as written.

@bajtos
Copy link
Member Author

bajtos commented Dec 14, 2017

@matt-landers would you mind contributing a pull request to loopback-context to update the documentation with the information you found, so that the next person using loopback-context with MongoDB does not have to pull their hair out? :)

@pbalan
Copy link

pbalan commented Mar 8, 2018

Adding my issue here also for anyone who attempts to fix and needs a reproducible repo.
loopbackio/loopback-connector-mongodb#421

@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.

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

No branches or pull requests

9 participants