async_hook gives older context.

[ronak-zymr]

Hello,

We are using async hook in our node application. We want to track each and every request's context as an when it comes. we are creating a chain of request context in one map based on the trigger ID and async ID.

Find the below code for creating a chain.

init(asyncId, type, triggerAsyncId, _resource) {
        if (this.tracked.has(triggerAsyncId)) {
            this.tracked.set(asyncId, this.tracked.get(triggerAsyncId));
        }
 }

Here in the above snippet "this.tracked" is map and we are creating a chain of context inside this map and also using this map to get the context based on the asyncID.

It works fine until we move one page to another page very quickly.

When we move from one page to another page very quickly and try to get the context object from our map using the async ID it gives the older context, and the chain breaks.

We believe that out page is called before async hook init finishes its job...

Let me know if you need any more information on this.

Thanks in advance
Ronak Pandya

[PhakornKiong]

Not sure if this is allowed, but I'm actually creating a context module that is consistent with Node.js AsyncLocalStorage API for a version that does not have AsyncLocalStorage. It deviates from cls-hooked in certain ways.

You might want to look at the implementation here
https://github.com/Darkripper214/ALS-Context/blob/master/src/cls/cls.ts

If you are using Node.js version that support AsyncLocalStorage, use it. It is indeed faster due to the optimization done by the team.

Coming back to your issue, could you describe how Node is serving the page? Best if you could come up with something that is reproducible for us to really take a look at what went wrong.

Your operation on init hook shouldn't be that heavy.

[Flarna]

@ronak-zymr Do you have a reproducer?

We believe that out page is called before async hook init finishes its job...

Javascript execution is single threaded therefore nothign will run in parallel or interrupt init.

[ronak-zymr]

@Darkripper214 and @Flarna Thanks for the quick reply.

We can not provide you the whole source code.. as it is tightly coupled, but of course we can provide you the scenario in which case it is happening.

We have a node application for tracking each and every HTTP request and check if there are any vulnerability (Like SQL Injection, Command Injection, CRLF Injection, DOM XSS and so on) in that request or not.

We inject our application into some other node code to enable the vulnerability check in that application.

We are focusing on the command injection only as we are facing the issue in that only.

So we have a page called ping in that application which calls an API that accept user entered commands as a post parameter.
> API will be called and it will call our application which then calls out HTTP prototype function from our application
> HTTP prototype function adds the context in tracked map
> Then command injection will happen and our command injection prototype function will be called
> Here in the command Injection page we are not getting the context of the API which will be added in HTTP Prototype
function (only in case when we moved quickly from dashboard page to ping page)
> If we wait for some time and then fires some command from ping page it will work fine.

This is the scenario that we are facing.. I believe this will help to find out the actual issue :)

Thanks

[ronak-zymr]

@Flarna No, we dont have any.. but you can go through the scenario that we have provided in above comment..

[Flarna]

It looks like a quite complex setup with a lot code outside of node core. Can you ensure that all this code (incl. used npm modules) correctly utilize AsyncResource in case they do e.g. some sort of userspace pooling/queuing?

Most of the time such problems happen because application code is combining async operations.

I don't expect that anyone will be able to help you based on above very generic description.

[ronak-zymr]

@Flarna let me get back to you with some specific information.. or with some portion of code if I am able to do it.

Thanks

[crysislinux]

@Flarna

in case they do e.g. some sort of userspace pooling/queuing?

Do you think there is a way to audit that for all used dependencies in a nodejs app? I also observed a similar issue (A later request to see the context for an older request) in one of our nestjs app. This makes me feel really unsafe using async_hooks for business-related logic (keep tenant info for example).

[Flarna]

Do you think there is a way to audit that for all used dependencies in a nodejs app?

Theoretically maybe as most dependencies are open source. But practically I doubt that this is possible.

This makes me feel really unsafe using async_hooks for business-related logic (keep tenant info for example).

I would not use AsyncLocal store for anything I have under my control. Passing such data around via object references is by far more easy to track and easier to keep correct. It's your businesses logic so you can and should control it.

But OTel auto instrumentations do not have this possibility of manual passing the state. It's expected that OTel instrumentations keep track the context without code modification across all sort of async tasks triggered via various npm modules. e.g if there is a db.request call somewhere deep in your module tree you expect that it is able to link the new span to that one created by a http server request callback located somewhere else and working mostly independent of each other.

The only way as of now to keep track of this is AsyncLocalStore in node.js. Even if the TC39 proposal for AsyncContext will be a thing at some time the responsibility to handle user space queuing will stay in the hand of library autors.

So AsyncLocalStore is definitely not perfect but as of now there is nothing which could do the task better.

Alternative for you is to don't use autoinstrumentations at all. Instead pass the spans around manually and build your parent/child relationship manually based on your knowledge of the inner workings of your application.

Note that this is problem is not limited to OTel node.js. E.g. Java tends to use thread local which works fine in a lot cases. But in presence of shared thread pools it's needed to do similar handling as in node.js for user space queuing/connection pooling.