Node: 12.16/13.8: http response listener throwing does not result in emit of uncaughtException

[michaelgoin]

• Version: 12.16.0 | 13.8.0
• Platform: Darwin Kernel Version 18.6.0
• Subsystem:

What steps will reproduce the bug?

Throwing from a response listener (callback) to http.get() will not trigger process.once('uncaughtException', () => {}). Interestingly, throwing from a request listener (callback) to http.createServer() will still.

This behavior changed with 12.16.0, I'm guessing likely due to the porting of the emit changes?

const http = require('http')

let server
let request

process.once('uncaughtException', function() {
  // never gets here from response listener in 12.16, works fine < 12.16.
  console.log('in uncaughtException handler')

  server.close(done)
})

server = http.createServer(function cb_createServer(request, response) {
  // Throw from request listener will result in uncaughtException
  //throw new Error('wat')
  response.writeHead(200, {'Content-Type': 'text/plain'})
  response.end()
})

server.listen(8183, function() {
  request = http.get({host: 'localhost', port: 8183}, function() {
    // Throw from response listener will not result in uncaughtException
    throw new Error('whoah')
  })
})

How often does it reproduce? Is there a required condition?

Consistently does not trigger uncaughtException / does not allow handling via process.on('uncaughtException', ...).

What is the expected behavior?

Should be able to notice the uncaught exception thrown from the handler.

What do you see instead?

Additional information

[addaleax]

Is there anything else you’re doing besides running the above script with node <filename> to reproduce? It works for me on x64 Linux (after removing the undefined done callback) on the versions you provided.

[michaelgoin]

Well that's embarrassing. Let me dig around a bit. It was failing in tests and then I ported out and ran via debugger and saw the same issues, but admittedly didn't run straight via script at that point.

[michaelgoin]

I think I let myself get tricked by the debugger. I'm back to thinking it is something we are doing that is incompatible with the new 12.16 code. Going to go ahead and close this out, can always re-open if proves real.

[michaelgoin]

Looks like something about creating an async hook is causing this.

[michaelgoin]

Specifically, looks like the existance of an after handler that changes the overall behavior.

Is this expected?

Adding this to the top results in the breaking behavior...

const asyncHooks = require('async_hooks')

var hook = asyncHooks.createHook({
  // init: () => {},
  // before: () => {},
  after: () => {},
  //destroy: () => {}
}).enable()

[addaleax]

This was caused by 08e55e302bb39bf491e0f71619d494f62b24428e; a fix is in https://github.com/nodejs/node/pull/31801.

I’m assuming it’s too late for the fix to get into #31781 unless the release gets pushed out to next week, though.