Skip to content

fix(node-core): do not print rejection warning in strict mode#21160

Open
mohd-akram wants to merge 1 commit into
getsentry:developfrom
mohd-akram:strict-rejection
Open

fix(node-core): do not print rejection warning in strict mode#21160
mohd-akram wants to merge 1 commit into
getsentry:developfrom
mohd-akram:strict-rejection

Conversation

@mohd-akram
Copy link
Copy Markdown
Contributor

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.

@mohd-akram mohd-akram requested a review from a team as a code owner May 26, 2026 07:10
@mohd-akram mohd-akram requested review from JPeer264 and andreiborza and removed request for a team May 26, 2026 07:10
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Fix All in Cursor

❌ 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.

Comment thread packages/node-core/src/integrations/onunhandledrejection.ts
@mohd-akram mohd-akram force-pushed the strict-rejection branch 4 times, most recently from cf63627 to f7b4f95 Compare May 26, 2026 08:12
@isaacs isaacs self-requested a review May 26, 2026 18:49
Copy link
Copy Markdown
Member

@isaacs isaacs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Copy Markdown
Contributor Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants