Skip to content

RFC: Drop the "--unhandled-rejection" CLI flag and exit on unhandled rejections #790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
saghul opened this issue Jan 6, 2025 · 7 comments · Fixed by #791
Closed

RFC: Drop the "--unhandled-rejection" CLI flag and exit on unhandled rejections #790

saghul opened this issue Jan 6, 2025 · 7 comments · Fixed by #791

Comments

@saghul
Copy link
Contributor

saghul commented Jan 6, 2025

Every other runtime does that by default. Node allows customizing the behavior but I guess that's for historical reasons, we could add the equivalent option too.

@saghul
Copy link
Contributor Author

saghul commented Jan 6, 2025

PR: #791

saghul added a commit that referenced this issue Jan 7, 2025
@ChALkeR
Copy link

ChALkeR commented Apr 23, 2025

This is a bit problematic with #39
i.e. Promise.reject().catch(() => {}) being considered an unhandled rejection

It doesn't track unhandled rejections, it tracks any rejections and exits on them

I.e. this never prints and instead exits:

Promise.reject().catch(() => {})
Promise.resolve().then(() => console.log('ok'))

@saghul
Copy link
Contributor Author

saghul commented Apr 23, 2025

Do you have any suggestions?

@ChALkeR
Copy link

ChALkeR commented Apr 24, 2025

I would argue that not crashing the process on handled rejections is more important that reporting unhandled rejections.

Every other runtime does that by default.

Those don't crash the process on handled rejections, so the rationale behind this change doesn't hold?

So perhaps this should be reverted until #39 is fixed.


Side note: also jsc CLI doesn't have unhandled rejection tracking enabled, but that is not very relevant

Enabling unhandled rejection tracking by default seems perfectly reasonable, but only under the assumption that it works (and does not misfire for all rejections, including handled)

@ChALkeR
Copy link

ChALkeR commented Apr 24, 2025

Specific suggestion: revert, reopen, block on #39.

@saghul
Copy link
Contributor Author

saghul commented Apr 24, 2025

I'll go through the entire discussion again because I remember there was something tricky about it.

@saghul
Copy link
Contributor Author

saghul commented Apr 28, 2025

I think I got it fixed: #1038

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 a pull request may close this issue.

2 participants