Repository navigation
async_hooks: context loss when awaiting a thenable #27599
Description
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}
Activity
BridgeAR commentedon May 7, 2019
AFAIK there has never been support for thenables and thus I marked this as feature request.
Qard commentedon May 7, 2019
Seems to me like a requirement for
async_hooksto 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 commentedon May 7, 2019
Adding
diag-agendato discuss if this should be considered a bug or feature request at the next meeting. Let me know if you disagree.devsnek commentedon May 7, 2019
related to/duplicate of?
#26064
#22360
kjin commentedon May 7, 2019
Yes, this is the same issue as I observed in #26064.
devsnek commentedon May 7, 2019
Duplicate of #26064
devsnek commentedon May 7, 2019
@Qard i deduplicated this, #22360 has the most conversation on this so i made that the
diag-agendaitemQard commentedon May 7, 2019
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
AsyncResourcefix is valid, but the fact thatasync_hookscontinues to be considered experimental discourages the community from using it and therefore means a huge chunk of the ecosystem will straight up breakasync_hookscurrently.