Promise rejection in timeout (versus module-level) treated as unhandled by debugger

rotu commented 3 months ago

Version

v22.4.0

Platform

Darwin Hypothesis.local 24.0.0 Darwin Kernel Version 24.0.0: Thu Jun 20 20:36:19 PDT 2024; root:xnu-11215.0.115.501.3~1/RELEASE_ARM64_T8103 arm64

Subsystem

No response

What steps will reproduce the bug?

Create this script:

setTimeout(() => {
  Promise.reject().catch(() => { })
}, 1)

And run it in the debugger with break on Uncaught enabled.

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

Every time. Note this does not happen if Promise.reject() is at top level instead of wrapped in setTimeout().

What is the expected behavior? Why is that the expected behavior?

It is expected that the debugger does (1) not pause on such a promise or (2) only pauses on such a promise if breakOnException is enabled.

The HTML spec guarantees that promise rejections are not considered unhandled if a handler is then synchronously attached.

What do you see instead?

Node breaks synchronously on the creation of the rejected promise. Similar issues happen when using the VSCode debugger and Chrome debugger.

node inspect promisetest.js
< Debugger listening on ws://127.0.0.1:9229/d18cff1f-6886-4ff6-9a4f-404128bd150a
< For help, see: https://nodejs.org/en/docs/inspector
< 
< Debugger attached.
< 
 ok
Break on start in promisetest.js:1
> 1 setTimeout(() => {
  2   Promise.reject().catch(() => { })
  3 }, 1)
debug> breakOnUncaught
debug> c
promiseRejection in promisetest.js:2
  1 setTimeout(() => {
> 2   Promise.reject().catch(() => { })
  3 }, 1)
  4

Additional information

A downstream issue where this interferes with usage of the web streams API: #51093

I too have been incredibly confused by this, as it makes it seem like even correct usage of promises is developer error.

VSCode reports this as "Exception has occurred" instead of "promiseRejection".

RedYetiDev commented 3 months ago

It's reproducible via the VSCode debugger.

The debugger treats it as both a caught exception, and an uncaught exception.

It appears to be a debugger specific issue, as it does not emit an uncaughtException (or unhandledRejection) event on the process:

process.on("uncaughtException", () => console.log("uncaughtException")) // This is never called
process.on("unhandledRejection", () => console.log("unhandledRejection")) // This is never called

setTimeout(() => {
    Promise
        .reject("This is a promise rejection")
        .catch(() => console.log("Caught promise rejection")) // This is called
}, 1)

rotu commented 3 months ago

It appears to be a debugger specific issue, as it does not emit an uncaughtException (or unhandledRejection) event on the process:

Yes, it is a debugger-specific issue caused by V8 "catch prediction". The debugger breaks synchronously when the promise is rejected.

It would be okay if the debugger paused here when breakOnException was specified, but it's inappropriate for breakOnUncaught.

Related discussion: https://issues.chromium.org/issues/41161875

RedYetiDev commented 3 months ago

@nodejs/v8

rotu commented 3 months ago

Relatedly, the below code does not trip the debugger (with breakOnUncaught enabled) and does not print anything. I think this situation is dual to the originally reported issue:

the rejected promise never has a handler attached
but catch prediction incorrectly expects the surrounding try statement to apply to the rejection (it doesn't because the promise is never awaited).

setTimeout(async () => {
  try {
    Promise.reject()
  } catch (e) { }
})

rotu commented 2 months ago

I think I figured out the setTimeout part of this mystery. When running a module as the main entry point, the code is within in a try block in an async function (in module_job.js).

https://github.com/nodejs/node/blob/4b8000c66c09da5f3c7667378c94ce7558f8c1b3/lib/internal/modules/esm/module_job.js#L253-L263

Because of this try/catch block, NO promise rejection at module level is regarded by the inspector as an "uncaught exception", REGARDLESS OF WHETHER OR NOT IT HAS A HANDLER ATTACHED.

I raised an issue for this catch prediction false positive but it was regarded as "infeasible" here: https://issues.chromium.org/issues/352455689

When code escapes this call stack by running asynchronously, the inspector no longer regards it as handled by that try/catch so it may or may not be considered an "uncaught exception" (based on V8's designed behavior).

Here's a demonstration:

// assume this is in a .mjs file

// this function creates a Promise which is rejected and handled
// This promise is NOT regarded as "caught" by the V8's catch prediction
const makeRejectedPromise = (reason) => {
   Promise.reject(reason).catch(() => {});
};

// rejection regarded as handled created synchronously:
makeRejectedPromise(1);
(async () => {
   makeRejectedPromise(2);
})();
await null;
// or asynchronously after a module-level await
makeRejectedPromise(3);

// rejection regarded as unhandled when created asynchronously:
(async () => {
   await null;
   makeRejectedPromise(4);
})();
queueMicrotask(() => makeRejectedPromise(5));
process.nextTick(() => makeRejectedPromise(6));
setTimeout(() => makeRejectedPromise(7));

nodejs / node