fix(node-core): rejection warning in strict mode

[mohd-akram]

Only print it if the reason doesn't have a stack. This matches the behavior of Node.js.

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

• If you've added code that should be tested, please add tests.
• Ensure your code lints and the test suite passes (yarn lint) & (yarn test).
• Link an issue if there is one related to your pull request. If no issue is linked, one will be auto-generated and linked.

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 03a493d. Configure here.}

[isaacs]

I'm a bit confused. I don't think that this actually does match the behavior in Node...?

$ node --unhandled-rejections=strict -e 'Promise.reject("ok")'
node:internal/process/promises:281
    reason : new UnhandledPromiseRejection(reason);
             ^

UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "ok".
    at strictUnhandledRejectionsMode (node:internal/process/promises:281:14)
    at processPromiseRejections (node:internal/process/promises:413:17)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:32) {
  code: 'ERR_UNHANDLED_REJECTION'
}

Node.js v26.0.0

Maybe I'm missing the intention. What is the situation that this is supposed to address?

[mohd-akram]

@isaacs Okay, I don't think I've ever thrown a string as an error, but it seems the behavior is different in Node.js for Error objects vs strings:

$ node --unhandled-rejections=strict -e 'Promise.reject(new Error("ok"))'
[eval]:1
Promise.reject(new Error("ok"))
               ^

Error: ok
    at [eval]:1:16
    at runScriptInThisContext (node:internal/vm:219:10)
    at node:internal/process/execution:483:12
    at [eval]-wrapper:6:24
    at runScriptInContext (node:internal/process/execution:481:60)
    at evalFunction (node:internal/process/execution:315:30)
    at evalTypeScript (node:internal/process/execution:327:3)
    at node:internal/main/eval_string:71:3

Node.js v26.2.0

If it matters, I can adjust the PR to keep the current behavior if the object doesn't have a stack property, similar to the check that's already there.

[isaacs]

Ah, I see yes, it is a different result if the thrown value is not an Error object. In that case it constructs an error object with the message.

If it matters, I can adjust the PR to keep the current behavior if the object doesn't have a stack property, similar to the check that's already there.

If we're going to be matching Node's behavior, then we ought to print the long warning whenever the thrown value is not an instanceof Error. If it's an Error object without a stack or message, it still does the quieter immediate exit.

$ node --unhandled-rejections=strict -e 'er = Object.assign(new Error(), { stack: undefined, message: undefined }); console.error(er.stack); Promise.reject(er)'
undefined
node:internal/process/promises:275
  triggerUncaughtException(err, true /* fromPromise */);
  ^

[Error]

Node.js v26.2.0

It looks like these are the cases to cover:

• with an Error object:
  • --unhandled-rejections=warn: print the error with the long message as a warning, do not exit or set status
  • --unhandled-rejections=strict: log the error object, immediately exit with status 1
  • --unhandled-rejections=none: do not exit, do not set status, do not print anything
• with any other value:
  • --unhandled-rejections=warn: print the value with the long message as a warning, do not exit or set status
  • --unhandled-rejections=strict: log a constructed error object with the long message, immediately exit with status 1
  • --unhandled-rejections=none: do not exit, do not set status, do not print anything

Here's a test that loops through each of the cases for reference:

#!/bin/bash

SETTINGS=(none warn strict)
VALUES=('true' 'new Error("x")')

for setting in "${SETTINGS[@]}"; do
  for er in "${VALUES[@]}"; do
    echo "CASE: unhandled-rejections=${setting} error=${er}"
    node --unhandled-rejections=${setting} \
      -e "Promise.reject($er); setTimeout(()=>console.error('did not exit'))" \
    echo $?
    echo "----"
  done
done

Results:

CASE: unhandled-rejections=none error=true
did not exit
----
CASE: unhandled-rejections=none error=new Error("x")
did not exit
----
CASE: unhandled-rejections=warn error=true
(node:38139) UnhandledPromiseRejectionWarning: true
(Use `node --trace-warnings ...` to show where the warning was created)
(node:38139) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
did not exit
----
CASE: unhandled-rejections=warn error=new Error("x")
(node:38140) UnhandledPromiseRejectionWarning: Error: x
    at [eval]:1:16
    at runScriptInThisContext (node:internal/vm:219:10)
    at node:internal/process/execution:483:12
    at [eval]-wrapper:6:24
    at runScriptInContext (node:internal/process/execution:481:60)
    at evalFunction (node:internal/process/execution:315:30)
    at evalTypeScript (node:internal/process/execution:327:3)
    at node:internal/main/eval_string:71:3
(Use `node --trace-warnings ...` to show where the warning was created)
(node:38140) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
did not exit
----
CASE: unhandled-rejections=strict error=true
node:internal/process/promises:273
    reason : new UnhandledPromiseRejection(reason);
             ^

UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "true".
    at strictUnhandledRejectionsMode (node:internal/process/promises:273:14)
    at processPromiseRejections (node:internal/process/promises:405:17)
    at process.processTicksAndRejections (node:internal/process/task_queues:106:37) {
  code: 'ERR_UNHANDLED_REJECTION'
}

Node.js v26.2.0
----
CASE: unhandled-rejections=strict error=new Error("x")
[eval]:1
Promise.reject(new Error("x")); setTimeout(()=>console.error('did not exit'))
               ^

Error: x
    at [eval]:1:16
    at runScriptInThisContext (node:internal/vm:219:10)
    at node:internal/process/execution:483:12
    at [eval]-wrapper:6:24
    at runScriptInContext (node:internal/process/execution:481:60)
    at evalFunction (node:internal/process/execution:315:30)
    at evalTypeScript (node:internal/process/execution:327:3)
    at node:internal/main/eval_string:71:3

Node.js v26.2.0
----

If you want to update it to match Node's behavior in each situation, I think we can get likely that in for v11.

[mohd-akram]

If we're going to be matching Node's behavior, then we ought to print the long warning whenever the thrown value is not an instanceof Error. If it's an Error object without a stack or message, it still does the quieter immediate exit.

I've pushed that change. Node.js does not use instanceof, but only checks if the stack property is there.

It looks like these are the cases to cover:

For all the cases, getting it exactly right will probably not be doable, definitely not for warn mode because if the user runs with --unhandled-rejections=warn, then Node.js will emit a warning regardless of whether there's an unhandledRejection handler, and then Sentry will emit its own warning, so there will be two of them. Probably Sentry should move to uncaughtExceptionMonitor so that it doesn't have to deal with all these edge cases.

[isaacs]

Ah, yes, you are right. I was mistaken, setting the stack to undefined is actually not the trick, because it's checking for an ownProp. But instanceof isn't relevant. So copying the ownProp test for Error-like-ness is probably fine.

For all the cases, getting it exactly right will probably not be doable, definitely not for warn mode because if the user runs with --unhandled-rejections=warn, then Node.js will emit a warning regardless of whether there's an unhandledRejection handler, and then Sentry will emit its own warning, so there will be two of them.

Yes, I guess what I'm saying is, we should endeavor to have the behavior with Node+Sentry to be the same as the behavior with Node alone. If we do something that changes behavior, we should simulate it (eg, listening prevents the crash, so if we're the only listener, then crash explicitly.) But of course, we don't need to warn if there's already a warning being printed.

Probably Sentry should move to uncaughtExceptionMonitor so that it doesn't have to deal with all these edge cases.

I actually agree, that's almost certainly a better way forward if we can make it work. However, this is a subtle and somewhat tricky thing to get exactly right, and Error reporting is about the most important thing that our users rely on Sentry to do reliably, so we have to approach it with a lot of care, and so it somewhat lands in "not broken, don't fix it" for v11.

More surface-level behavior choices that don't fundamentally alter how it works, but brings the behavior closer to Node's default behavior, is much lower risk.