workerd: 🐛 Bug Report — Runtime APIs: The AsyncContext of AsynLocalStorages gets lost in custom thenables

If I define a custom thenable and try to access the store of an AsyncLocalStorage in the then method, the store seems to be undefined although it shouldn’t be.

Basically:

import { AsyncLocalStorage } from 'node:async_hooks';

const als = new AsyncLocalStorage();

export default {
  async fetch() {
    return als.run({ }, async () => {
      const thenable = {
        then(onfulfilled) {
          // ...
          als.getStore(); // <- yields undefined ❌
        },
      };
       // ...
    });
  },
};

minimal reproduction

About this issue

Original URL
State: closed
Created 6 months ago
Reactions: 3
Comments: 19 (13 by maintainers)

Most upvoted comments

This is now in production.

+19

kentonv on Feb 24, 2024

Quick update… I’ve got the v8 patch put together and internal testing is showing that it’s good. I want to get some review before I open the PR that adds the patch here. But, progress!

@qard … just a heads up that I was able to use your older patch as a roadmap here. When I open the PR here, the patch should contain everything you need to move forward to upstream the updated patch in v8 itself.

+11

jasnell on Feb 9, 2024

We @prisma would appreciate a further comment here when this has been rolled out to production, so we can confirm and then remove documentation for the current blocking problem. Thanks.

janpio on Feb 22, 2024

Fixed by #1665 which was merged yesterday. Likely will go to production some time next week.

kentonv on Feb 20, 2024

Going to go ahead and close this issue now since the fix has landed. As Kenton mentioned, if all goes well that’ll end up in production likely by next week.

jasnell on Feb 22, 2024

… where it would make more sense to do so

No ETA yet on the v8 change. I’ll look at it today. The change definitely needs to be made in v8. We don’t have the necessary visibility in to the necessary bits within v8 to apply a reasonable workaround in workerd.

jasnell on Feb 6, 2024

It requires a fix in v8 to get working. We can patch v8 to get it working but I’ve been waiting on a larger change coming for the implementation of the standardized AsyncContext API. I was hoping that would come faster than it has but it’s looking like there’s no ETA yet. I can work up the patch and we can float it until v8 receives the other updates.

jasnell on Jan 2, 2024

Ah, yes, custom thenables do not currently automatically propagate the async context. You’ll need to use AsyncLocalContext.bind(...) or AsyncLocalContext.snapshot(...) with the thenable to capture the context.

const thenable = {
  then: AsyncLocalStorage.bind(function (onfulfilled) {
    als.getStore();
  })
};

jasnell on Jan 2, 2024

waiting for the patch, thanks for your hard work!

rakibatomnixima on Feb 20, 2024

@jasnell yeah that would be awesome since I have seen a few developers hitting this issue 😄

Hopefully it’s not too too much work/complex to implement such temporary patch? 🤞

dario-piotrowicz on Jan 2, 2024