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

The context on observers is null or void #2597

Closed
desaroger opened this issue Aug 9, 2016 · 7 comments
Closed

The context on observers is null or void #2597

desaroger opened this issue Aug 9, 2016 · 7 comments
Assignees
Labels

Comments

@desaroger
Copy link

desaroger commented Aug 9, 2016

Overview

I have a project where I use mongodb, promises and generator functions (with Q.async). For some reason sometimes I are unable to get the context. Sometimes is null, sometimes is a new void context that doesn't have the data we saved on the context.

Tried:

  • Patching Q library with cls-q
  • Changing connector with both mongodb or memory connector, same results on both.
  • Removing all promises and generator functions of observers, same results.

Sandbox repo

Here is a sandbox repo where you can see the issue.

Steps to reproduce the issue:

  1. Clone repo
  2. npm install
  3. npm start
  4. GET /api/Pokemons

Obtained Results

You will see something like this:

mongo find

There are logs on some middlewares and observers. There is two labels:

  • lc: Shows if loopback.getCurrentContext() returns not null.
  • data: Shows if there is on the loopback context the data we previously (on the middleware) saved on the context. If lc=true, data=false I understand that there is a context but it isn't the same context where we saved the data.

Understanding Results

As you can see, all middlewares have no problem on obtain the context and works as expected. But the observers doesn't have context. And the ones that have context I think is because the first observer to use a promise is creating a new context and the next observers on run have a new context.

I have other parallel context problem that sometimes make some functions to not have context, or have context only when a "READ" operation is requested. But on "WRITE" operations we don't have any problems. I only showed the above example because when I created the sandbox and saw no context on the observers, and I need to understand why before make the big question. I saw that this issue showed on the sandbox is independent of the connector, but in our random context problem seems to be something with the connectors, as on memory works fine but on mongo isn't.

Thanks in advance.
PD: This is my first issue on github and english isn't my first language, so sorry in advance if something is wrong with anything.

@jannyHou jannyHou self-assigned this Aug 9, 2016
@0candy
Copy link
Contributor

0candy commented Aug 9, 2016

There has been many issues with getCurrentContext and we have plans on deprecating it.
#1676
There is a workaround specified in that issue. See if that works for you.

@josieusa
Copy link

josieusa commented Aug 10, 2016

Also, follow the discussion in strongloop/loopback-context#2 for an alternative workaround which is less "official" (in fact, it keeps using the deprecated API), and it's going to work only in a subset of the issues, but requires less refactoring, and opens the way to future reimplementations of getCurrentContext() since it uses the native-ish (sort of) cls-hookedmodule.

@desaroger
Copy link
Author

Hi!

@0candy I applied the workaround and works... works. But extremely slow. And this not solve my other issue (#2603) I have. I want to change the mongoDB collection dynamically. As you can see on the issue, my try was to override a function of the mongodb connector. On this function sometimes I have context, and sometimes nope. The proposed workaround is fine but is not applicable to my issue related with mongo. I need the actual loopback context to work.

@josieusa, thanks for your workaround. As you can see on #2603, my problem is not full-solved with the proposal #1676 (besides the slowdown of this workaround has made to our api), so I will check yours to see if we can solve our issue with it. We are working on solve this issue, as this blocks our company workflow, but feel free to read #2603 and check if you can help us 😓

@0candy 0candy added bug and removed triaging labels Aug 10, 2016
@desaroger
Copy link
Author

desaroger commented Aug 10, 2016

@josieusa I tried to apply your workaround, but as is a PR for loopback-context I had difficulties to implement it. You can see my attempt on my loopback-sandbox (master/45c71...890f) repo.

I required the middleware and applied on initial:before. Isn't working, now I have no context, never XD
approach2 get

I tried too:

  • Making an Object.assign(loopback, loopbackContext) where loopbackContext is the loopback-context repo on your PR. Same results.

Edit: I said 0candy but was trying to say josieusa.

@desaroger
Copy link
Author

@josieusa Wow! I tried to implement your PR another time and I have noticed that I had to use loopback-context to get current context. I was stubborn to extend loopback context methods. Now works nicely with memmory connector.

cls-hooked get

Sadly, with mongo...

cls-hooked get mongo

I will try to make this work.

@desaroger
Copy link
Author

desaroger commented Aug 11, 2016

Finally I could solve my issue. I found two points where loopback-connector-mongodb was losing the context. I hardcoded a ns.bind on two functions and now I have context always.

I made a PR to loopback-connector-mongodb, it may help someone.

@bajtos
Copy link
Member

bajtos commented Jan 9, 2017

As I wrote in loopbackio/loopback-connector-mongodb#275 (comment), I think we should not be monkey-patching our code to support CLS. Instead, the fix should be implemented in the MongoDB client library we are using under the hood.

Let's keep all discussion related to loopback-context + MongoDB in the following issue please: strongloop/loopback-context#21

@bajtos bajtos closed this as completed Jan 9, 2017
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

5 participants