Skip to content
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

[v20.x backport] src: disable uncaught exception abortion for ESM syntax detection #51989

Conversation

Mateusz-Kopec
Copy link

@Mateusz-Kopec Mateusz-Kopec commented Mar 6, 2024

Refs: #50987
Fixes: #50878

I confirmed that Wrapping a require of an ES module while using --abort-on-uncaught-exception fails without the fix

PS. I read through Backporting release lines and Contributing and I'm still not sure what should I do to get the fix backported.

a) Is it still "baking", is the process automated so should I just wait a bit more?
b) Should I just add some label backport-requested-vN.x and because there are no conflicts it will be automatically backported? I guess it's not possible.
c) Was it decided that fix shouldn't land in LTS?

Please let me know if I should close this PR and request it some other way. :)

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch. vm Issues and PRs related to the vm subsystem. labels Mar 6, 2024
PR-URL: nodejs#50987
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>

Backport-PR-URL: nodejs#51989
@anonrig
Copy link
Member

anonrig commented Mar 6, 2024

cc @nodejs/releasers

@targos
Copy link
Member

targos commented Mar 6, 2024

Is it a clean cherry-pick? If that's the case, a backport PR shouldn't be necessary. We use lts-watch- label to bring PRs to the attention of releasers.

@Mateusz-Kopec
Copy link
Author

Mateusz-Kopec commented Mar 6, 2024

Is it a clean cherry-pick? If that's the case, a backport PR shouldn't be necessary. We use lts-watch- label to bring PRs to the attention of releasers.

It is clean cherry-pick, is there a way for me to add label? I assume there's some permission that prevents label spam from people that are not maintainers.
Or would the correct action be to just comment in the mentioned PR?

@anonrig
Copy link
Member

anonrig commented Mar 6, 2024

I've added the labels.

@Mateusz-Kopec
Copy link
Author

I've added the labels.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch. vm Issues and PRs related to the vm subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants