How to use cls-hooked with loopback-context?

[ahmetcetin]

I'm using Node 6, and I rely on loopback-context module, actually just switched to it as per recommendation in the recent warnings, previously I was using loopback.getCurrentContext(). I read the issues on continuation-local-storage, and this package looks still relying on it, but it's mentioned in readme that it's suggested to use cls-hooked instead for Node 6 users. Can you suggest how can I use cls-hooked with loopback-context? Is it enough to fork and just replace continuation-local-storage with cls-hooked in require calls?

[josieusa]

EDIT: if you are here, I guess that you already read strongloop/loopback#1495 for an alternative approach than loopback-context.
..
First of all, in order to solve your problem with getCurrentContext(), I suggest you to use npm shrinkwrap especially in production. I'm going to add that to the README. But before that, you absolutely need to be aware that, every time you run npm install right before a shrinkwrap in production, getCurrentContext() may break as a result of running install (it also depends on which package versions are resolved by npm install), and this is also hard to notice.
So it's not easy to test if npm install right before the shrinkwrap resulted in a working install or not.
This is why I'm working on my own fork, which is still a work in progress.
I already forked the repo and replaced the module, but it's not enough, this is a work in progress. In fact there are cases when cls-hooked fails too. For example, when you require the async package before cls-hooked, depending on package version numbers, getCurrentContext() may break. It's even reproducible in a unit test.
That said, one pro of cls-hooked over continuation-local-storage is that it uses the brand-new native Node AsyncWrap; this is why my fork currently uses it, but in the future I hope that the mantainer of cls-hooked will send a pull request to continuation-local-storage, so that the change is not needed anymore.
Since it's not enough, I'm going to make more commits, so please follow PR #2 . I don't know if there are other cases where cls-hooked fails; but for the case mentioned above, which is common anyway (see the issues on Loopback), my branch will introduce a helpful early warning. Neither this is enough, but if you are using npm shrinkwrap in production, the combination of the two approaches makes it all not a nightmare anymore to mantain.
However, if you don't use npm shrinkwrap, my branch doesn't really help to improve the quality of your production apps (by now).
More comprehensive solutions are possible (I think), if you hack Node require, but I consider that heavier and harder to implement than a warning.
Since I already use shrinkwrap as a best practice, this will be enough for me, but your mileage may vary.

[Jeff-Lewis]

@josieusa since cls-hooked uses the "unofficial" and undocumented AsyncWrap node class, I'm hesitant to put it in CLS. I think the plan is when AsyncWrap becomes official, it will hopefully be put in CLS. Until then, cls-hooked is available for people to test and see if there is an improvement over async-wrap which was the best alternative until AsyncWrap.

Also, your note about cls-hooked being required early in the app life cycle is an important one. I'll add something to the cls-hoooked readme.

[ahmetcetin]

@josieusa thx for the info, i just can't do shrinkwrap, as it gives maximum call stack size exceeded error, apparently it happens if there is circular dependencies. It's fixed in 3.10.7 it looks (npm/npm#13327) but sill not available to download. I tried to install it using npm@next but no luck. I'll do when it becomes available. Anyways, I'm using getCurrentContext in few places in my project, and so far I have no problem yet, running on two beanstalk instances on aws for staging and production. We're using mysql in storage layer.

Our project is used internally in our company still, but we plan to rollout to public in November. So, I tend to keep my code as it is until AsyncWrap becomes official, or until having problem with getCurrentContext, though I must tell using a not reliable lib scares me a lot. Although I didn't like much the proposal in strongloop/loopback#1495, but it looks I might need to try it out. Any thoughts?

[sompylasar]

[email protected] is available as explicit version number npm i -g [email protected], have installed it successfully about 5 hours ago (with the stack overflow bug verified fixed).

[Qard]

@Jeff-Lewis I'd encourage you to open a PR to CLS at least. We can leave it open until async_wrap lands officially but work toward readiness, so CLS is ready to go immediately when async_wrap lands. We'd still need a polyfill though, so either async_wrap would need a polyfill or we'd need to fallback to the async-listener module it uses currently, when async_wrap is unavailable.

[Jeff-Lewis]

@Qard That sounds good. I'd be happy to.

[josieusa]

@ahmetcetin does the last commit to PR #11 help solve your issue? Let me know!

[bajtos]

We are working on a new semver-major version of loopback-context that will use cls-hooked under the hood. The change has been already implemented by #11. I am closing this issue as done.