Mongodb breaks AsyncLocalStorage context spread

[kas84]

• Node.js Version: v15.6.0
• OS: MacOS Big Sur 11.1 (20C69)

I am having an issue using async hooks where the callback API of both mongodb and mariadb return undefined when used inside the callback (they work fine in the promise API though).

On mongodb when using useUnifiedTopology option set to true, sets the store correctly on the first run, but on subsequent requests, you still get the first store.
This is most likely a bug with their drivers, but, can somebody point me out in the right direction of what could be causing this?

I've built a sample app with mongodb: https://github.com/kas84/MongoDBCallbacksAsyncLocalStorage

[0.21756164732220062] request received
[0.21756164732220062] request received setTimeout
[undefined] request callback received
[0.21756164732220062] request promise received


When used with {useUnifiedTopology:true} , instead of undefined, you get the store, but on subsequent requests, you still get the first store

[0.7697388970645624] request received
[0.7697388970645624] request received setTimeout
[0.7697388970645624] request callback received
[0.7697388970645624] request promise received


[0.6329185718150439] request received
[0.6329185718150439] request received setTimeout
[0.7697388970645624] request callback received
[0.6329185718150439] request promise received


[0.2698320081709449] request received
[0.2698320081709449] request received setTimeout
[0.2698320081709449] request promise received
[0.7697388970645624] request callback received

[vdeturckheim]

Hey @kas84 thanks for this. I am tracking a similar issue atm and there might be a PR to do on mongo driver to fix this (cc @simon-id)

[kas84]

I opened an issue here: https://jira.mongodb.org/browse/NODE-3010 but they said they won't fix it cause of the experimental nature of the feature.

By the way, happens the same way with MariaDB driver when using callback API

[vdeturckheim]

Interesting thanks for this link.
I guess we need to go deeper into nodejs/node#35286 then. Also, I wonder if we can make them change their position (given nodejs/node#35286 they actually used to have that higher in the backlog).

cc @bengl @Qard @Flarna @rochdev in case you meet the issue in your APMs

[vdeturckheim]

Hey @kas84 ,
I am checking what it would take to mark part of the AsyncLocalStorage API stable (namely, the constructor, getStore and run as they seem to be most agreed upon parts of the API). But this might take time as it is not clear to me if AsyncResource and executionAsyncResource need to be stabilized too to get there.

In the meanwhile, you can also use AsyncResource to bind the mongo callback and fix the issue. Hopefully, there is a way to have https://jira.mongodb.org/browse/NODE-3010 reconsidered but I am not sure yet how to do it.

One of the goal of AsyncLocalStorage as a standard core API is to push the ecosystem to become Async hooks friendly but there is still a bit of work to get there ^^

[kas84]

Hi @vdeturckheim thanks for the response!

So that means I would have to bind it every time I use a mongodb callback, correct?

Other way around it would be to monkeypatch mongoDB driver but I wouldn't be thrilled to do it.

And for the most part, I use mongoDB streams API which doesn't work either but I think it would be the same fix...

[Qard]

This is exactly the reason I brought up stabilizing these things in the first place.

There seems to at least be consensus at this point that AsyncResource is de facto "stable" and we just haven't got around to marking it as such yet. For AsyncLocalStorage there was suggestions that it should be left to brew for a bit longer, which I think is fair though it already has substantial production users and I feel it has proved itself well enough so far. It might be worth reopening the stabilization discussion to see if perhaps we can mark both features as stable by the time 16.x lands. 🤔

[vdeturckheim]

@Qard 💯
I just sent you a DM with a (very conservative) stabilization proposal message to revive it. There might be a least friction/fastest path to have something out of experimental out of the door quickly and I think we should catch it.

[vdeturckheim]

@kas84 I started to move forward with it nodejs/node#37675.

[kas84]

Thanks @vdeturckheim, I will let the mongoDB team know when the PR gets merged.

Let's hope they fix the driver! 🤞

[github-actions]

It seems there has been no activity on this issue for a while, and it is being closed in 30 days. If you believe this issue should remain open, please leave a comment.
If you need further assistance or have questions, you can also search for similar issues on Stack Overflow.
Make sure to look at the README file for the most updated links.

[github-actions]

It seems there has been no activity on this issue for a while, and it is being closed. If you believe this issue should remain open, please leave a comment.
If you need further assistance or have questions, you can also search for similar issues on Stack Overflow.
Make sure to look at the README file for the most updated links.