As in https://github.com/strongloop/loopback/issues/569 I was trying to set currentUser in context in order to retrieve it after in requests.
app.use(loopback.token()); // this calls getCurrentContext
app.use(loopback.context()); // the context is started here
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();
if (loopbackContext) loopbackContext.set('currentUser', user);
next();
});
});
the problem is that loopback.getCurrentContext() returns null so it will not be set.
Why does this happen?
I am using almost precisely the same code, also extracted from #569. It works okay for me. Where are you placing the code?
In my server.js.
I am using loopback 2.8.2, node 0.10
All the same on my side, sorry. If you post a more complete code snippet I can do a more thorough comparison, if you like.
Here it is: http://pastebin.com/kiHwaVeh but it's almost the default server.js, thank you.
Have I to set something in configuration somewhere?
There is no special configuration for it that I have done.
Can I suggest you do an npm upgrade on your project to ensure related modules are not at an incompatible version?
I did it, nothing changed.
This is the getCurrentContext() function in loopback, ns is created, but ns.active is null so it returns null.
function createContext(scope) {
// Make the namespace globally visible via the process.context property
process.context = process.context || {};
var ns = process.context[scope];
if (!ns) {
ns = cls.createNamespace(scope);
process.context[scope] = ns;
// Set up loopback.getCurrentContext()
loopback.getCurrentContext = function() {
return ns && ns.active ? ns : null;
};
chain(juggler);
chain(remoting);
}
return ns;
}
I really don't know why it doesn't work, it seems that everyone uses this code snippet
I have built you a very simple project which implements this routine. Everything seems to work fine. Perhaps you can use it to find what's broken in your project: http://goo.gl/CCrbgM
Thank you, you were very kind. I did the same creating a new loopback application and it works either!
How strange is that?
Now I'll put all old files in this projects and hope. Thank you again!
Could you try what happens if you link your User model to a MySQL datasource?
Because if I do this, it returns null to me
I'm not set up right now with a MySQL datasource, but can try perhaps in a day or two. Sounds like you may have found the difference, though.
Finally, but it costed me one day.
Here is your example with MySQL configured: https://www.dropbox.com/s/uobkp2x8bg610o6/contextTestBugged.zip?dl=0
I hope that MySQL is the cause (and if it is that someone will fix it!)
Just a suggestion, I did not test it, but could you try
app.use(loopback.context());
app.use(loopback.token());
rather than
app.use(loopback.token());
app.use(loopback.context());
@diasalvatore Your test project w/ a MySQL datastore indeed does fail for me also.
This is most likely a duplicate of #809. There is a pending pull request #885 to fix the problem.
The bug should have been fixed by #809 in [email protected], I am closing this issue as resolved. Please leave a comment if the problem is still present in the latest loopback version.
I have installed [email protected] but still gets currentUser: null , as mentioned below
var ctx1 = loopback.getCurrentContext();
var currentUser = ctx1 && ctx1.get('currentUser');
console.log('currentUser: ', ctx1);
console.log('currentUser.username: ', currentUser);
//console.log('ctx: ', ctx); // voila!
if (ctx.instance) {
ctx.instance.updated = new Date();
} else {
ctx.data.updated = new Date();
}
next();
@viveke10 please provide an app that we can use to reproduce the problem locally on our machines, see wiki instructions.
I cannot provide you app because its client project of our company
@viveke10 Have you read the instructions? We are not asking you to disclose your proprietary code, but to create a new small app that reproduces the problem.
- Fork loopback-sandbox
- Add the code required to reproduce your issue in the forked repository.
Before trying the sandbox, here a report I got same (it seems) problem, with MongoDB though, even as I have the latest versions everything, with supposed fix for MongoDB.
loopback.getCurrentContext() returning null because its ns.active is null .
I found in a beforeRemoteMyMethod method a loopback.getCurrentContext() still works. It is only once it gets to the corresponding myMethod, a custom method I wrote, that it is null.
Further I have checked, it isn't the namespace it is supposed to be. I am looking in the debugger where it gets to node_modules/loopback/server/current-context.js at method loopback.createContext and am doing a debugger console test with ns.get('mystuff') and what was there before isn't there. That seems to indicate not only that it isn't active, but it isn't the same namespace that it was in my beforeRemoteMyMethod.
Got current versions couple days ago, to be sure did slc update and npm update . Then from npm list | grep loopback I have
โโโฌ [email protected]
โ โโโ [email protected]
โ โโโ [email protected]
โโโฌ [email protected]
โโโฌ [email protected]
โโโฌ [email protected]
โ โโโ [email protected]
โโโฌ [email protected]
โ โโโ [email protected]
โโโฌ [email protected]
node --version gives v0.12.3, but was same with v0.12.2.
Have a reproducible case at
https://github.com/srguiwiz/loopback-sandbox/commit/5349520438015b7f8de55b4c5f21604e1067da30
which is on this branch
https://github.com/srguiwiz/loopback-sandbox/commits/no-loopback-context-leo
Before that commit it was good, after that commit it wasn't.
The extra .then appears to trigger this defect, from this point of view.
I got to the reproducible case with memory db. This is equivalent to our application with a real database.
For that reproducible case at
https://github.com/srguiwiz/loopback-sandbox/commit/5349520438015b7f8de55b4c5f21604e1067da30
a workaround by using bluebird promises instead of Node.js 0.12.3 native promises has been committed
https://github.com/srguiwiz/loopback-sandbox/commit/89393a634434ec985a6055f04c962b63e4f22c9a
next on the same branch
https://github.com/srguiwiz/loopback-sandbox/commits/no-loopback-context-leo
@srguiwiz thank you for the update. I am curious, does the same problem occur when running on io.js 2.1? It ships a much newer version of V8 which may have fixed promise-related issues like this one.
The promise-related problem should be supposedly fixed by https://github.com/othiym23/async-listener/commit/25f30d2bce7d3ef0a59aa8d3f69d39bedab824f0 and https://github.com/othiym23/async-listener/commit/fad6f737d72320e4cb638a4a9b9adfb0949be833, the latter was landed 6 days ago. I used your steps to reproduce @srguiwiz and got the following console log:
the color before is red
about to call next() from before remote method
got into remote method
loopback.getCurrentContext() returns a value
the color is red
interestingly on statement after next() in before remote method
That makes me believe the problem is fixed for you. Could you please confirm, @srguiwiz?
FWIW, I also believe this is a different problem than the MySQL-related issue described by @diasalvatore's in the issue description.
@bajtos Confirming, yes, the console log you are showing indicates the problem I experienced (certain promises code caused loopback.getCurrentContext() to return null) is fixed. Thank you.
Hello @bajtos @srguiwiz ,
Has this issue been fixed? The issue is still keep opening. I still have the issue with the DB as MySQL, the active of the Context is null. But it works with memory DB.
Thanks,
Yong
@bajtos
I just found that it's the passport got the active of current context being null. I've update the sandbox at https://github.com/yuanyong/loopback-sandbox. It can reproduce this issue.
@bajtos
I could fix it by removing:
app.middleware('auth', loopback.token({
model: app.models.AccessToken
}));
While it works with:
app.use(loopback.token());
Some one have the same issue as I got:
https://github.com/strongloop/loopback-example-passport/issues/41
@yuanyong in your sandbox https://github.com/yuanyong/loopback-sandbox
app.use(loopback.context());
app.use(loopback.token());
or define it in middleware.json
"session:after": {
"loopback#context": {},
"loopback#token": {}
}
p.s. Please update loopback to newest version 2.19.0. Because the previous version(e.g. 2.18.0) will give wrong currentContext in callback function if login a different user in another browser.
with the latest version of everything, this no longer works in before save hooks.
it used to work, no longer does. .getCurrentContext in these hooks always returns null
+1
+1
with postgresql backend
using loopback 2.25.0
loopback-boot 2.14.0
loopback-connector-postgresql 2.3.0
loopback-datasource-juggler 2.42.0
looback.getCurrentContext() returns null inside a before save operation hook
+1
UPDATE:
We detected that for us, it is due to the node version.
I was developing locally on 0.12.7 - and it was working.
Only when deploying to modulus, which was using per default 5.0.0, we found out about the problem.
So we could reproduce this issue in 5.0.0 and 5.1.0.
For the time being our workaround is to revert back to v0.12.7.
Please let us know if you see any problems with this and/or if you have any idea about a bug fix.
Note: we could contribute to the bug fix if we'd know what to look for. Thanks.
@bajtos seems like this issue has been unfixed...
I have a Todo model where I call a "before save" hook:
Todo.observe('before save', linkUserSetTimestamps);
In the linkUserSetTimestamps method I try and set the context:
var context = loopback.getCurrentContext();
Further more, the "save" triggering this "before save" hook is a custom remote method where I do a couple of things using a MySQL transaction:
async.auto({
transaction: function(callback) {
Todo.beginTransaction({ isolationLevel: Todo.Transaction.READ_COMMITTED, timeout: 10000 }, callback);
},
todo: ['transaction', function(callback, self) {
if (!self.transaction) return callback(new Error('No transaction for todo in todo assign.'));
Todo.upsert(todo, { transaction: self.transaction }, callback); // this will trigger "before save"
}]...
I don't know if this has any relevance in the issue, but it's worth a mention.
The value of context is null. This is quite a big problem...
Any ideas on what's causing it / how to fix it?
I didn't have the issue on my development machine, only in production.
Even though both obviously use the same package.json and I ran npm update on both, it still only worked on dev not prod.
Then I thought I'd just give this a try, copied from node_modules on dev to prod:
loopback (2.25.0)
loopback-connector-mysql (2.2.0)
loopback-datasource-juggler (2.43.0)
After that it's now also working on production machine.
I have no idea idea which fixed it. Also don't know why the versions of both weren't exactly the same as I had run the npm update on both machines.
What is obvious though is that there is some bug somewhere that was reintroduced. Since these are all the newest versions there must be some dependency or something that is has introduced the issue?
@fablife thank you for pinning down the issue to different Node version. Would you mind checking whether the context works on v0.10 and v4.x, to give us a full picture?
@EmileSpecs what is your Node.js vesrion on dev and production? Could the version difference explain the problem?
@bajtos, both use v.4.2.2, it was the first thing I checked to make sure it couldn't be that.
Somewhere some dependency or combination thereof is causing this issue, it's one of those that is probably going to be a irritation to find...
I can confirm we have the same issue as @EmileSpecs .
We found out about this problem only after deploying to modulus (http://modulus.io).
I have been developing locally on my machine, and locally it works even with node 5.1.0, 4.2.3, 0.12.7, 0.10.38
The problem appears when we deploy to modulus. There, 5.1.0 and 4.2.3 produce the nullcontext, and it only works there if we use 0.12.7 (didn't try any other version there).
So this seems to not be related to the node version, but could be some speed/timing issue. Fyi, I am developing locally on a laptop with SSD storage, speculating that it may be related to I/O speed?
Hey i face the same issue. It works ok when i run locally (OS X). But when i try running it on my ubuntu digital ocean server it gives a 500 error saying "Cannot call method 'set' of null" where in I am not able to set the value. The code base is same. Not able to understand how to proceed.
Same issue here.
In my middleware "routes:before" the context is ALWAYS null.
In my operation hooks the context state is completely random. 25% of the time it's gonna crash saying the context is null, 75% of the time the context will be there, but without the access token.
This bug seems really old, anyone found a workaround? I am completely stuck right now as I use loopback in a commercial project, and i need to access the user token in my hooks.
On OSX Yosemite, MongoDB v3.2.0, node v5.3.0, using ES6 (which requires strict mode).
Edit:
I now use .afterRemote('create', (ctx, obj, next) => {}); instead of .observe('before save', (ctx, next) => {}) and I have access to ctx.req (which was undefined in the observe) so I can bypass loopback.getCurrentContext().
Same for me. Its not happening always. For one request, context is available. For the next, its not available. Same cycle repeats. I am using postgres connector.
Of-course, I was setting a value in context from a then callback. Making it normal node style callback fixed the issue.
I am having the same issue ...I keep loosing CurrentContext every now and then....don't know what to do
Whenever do slc run...I get the context but when i try slc debug.....It gives me null.....Its funny how it is possible
I am getting this as well. app.loopback.getCurrentContext() is randomly (most of the time, but occasionally not) null in a app.dataSources.service.connector.remotes.before("*", function(...)) callback function. The remote call is done after saving a model to mysql in a transaction.
This is on loopback 2.26.0, node.js 4.2.2.
+1
This issue is actually driving me nutsโฆ
I need to use a hook on access but it seems that i can only use it with an observe. And the only way to have access to the request object in observe is to use getCurrentContext()(the "local" ctx is pretty much empty). But then getCurrentContext() inside access returns null when my main hook is updateAttributes (i use access to check the permissions of the user, it works well with all the other hooksโฆ).
Anyone here managed to use access without observe (like with a beforeRemote or something)?
@Nivl Can you post the an .observe that you have implemented?
I managed to avoid the problems with the context being null by wrapping some functions in the node-mysql library using npm install cls-mysql and adding the following at the very start of server.js
//THIS MUST BE REQUIRED FIRST TO AVOID PROBLEMS WITH THINGS SAVING FUNCTIONS THAT GET SWAPPED OUT - the rest connector was causing problems due to this not being here.
require('continuation-local-storage');
var loopback = require('loopback');
/* Patch MySQL driver and other libraries to work with continuation local storage properly */
var patchMySQLWithContext = require('cls-mysql');
var loopbackContext = loopback.createContext('loopback');
patchMySQLWithContext(loopbackContext);
/* END Patch*/
@Sandy1088 Here's my access (or most of it). Basically, either it works perfectly, or it just crashes on appContext.get('http') because appContext is null.
Group.observe('access', (ctx, next) => {
const appContext = loopback.getCurrentContext();
const httpData = appContext.get('http');
const req = httpData.req;
const groupId = ctx.query.where.id;
const accessToken = new AccessTokenHandler(req.accessToken); // Custom wrapper to avoid side effects with the userId that needs to be called with toString(); (because it's an Object and not a String)
if (accessToken) {
// I have here a request that fetch the user permissions for the group "groupId", and set them in the var permissions. (it also throw a 403 if the user is not a member of the group)
// I use req.customData to pass data to my remote hooks (so I can bypass loopback.getCurrentContext())
req.customData = {
accessToken,
permissions,
};
next();
} else {
// throw a 403
}
});
After upgrading Node from 0.12.9 to 4.2.4 I have hit this issue (I'm not using MySQL as a datasource but I have a feeling that the MySQL aspect of this is a red herring and should probably be removed from the title of this thread).
I am finding that after saving some data in the the current context in a Model's prototype method, attempting to retrieve it again in another model's before save hook implementation does not return the data. The exact same code works on Node 0.12.9.
I should have also noted that I am hitting this issue when calling a Model prototype method directly when running a mocha unit test.
I have noticed in the past that the loopback current context only ever get initialised when methods are called remotely.
I did in fact previously hit the same issue in Node 0.12.9, but was able to work around it by ensuring that the loopback context is initialised before any of the unit tests run. This is done using the following code:
/**
* Ensure that the current context is initialized before any unit tests run. This is needed because the loopback
* context appears to only be available when calling methods remotely. In our unit tests we call these methods
* directly and so we manually initialise the context first to ensure its available to our tests.
*/
beforeEach(function () {
var loopbackContext = loopback.getCurrentContext();
/**
* Reset the current context before each test.
* This is required because when running the test suite, mocha appears to persist the current context
* across all tests in the suite whish isn't the right thing to do. Each test should have a fresh loopback context.
*/
if (loopbackContext) {
// FIXME: It would be better to completely reset the context, but I can't find a way to do that.
// so just clear all known used keys instead.
_.forEach(['contextOptions', 'PersonUpdated', 'PersonChanges', 'currentUser'], function (key) {
loopbackContext.set(key, undefined);
});
}
});
Unfortunately, this hack doesn't appear to work in Node 4.2.4 for some reason, so we are back to a very broken implementation of getCurrentContext.
Does anyone know any plugin to secure loopback api?
Ok so getCurrentContext() is actually about to get deprecated. The proposed alternative is not working for me so i ended up staying away of observe, and I created a beforeRemote hook for each of my routes to replace access. More code, less magic, less problem.
@Nivl can u share the beforeRemote that you have implemented?
and also I've built my logic around currentUser so I am not able to move further as its null every now and then.....please suggest
we are also experiencing this.. just recently upgraded node on our production servers to 5.3 and this issue started... i rolled back to 0.12.3 and the issues are now gone...
we'd obviously prefer to stick with 5.3 on production. any ideas on how to address this?
@stringbeans Like I said the function is now deprecated, so I think it's safe to say that no fixes are coming.
I see three solutions here:
@Nivl But Operation Hooks and Remote Hooks are 2 completely different things.
Is it right that a current context should only be accessible within methods only if those methods are called via a remote method?
We find it pretty common to call model methods directly (in code) and expect our methods be able to access the current context the same way as if the method were called remotely.
Whilst I understand the proposed workaround of injecting the context via options in remote hooks and passing it around manually (not very graceful, but if it works its better than nothing), this only works if you are calling into your methods remotely, so I'm not sure how it can be considered as a replacement solution for getCurrentContext which is possible to use regardless of wether a model method is called remotely or not. Am I missing something?
I am with @mrfelton
What about stuff created in boot and model scripts....
Also, isn't the official way of knowing the caller's user id via the accesstoken - by calling getCurrentContext().get("accessToken")?
I hope I am the one missing something here...
@mrfelton You're right, they are different, but you can still manage to replace Operation Hooks by Remote Hooks. That's definitely something one wants to avoid though, but it's a working solution (and it's the one I ended up choosing after spending way to much time trying to find a proper workaround).
@fablife Yup, it isโฆ
Unfortunately it seems that none of you guys are missing anythingโฆ The loopback team is working on loopback 3.0, let's hope they gonna find a new way to handle that. But since this bug is over a year old, I'm afraid we won't get anything for 2.xโฆ
@Nivl I am working with v0.12.4 ...Do I need to downgrade it further?
@Sandy1088 have you tried @mrfelton fix?
@Nivl Nope.... Sorry but I was not sure where should I put the that chunk of code? Can You help me with that?
I've a boot script which has this chunk
var loopback = require('loopback');
var boot = require('loopback-boot');
var app = module.exports = loopback();
app.start = function() {
// start the web server
return app.listen(function() {
app.emit('started');
var baseUrl = app.get('url').replace(/\/$/, '');
console.log('Web server listening at: %s', baseUrl);
if (app.get('loopback-component-explorer')) {
var explorerPath = app.get('loopback-component-explorer').mountPath;
console.log('Browse your REST API at %s%s', baseUrl, explorerPath);
}
});
};
app.use(loopback.context());
app.use(loopback.token());
app.use(function setCurrentUser(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.'));
}
var loopbackContext = loopback.getCurrentContext();
if (loopbackContext) {
loopbackContext.set('currentUser', user);
}
next();
});
});
// Bootstrap the application, configure models, datasources and middleware.
// Sub-apps like REST API are mounted via boot scripts.
boot(app, __dirname, function(err) {
if (err) throw err;
// start the server if `$ node server.js`
if (require.main === module)
app.start();
});
@Nivl @mrfelton @fablife I think it's indeed unacceptable to deprecate this feature and have us wait & pray for a 3.0 solution. Let's not forget, this feature has not been deprecated officially at all - there's an open issue for that, yes - but the documentation doesn't make any remarks or warnings regarding the risks involved.
If the LB team would've cared to resolve this, the solution should have been to transparently pass the context with options of all methods that accept this argument.
What's currently completely missing in the workaround (https://github.com/strongloop/loopback/issues/1495), is that AFAIK none of the core DAO methods (which actually do take options) have this declared in the removing setup within accepts - so hasOptions would not work on them I believe.
This is why app.remotes().methods().forEach(function(method) is called, but it's a gross over-generalization of the methods that actually support options. If you have any methods implemented yourself, or in the case of login (which others found) it will bomb out when you access it. It's also a potential security issue, because there might very well be options that you don't want to expose over remoting/REST at all.
At some point it was decided to add options to all DAO methods, but the implementation was only partially done in order to resolve the CLS issue.
Rather than adding options to all remote method definitions, there should be a way to manipulate options in a beforeRemote hook, which in turn should be passed to the method itself and as such, to any operation hooks as well.
FWIW I'm running a local setup on 4.2.3 fine, but I'm worried there will be issues running on a different platform or in production environment.
In short, not only was the choice of using CLS for getCurrentContext rather poor to begin with, still no warnings are officially given, and the workaround is a half-baked hack at most.
@ritch @raymondfeng @bajtos I'm sorry guys, but if you're not too busy dreaming up 3.0 (don't we all love a blank slate or second chance?), please help us fix the mess that was left.
@Nivl You can not completely replace Operation Hooks with Remote Hooks. They are totally different, fire at totally different times, and you can not use purely Remote Hooks to achieve the same as you can achieve with Operation Hooks
You are assuming that all code is run via remote methods, which is often not the case - boot scripts, migration scripts, cli interface, any other code that bootstraps the app manually and expects to be able to call into it's model methods - None of these trigger Remote Hooks but do trigger Operation Hooks.
The fact that this bug is a year old is all the more reason it needs to be addressed asap. As pointed out by @fabien the method has not been depreciated and if it were to be depreciated that should not happen until there is a comparable solution available. The suggested workaround mentioned in #1495 is not a viable alternative for many use cases.
@mrfelton indeed you are right, I didn't think about the non-remote action, my bad.
From what I have seen on other projects, the older a bug gets, the less likely it is to be fixed. And since they have been talking about deprecating the feature for a while now, and that the development of the next version has started, the probability of having a fix pretty much drops near 0.
Let's hope they will have some time to put into that issue.
Totally agree with @fabien. And yet - is it really not possible that this get fixed? What is the status of the debugging of the issue? Is there a clear sample project + environment (node etc) that can be used to reproduce it?
@doublemarked ive been trying since yesterday to try to reproduce this locally as well as on an ec2 instance.
I've been trying with:
"loopback": "2.26.2",
"loopback-boot": "2.16.0",
"loopback-connector-mysql": "2.2.0",
"loopback-datasource-juggler": "2.44.0"
with Node v5.3.0 (via nvm), a remote mysql database (ie. not localhost), and NODE_ENV=production
and no luck :-1:
Anyone have repro steps that work?
@stringbeans that's interesting...earlier you wrote that you were experiencing the problem, right? What changed? Different EC2 instance, or something else?
@doublemarked still trying to figure that out :S
i created an AMI from the affected ec2 instances and then created a new ec2 instance off that AMI, so the environment should be exactly the same.... will report back if we can figure out whats going on here
Ok guys, I created a minimal repo that contains crashes: https://github.com/Nivl/loopback-context-bugs
Right now I just added one crash on a middleware (so a simple post on /User should trigger the crash).
I added a test case (npm test), but for an unknown reason the crash does not occur when using mocha, so I also added a list o bugs in the readme.
If you experience a bug on your personal projets, but all my listed issues are working fine for you, please send me a pull request with an other test case (so we can cover as much as different scenario as possible).
@mrfelton agreed, operation hooks are completely different from remote hooks. However, it's probably easier to pass along the context directly to a (operation-hook enabled) method with options in a non-remoting setup, than through any of the core remoting methods.
I'm sure there are other setups where the currentContext comes into play, but I think the most pressing need comes from things like being able to retrieve the accessToken (or other per-request data) in such an operation hook.
@Nivl are you sure the currentContext is actually registered at this point? IIRC it's quite possible for the currentContext to not be available in middleware, if it's set to run before https://github.com/strongloop/loopback/blob/master/server/middleware/context.js has been executed.
I'm more worried about accessing loopback.getCurrentContext().get('AccessToken'); inside an Operation Hook. Isn't that what this thread is about?
@doublemarked it should be possible to get it fixed, but from what I understand is that there might be one or more external dependencies that turn out to be problematic with CLS. This is why you'll find some CLS-specific packages on NPM. And it might explain why it appears that on some setups an app works, while it fails on another; somewhat loosely defined dependencies (think npm's default: ~) might be enough to get slight changes from setup to setup.
I suspect that LB + MySQL is such a combo, for example, given the trouble people seem to be having. The async library had similar issues, as do some Promise implementations. Which is not all that strange, considering how CLS works.
@fabien It's also used a lot in middlewares to (for exemple) retrieve the current user and set it into the context. They have a code in the documentation that does that (see server/server.js).
I'll be adding more tests soon. This one was an easy one, so i put it first.
@Nivl agreed - just wondering if the context.js middleware is active at this stage. You should make sure it is, otherwise it could be a false-positive I believe.
@fabien, You are right, the middleware was executed before context.js, as the context middleware is called in the "routes" phase.
I will update my repo later today with an operation hook test case.
Ok so I unfortunately didn't manage to reproduce the bug on a clean projet, but on my commercial projet, I managed to trim it down to this:
var loopback = require('loopback')
function dummy() {
return new Promise(function dummyPromise(resolve, reject) {
resolve();
});
}
module.exports = function(Item) {
Item.observe('access', function access(ctx, next) {
dummy()
.then(function promiseResolved() {
if (loopback.getCurrentContext()) {
next();
} else {
next(new Error("context is null in access"));
}
});
});
Item.observe('before save', function access(ctx, next) {
if (ctx.isNewInstance) {
next();
} else {
if (loopback.getCurrentContext()) {
next();
} else {
next(new Error("context is null in beforeSave"));
}
}
})
};
This gives me the error context is null in beforeSave when doing a PUT /items/{id}. What causes the issue here is the promise, if I remove the dummy promise it works. I removed all promises from my commercial project and I have no more context issues (well at least for now).
If I use the promises from bluebird instead of the one from V8 by adding const Promise = require(bluebird); on top of my script, everything works fine and the context is not longer nullโฆ
Hopefully someone will make sense of all of this.
@nivl when getContext is null, what does global.Promise point to? Is it a "wrappedPromise" from CLS?
@Jeff-Lewis In both cases it seems that global.promise points to a "wrappedPromise".
@Nivl it's good to hear you were able to pinpoint the issue. They way CLS works means that none of your methods should drop the ball - which might happen when the callback is wrapped in a Promise.
I think Bluebird already does what this older CLS fix handles: https://github.com/TimBeyer/cls-bluebird - but apparently, native Promises don't work like that yet.
It could explain why people are starting to see issues upwards of Node v.0.12 - IIRC native Promises were enabled by default since Node v.4.x.
Since I haven't run into the issue myself on Node v.4.x and don't use Promises in currentContext-enabled code, I'm starting to wonder about some of the issue reports which suggest that currentContext is null happening at random ("In my operation hooks the context state is completely random." above).
The way I took this was: random per request, so the _same_ request would sometimes work and sometimes not. @Nivl is this what you actually meant?
@fabien Yes. I actually had the three different scenarios:
1) The same request always works.
2) The same request always work fails.
3) The same requests sometimes works, sometimes fails.
@fabien I'm neither using bluebird nor promises but with callbacks I keep loosing and getting the context
@Nivl hmmm ... that's discouraging ... any of this still true when using Bluebird?
@Sandy1088 can you post a gist of the code where this happens (at random, apparently)?
@fabien I haven't had any issues since I stopped using the native Promises actually. I'm starting to put back operation hooks and getCurrentContext everywhere in my app. If I find any new issue I will report it, but as far as I'm concerned, everything seems to be working well now.
@Nivl great! Perhaps this fix (which actually was meant for a Polyfill of ES6 Promises) applies to Node too? https://github.com/building5/cls-es6-promise/blob/master/index.js
@fabien @Nivl Not sure what am I missing but my code looks like this
//Members.js
Members.getMember = function(Id, cb) {
var ctx = loopback.getCurrentContext();
var currentUser = ctx && ctx.get('currentUser');
var realms = [];
realms = currentUser.realm.split(",");
var fltr = {
"where": {
"id": Id,
"center": {
"inq": realms
}
}
};
Members.find(fltr, function(err, res) {
console.log(err,res);
});
};
//boot script
var loopback = require('loopback');
var boot = require('loopback-boot');
var app = module.exports = loopback();
app.start = function() {
// start the web server
return app.listen(function() {
app.emit('started');
var baseUrl = app.get('url').replace(/\/$/, '');
console.log('Web server listening at: %s', baseUrl);
if (app.get('loopback-component-explorer')) {
var explorerPath = app.get('loopback-component-explorer').mountPath;
console.log('Browse your REST API at %s%s', baseUrl, explorerPath);
}
});
};
app.use(loopback.context());
app.use(loopback.token());
app.use(function setCurrentUser(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.'));
}
var loopbackContext = loopback.getCurrentContext();
if (loopbackContext) {
loopbackContext.set('currentUser', user);
}
next();
});
});
// Bootstrap the application, configure models, datasources and middleware.
// Sub-apps like REST API are mounted via boot scripts.
boot(app, __dirname, function(err) {
if (err) throw err;
// start the server if `$ node server.js`
if (require.main === module)
app.start();
});
@fabien I tried to play with it a bit, and it didn't seem to fix the issue (but then I might have used it wrong).
@Sandy1088, you said:
Whenever do slc run...I get the context but when i try slc debug.....It gives me null.....Its funny how it is possible
Is it still what happens here? Can you be very precise about the issue you have? When does it happen, Is it in setCurrentUser or getMember? What is getMember, and operation Hook?
I had assumed it but later I realized it happens randomly and had nothing to do with slc run or slc debug. And I have Members model belonging to realms under User model of loopback. And I check if the current logged in User belongs to the realm to access Members list but when getMember() executes sometimes ctx is there or it simple says that its null.....
Also in boot script, I get the currentContext but when it comes to Member.js......it shows as null randomly and sometimes is available
@Sandy1088 Hum, I have never used the realm and I don't know when and where getMember is called so I can't really help here. Can you create a new loopback project from scratch, try to reproduce the bug (with as few code as possible), and put it in a Github repo (so we can clone it and try it)?
Okkay I can do that soon....Thanks
@Nivl Find the repo here...Kindly add a User to produce currentUser via swagger....And let me know if you find any difficulty in comprehending the code.....Thanks Again
//Given function in repo sometimes gives currentUser and sometimes does not....Please ignore other functions
Members.getMember=function (id,cb) {
var ctx = loopback.getCurrentContext();
var currentUser = ctx && ctx.get('currentUser');
console.log(currentUser);
Members.find({"where":{MemberId:id}},function (mem_err,mem_res) {
if (!mem_err)
{
cb(mem_err,mem_res);
}
})
}
@Sandy1088 code worked fine for me on node v4.2.4 and v5.3.0. It is difficult to work with the sample like this without knowing more about your environment. It would also help if the sample project were more distilled.
To anybody else - if there already exists a project which can be used to reproduce the problem, please provide a link and details to reproduce?
@Sandy1088 regarding your environment - can you please:
1) Tell us your exact node and OS version?
2) Shrinkwrap your node modules? Or provide a zip of your project including node_modules
Regarding project distillation - can you please make the following changes to your project, ensuring that you can reproduce the error at the end? After the changes, calling getMember should print an incrementing value on the console. If you cannot reproduce it after these changes (even when creating a user and authenticating), roll back the changes until the point that they are again reproducible, and then explain in detail what causes the shift.
// Add this new middleware to server.js:
var reqIndex = 0;
app.use(function setContextMarker(req, res, next) {
var loopbackContext = loopback.getCurrentContext();
if (loopbackContext) {
loopbackContext.set('marker', reqIndex++);
} else {
console.error('Missing loopbackContext in middleware from server.js');
}
next();
});
// Make the top of your Member.getMember function look like this:
Members.getMember=function (id,cb) {
var ctx = loopback.getCurrentContext();
if (ctx) {
console.log('Request Marker:', ctx.get('marker'));
} else {
console.error('Members.getMember missing Loopback Context!');
}
// whatever else
};
After those changes, hitting /api/Members/getMember?id=whatever should produce a message to the console without need for user or authentication. Actually there's a lot more distillation we can do than that, but this will be simple enough.
If you're having trouble reproducing this @Sandy1088 after the changes above, please drop me a PM or find me on Gitter (https://gitter.im/strongloop/loopback) and we can walk through it to try to distill a project which can be used to reliably reproduce the problem.
The issue of CLS losing context has little to do with Loopback, but rather the Async and Promise implementations used by Loopback and our apps. The order in which libs are required makes a BIG difference. That's why slc run might work but slc debug might not.
See how DylanSale resolved it for him with MySQL and what all of the cls-lib modules have to do fix other libraries.
Because the V8 engine does not provide a way to trace async call stack (yet hopefully), we in user-land have to hack and Shim libraries and try to keep track the async call stack which is rather difficult, especially when async and promise implementations use process.nextTick() without informing CLS.
Try your test in a way that ensures you require("continuous-local-storage") before ANYTHING else. Then make sure your all dependencies of all npm modules are using a cls wrapped version of Async, including bluebirds own asyn internal implementation.
This will make it work, but it's difficult to maintain and is a pain in the ass. That's why many of us struggle with it.
See the NG-Tracing group and @trevnorris's work on async-wrap for future developments of this.
@Jeff-Lewis That's interesting and makes sense. If that's the case, it does give credence to the call to deprecate use of CLS, though still leaves LB users without a clear alternative.
@all Interesting developments - I think the main take-away is that, while we can (and probably should) deprecate the use of CLS, the functionality it offered (currentContext) should definitely be preserved going forward.
We're working to get AsyncWrap stable enough to be considered public API in the next couple months. Though we're still hosed in regards to the native Promise implementation. Nothing we can do ATM to observe what's going on.
I just want keep the currentUser in the context and use it for my business logic. So I use loopback.createContext() and then I add currentUser to the created context but I am not sure how will I access it in another js file. Any Ideas?
Could this issue affect loopback's implementation of checking whether a user in authenticated via the ACL? ie. with principalId: "$authenticated"
Just got the issue again. Updating a model in a RemoteMethod was causing the getCurrentContext in the model's access to be null.
I fixed the issue byโฆ removing newrelic (see #1984 )
Should everyone be avoiding using node above v0.12.* for right now with loopback?
I have got this working on newer versions of Node, including the latest 5.9.1 - but only on certain machines - for instance my dev machine it works fine, on my colleague's it has this same problem. However, reverting to 0.12.13 version of Node it seems to work reliably across machines and servers. Obviously this is annoying as this is quite an old version of Node. Is there any update on this?
Just ran into this again with 5.10.1... the issue is very intermittent
If this is currently an intermittent issue why don't we change the documentation to warn this is currently not working. I have to spend some hours before I noticed it was a problem in the framework not in my own code. Also as it's intermittent it could led people to release their software with a piece of code that will be affecting in a bad manner their production software. This is unacceptable.
We should add a warning right here: https://docs.strongloop.com/display/public/LB/Using+current+context
I am seeing this issue when using promises. Every call that is made to getCurrentContext from a promise completion is causing it to be null. Not just before save, access is also returning null. Possibly others as well. I have switched to callback and is working fine. Tried with node v4.4.5 and v4.4.7 LTS versions
Never had this issue. Changed nothing but my version of node from 0.10 to 4.4.7 and it started. Using callbacks with async lib, not promises.
I added require('continuation-local-storage'); to the top of server.js and the issue seems to have gone away, though the fact that it happens intermittently makes me question the results. This issue, however, boosts my confidence in it as a fix: https://github.com/othiym23/node-continuation-local-storage/issues/55
Have had this issue intermittently and can't seem to reproduce it consistently on any environment, but it's enough to discourage usage of getCurrentContext.
I'm only really using the loopback context to propagate user authentication details to a couple of dynamic role resolvers. I can consistently get around this issue by directly accessing the remotingContext in the role resolvers, although this isn't ideal as pointed out by #1495.
This means that the implementation won't be transport agnostic, but for now at least the issue seems to have disappeared completely.
Having the same issue while using promises with Transactions. Switched to callbacks and it worked. Same issue appeared in an after save hook where I used callbacks though. Turns out the issue was cause I used a promise in a asyncValidator and that messed up the after save (even though I didn't use any promises in there).
Hello. The original getCurrentContext implementation was based on continuation-local-storage that's know to loose the context in many situations. We decided to deprecate this API in favour of an explicit propagation of context via the "options" argument, see https://github.com/strongloop/loopback/issues/1495 and the related pull requests.
In the meantime, a new version [email protected] offers a different implementation of getCurrentContext, this one is based on cls-hooked that should work better on Node 4 and newer. Feel free to give it a try. If you run into any issues with loopback-context, then please report them in loopback-context's repository on GitHub.
Most helpful comment
If this is currently an intermittent issue why don't we change the documentation to warn this is currently not working. I have to spend some hours before I noticed it was a problem in the framework not in my own code. Also as it's intermittent it could led people to release their software with a piece of code that will be affecting in a bad manner their production software. This is unacceptable.
We should add a warning right here: https://docs.strongloop.com/display/public/LB/Using+current+context