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

Context lost when awaiting on custom thenable #37

Open
vkarpov15 opened this issue Dec 11, 2018 · 4 comments
Open

Context lost when awaiting on custom thenable #37

vkarpov15 opened this issue Dec 11, 2018 · 4 comments

Comments

@vkarpov15
Copy link

Not sure if this is an issue or expected behavior, but figured I'd ask. I've been tracking down a bug with mongoose and express-http-context, and found that if you use await with a custom then() function, context goes away. Here's an example:

const clsHooked = require('cls-hooked');

const session = clsHooked.createNamespace('mytest');

session.run(() => {
  run().catch(err => console.log(err));
});

async function run() {
  session.set('test', '42');

  const thenable = {
    then: (resolve, reject) => {
      console.log('Session', session.get('test')); // undefined
      return Promise.resolve('').then(resolve, reject);
    }
  };

  await thenable; // This is the problem, context gets lost in `then()`

  // This works fine
  await new Promise((resolve, reject) => {
    console.log('Session2', session.get('test')); // 42
  });
}

Any ideas as to what might be going on here?

Re: Automattic/mongoose#7292

@Jeff-Lewis
Copy link
Owner

@vkarpov15 ! Sorry I haven't gotten to this. I'm a little fuzzy on what mongoose's exec() does under the hood but my first thought is that a custom then() would need to be wrapped similar to how cls-bluebird does it.

@rochdev
Copy link

rochdev commented Aug 14, 2019

This is caused by an issue in Node where the context is restored by the Promise class and not the await language construct which is handled within v8.

@vkarpov15
Copy link
Author

@Jeff-Lewis I've been looking more into this issue and found something you might find interesting: this issue only happens with run(). It works fine as long as you wrap your async function with runPromise() as shown below.

const clsHooked = require('cls-hooked');

const session = clsHooked.createNamespace('mytest');

session.runPromise(run);

async function run() {
  session.set('test', '42');

  const thenable = {
    then: (resolve, reject) => {
      console.log('Session', session.get('test')); // undefined
      return Promise.resolve('').then(resolve, reject);
    }
  };

  await thenable; // This is the problem, context gets lost in `then()`

  // This works fine
  await new Promise((resolve, reject) => {
    console.log('Session2', session.get('test')); // 42
  });
}

It seems like this only happens if you start using async functions inside of run() rather than runPromise().

@rochdev
Copy link

rochdev commented Oct 7, 2020

@vkarpov15 If your issue is caused by the bug in Node, it should be fixed by this PR in ^12.19.0 || >=14.5.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants