Node: async_hooks: context loss when awaiting a thenable

[Qard]

It seems async_hooks is losing context, in certain cases, when awaiting a thenable. This appears to occur in all major versions of Node.js with async_hooks available. See the code example and output for reference.

Within the then function on the thenable object, the trigger id is 0. Note that this _does not_ happen when awaiting a thenable as the first await within an async function.

cc @nodejs/diagnostics

Code example

const async_hooks = require('async_hooks')

const asyncHook = async_hooks.createHook({
  init (asyncId, type, triggerAsyncId) {
    log({
      event: 'init',
      asyncId,
      type,
      triggerAsyncId
    })
  },

  before (asyncId) {
    log({
      event: 'before',
      asyncId
    })
  },

  after (asyncId) {
    log({
      event: 'after',
      asyncId
    })
  }
})
asyncHook.enable()

function log (data) {
  process._rawDebug(JSON.stringify(data))
}

function tick () {
  return new Promise(setImmediate)
}

async function main () {
  await tick()
  await {
    then (fn) {
      log({
        executionAsyncId: async_hooks.executionAsyncId(),
        triggerAsyncId: async_hooks.triggerAsyncId(),
        message: 'these ids should not be zero'
      })
      setImmediate(fn)
    }
  }
}

main()

Output

{"event":"init","asyncId":2,"type":"PROMISE","triggerAsyncId":1}
{"event":"init","asyncId":3,"type":"PROMISE","triggerAsyncId":1}
{"event":"init","asyncId":4,"type":"Immediate","triggerAsyncId":1}
{"event":"init","asyncId":5,"type":"PROMISE","triggerAsyncId":3}
{"event":"before","asyncId":4}
{"event":"after","asyncId":4}
{"event":"before","asyncId":5}
{"event":"init","asyncId":6,"type":"PROMISE","triggerAsyncId":2}
{"event":"init","asyncId":7,"type":"PROMISE","triggerAsyncId":6}
{"event":"after","asyncId":5}
{"executionAsyncId":0,"triggerAsyncId":0,"message":"these ids should not be zero"}
{"event":"init","asyncId":8,"type":"Immediate","triggerAsyncId":0}
{"event":"before","asyncId":8}
{"event":"after","asyncId":8}
{"event":"before","asyncId":7}
{"event":"after","asyncId":7}

[BridgeAR]

AFAIK there has never been support for thenables and thus I marked this as feature request.

[Qard]

Seems to me like a requirement for async_hooks to actually be useful. There are _many_ popular libraries that use the thenable pattern. In my particular case, we ran into knex using thenables internally, but in theory _any_ userland promise implementation should suffer from the same problem.

[Qard]

Adding diag-agenda to discuss if this should be considered a bug or feature request at the next meeting. Let me know if you disagree.

[devsnek]

related to/duplicate of?

https://github.com/nodejs/node/issues/26064
https://github.com/nodejs/node/issues/22360

[kjin]

Yes, this is the same issue as I observed in #26064.

[devsnek]

Duplicate of #26064

[devsnek]

@Qard i deduplicated this, #22360 has the most conversation on this so i made that the diag-agenda item

[Qard]

Yes, this appears to be a duplicate. I _do_ still think this needs to be brought up in the diagnostics meeting though to figure out how to proceed. The AsyncResource fix is valid, but the fact that async_hooks continues to be considered experimental discourages the community from using it and therefore means a _huge_ chunk of the ecosystem will straight up break async_hooks currently.