done() cause AsyncLocalStorage context propagation

[wvq]

Prerequisites

• [ x ] Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
• [ x ] Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
• [ x ] 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
• [ x ] Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend that you not install Mocha globally.

Description

Steps to Reproduce

Expected behavior:

const { AsyncLocalStorage } = require('async_hooks')

const STORAGE = new AsyncLocalStorage()

STORAGE.run({prop1: 1, prop2: 2}, function() {
  // print: { prop1: 1, prop2: 2 }
  console.log(STORAGE.getStore())
})

// print: undefined
console.log(STORAGE.getStore())

Actual behavior:
when use mocha with done():

const { AsyncLocalStorage } = require('async_hooks')

const STORAGE = new AsyncLocalStorage()

describe('AsyncLocalStorage test', function () {
  it('with context', function (done) {
    STORAGE.run(new Map(Object.entries({prop1: '1', prop2: '2'})), function () {
      // print: { prop1: 1, prop2: 2 }
      console.log(STORAGE.getStore())


      // done() is run on this context, so below test will still in this context
      done()
    })
  })

  it('without context', function() {
    // should print : undefined
    // but got : { prop1: 1, prop2: 2 }
    console.log(STORAGE.getStore())
  })

})

Reproduces how often: [What percentage of the time does it reproduce?]

Versions

• The output of mocha --version and node node_modules/.bin/mocha --version:
• The output of node --version:
• Your operating system
  • name and version:
  • architecture (32 or 64-bit):
• Your shell (e.g., bash, zsh, PowerShell, cmd):
• Your browser and version (if running browser tests):
• Any third-party Mocha-related modules (and their versions):
• Any code transpiler (e.g., TypeScript, CoffeeScript, Babel) being used (and its version):

Additional Information

[juergba]

@wvq you are calling done() within STORAGE.run() context. That way you are continuing the async chain incl. calling the next test. The store is passed to the next test since it is part of the same context (async chain).

Call done() outside of STORAGE.run() and store will not be passed to the next test.

[juergba]

Async hooks is still an experimental feature, so my opinion may change in future. If you open an AsyncLocalStorage it is your responsibility to close it as well. It's not Mocha's task to accomplish this for you, nor does Mocha know where you would like to exit the context.

[alxndrsn]

@juergba while async_hooks is still experimental "Asynchronous Context Tracking" and the parts of AsyncLocalStorage used in @wvq's example are now stable: https://nodejs.org/api/async_context.html#async_context_class_asynclocalstorage, nodejs/node@7612d82

@wvq based on your example, I've had a quick go at writing a wrapper for the mocha test functions to protect against async context leaking between tests. It's available at https://www.npmjs.com/package/mocha-async-hooks-aware / https://github.com/alxndrsn/mocha-async-hooks-aware