module: `process.exit()` should result in exit code 0 #41388

MoonBall · 2022-01-03T16:09:13Z

Fixes: #40808

nodejs-github-bot · 2022-01-03T16:09:18Z

Review requested:

@nodejs/modules

test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs

test/fixtures/es-modules/tla/process-exit.mjs

lib/internal/modules/run_main.js

lib/internal/process/per_thread.js

test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs

lib/internal/modules/esm/handle_process_exit.js

test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs

…it.mjs Co-authored-by: Antoine du Hamel <[email protected]>

Co-authored-by: Antoine du Hamel <[email protected]>

…it.mjs Co-authored-by: Mestery <[email protected]>

Co-authored-by: Jordan Harband <[email protected]>

aduh95 · 2022-01-03T23:36:00Z

Can you also make sure test/parallel/test-bootstrap-modules.js is passing please?

lib/internal/modules/esm/handle_process_exit.js

GeoffreyBooth · 2022-01-04T06:05:25Z

lib/internal/process/per_thread.js

@@ -48,6 +48,10 @@ const {
 } = require('internal/validators');
 const constants = internalBinding('constants').os.signals;

+const {
+  handleProcessExit,
+} = require('internal/modules/esm/handle_process_exit');


If this logic happens for all code, not just ESM code, should handle_process_exit.js be under modules/esm?

It only happens for ESM code, because it's the only place we have TLA in Node.js.

I am curious what's the full name of TLA? 🤣

It's an acronym for Top Level Await, sorry I was too lazy to type it all out 🙈

MoonBall · 2022-01-04T07:55:43Z

Can you also make sure test/parallel/test-bootstrap-modules.js is passing please?

fixed

lib/internal/modules/esm/handle_process_exit.js

Co-authored-by: Antoine du Hamel <[email protected]>

MylesBorins · 2022-01-07T19:28:35Z

what is the current behavior? Is this a semver-major change?

aduh95 · 2022-01-07T19:48:47Z

what is the current behavior? Is this a semver-major change?

$ node --input-type=module -e 'await Promise.resolve()'; echo $?
0
$ node --input-type=module -e 'await new Promise(()=>{})'; echo $?
13
$ node --input-type=module -e 'process.exit()'; echo $?
13
$ node --input-type=module -e 'process.exit(0)'; echo $?
0

With this change:

$ out/Release/node --input-type=module -e 'await Promise.resolve()'; echo $?
0
$ out/Release/node --input-type=module -e 'await new Promise(()=>{})'; echo $?
13
$ out/Release/node --input-type=module -e 'process.exit()'; echo $? 
0
$ out/Release/node --input-type=module -e 'process.exit(0)'; echo $?  
0

I'm tempted to say it's a patch, and that we should backport that as far as we can, but also can be convinced otherwise.

nodejs-github-bot · 2022-01-07T23:57:03Z

CI: https://ci.nodejs.org/job/node-test-pull-request/41783/

nodejs-github-bot · 2022-01-14T00:05:01Z

CI: https://ci.nodejs.org/job/node-test-pull-request/41861/

aduh95 · 2022-01-14T00:38:02Z

Landed in 775bfd1.

Due to a bug in top-level await implementation, it used to default to exit code 13. PR-URL: nodejs#41388 Fixes: nodejs#40808 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Guy Bedford <[email protected]>

Due to a bug in top-level await implementation, it used to default to exit code 13. PR-URL: #41388 Fixes: #40808 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Guy Bedford <[email protected]>

Due to a bug in top-level await implementation, it used to default to exit code 13. PR-URL: nodejs#41388 Fixes: nodejs#40808 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Guy Bedford <[email protected]>

Due to a bug in top-level await implementation, it used to default to exit code 13. PR-URL: #41388 Backport-PR-URL: #41508 Fixes: #40808 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Guy Bedford <[email protected]>

Due to a bug in top-level await implementation, it used to default to exit code 13. PR-URL: nodejs#41388 Fixes: nodejs#40808 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Guy Bedford <[email protected]>

Due to a bug in top-level await implementation, it used to default to exit code 13. PR-URL: #41388 Fixes: #40808 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Guy Bedford <[email protected]>

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [node](https://github.com/nodejs/node) | stage | minor | `14.18.3-bullseye-slim` -> `14.19.0-bullseye-slim` | --- ### Release Notes <details> <summary>nodejs/node</summary> ### [`v14.19.0`](https://github.com/nodejs/node/releases/v14.19.0) [Compare Source](nodejs/node@v14.18.3...v14.19.0) ##### Notable Changes ##### Corepack Node.js now includes Corepack, a script that acts as a bridge between Node.js projects and the package managers they are intended to be used with during development. In practical terms, **Corepack will let you use Yarn and pnpm without having to install them** - just like what currently happens with npm, which is shipped in Node.js by default. Please head over to the [Corepack documentation page](https://nodejs.org/dist/latest-v14.x/docs/api/corepack.html) for more information on how to use it. Contributed by Maël Nison - [#39608](nodejs/node#39608) ##### ICU updated ICU has been updated to 70.1. This updates timezone database to 2021a3, including bringing forward the start for DST for Jordan from March to February. Contributed by Michaël Zasso - [#40658](nodejs/node#40658) ##### New option to disable loading of native addons A new command line option `--no-addons` has been added to disallow loading of native addons. Contributed by Dominic Elm - [#39977](nodejs/node#39977) ##### Updated Root Certificates Root certificates have been updated to those from Mozilla's Network Security Services 3.71. Contributed by Richard Lau - [#40280](nodejs/node#40280) ##### Other Notable Changes - \[[`0d448eaab5`](nodejs/node@0d448eaab5)] - **(SEMVER-MINOR)** **crypto**: make FIPS related options always available (Vít Ondruch) [#36341](nodejs/node#36341) - \[[`004eafbebf`](nodejs/node@004eafbebf)] - **(SEMVER-MINOR)** **lib**: add unsubscribe method to non-active DC channels (simon-id) [#40433](nodejs/node#40433) - \[[`625be7585d`](nodejs/node@625be7585d)] - **(SEMVER-MINOR)** **lib**: add return value for DC channel.unsubscribe (simon-id) [#40433](nodejs/node#40433) - \[[`607bc74eae`](nodejs/node@607bc74eae)] - **(SEMVER-MINOR)** **module**: support pattern trailers (Guy Bedford) [#39635](nodejs/node#39635) - \[[`f74fe2a59c`](nodejs/node@f74fe2a59c)] - **(SEMVER-MINOR)** **src**: make napi_create_reference accept symbol (JckXia) [#39926](nodejs/node#39926) ##### Commits - \[[`0231ffa501`](nodejs/node@0231ffa501)] - **build**: add `--without-corepack` (Jonah Snider) [#41060](nodejs/node#41060) - \[[`5389b8ab05`](nodejs/node@5389b8ab05)] - **crypto**: update root certificates (Richard Lau) [#40280](nodejs/node#40280) - \[[`0d448eaab5`](nodejs/node@0d448eaab5)] - **(SEMVER-MINOR)** **crypto**: make FIPS related options always available (Vít Ondruch) [#36341](nodejs/node#36341) - \[[`cd20ecc7cb`](nodejs/node@cd20ecc7cb)] - **deps**: upgrade Corepack to 0.10 (Maël Nison) [#40374](nodejs/node#40374) - \[[`737df75e17`](nodejs/node@737df75e17)] - **(SEMVER-MINOR)** **deps**: add corepack (Maël Nison) [#39608](nodejs/node#39608) - \[[`b85aa5a143`](nodejs/node@b85aa5a143)] - **deps**: upgrade npm to 6.14.16 (Ruy Adorno) [#41603](nodejs/node#41603) - \[[`2755d391a5`](nodejs/node@2755d391a5)] - **deps**: update ICU to 70.1 (Michaël Zasso) [#40658](nodejs/node#40658) - \[[`3089326d89`](nodejs/node@3089326d89)] - **deps**: update archs files for OpenSSL-1.1.1m (Richard Lau) [#41173](nodejs/node#41173) - \[[`59da7c12aa`](nodejs/node@59da7c12aa)] - **deps**: upgrade openssl sources to 1.1.1m (Richard Lau) [#41173](nodejs/node#41173) - \[[`cede1f26f6`](nodejs/node@cede1f26f6)] - **deps**: add -fno-strict-aliasing flag to libuv (Daniel Bevenius) [#40631](nodejs/node#40631) - \[[`4477da858f`](nodejs/node@4477da858f)] - **doc**: fix corepack grammar for `--force` flag (Steven) [#40762](nodejs/node#40762) - \[[`5971d58600`](nodejs/node@5971d58600)] - **doc**: add missing YAML tag in `esm.md` (Antoine du Hamel) [#41516](nodejs/node#41516) - \[[`e903798ae1`](nodejs/node@e903798ae1)] - **doc**: add note regarding unfinished TLA (Antoine du Hamel) [#41434](nodejs/node#41434) - \[[`a90defebcf`](nodejs/node@a90defebcf)] - **esm**: make `process.exit()` default to exit code 0 (Gang Chen) [#41388](nodejs/node#41388) - \[[`fc328f1ab0`](nodejs/node@fc328f1ab0)] - **fs**: nullish coalescing to respect zero positional reads (Omar El-Mihilmy) [#40716](nodejs/node#40716) - \[[`004eafbebf`](nodejs/node@004eafbebf)] - **(SEMVER-MINOR)** **lib**: add unsubscribe method to non-active DC channels (simon-id) [#40433](nodejs/node#40433) - \[[`625be7585d`](nodejs/node@625be7585d)] - **(SEMVER-MINOR)** **lib**: add return value for DC channel.unsubscribe (simon-id) [#40433](nodejs/node#40433) - \[[`2c365961d0`](nodejs/node@2c365961d0)] - **module**: support pattern trailers for imports field (Guy Bedford) [#40041](nodejs/node#40041) - \[[`607bc74eae`](nodejs/node@607bc74eae)] - **(SEMVER-MINOR)** **module**: support pattern trailers (Guy Bedford) [#39635](nodejs/node#39635) - \[[`f74fe2a59c`](nodejs/node@f74fe2a59c)] - **(SEMVER-MINOR)** **src**: make napi_create_reference accept symbol (JckXia) [#39926](nodejs/node#39926) - \[[`b050c65885`](nodejs/node@b050c65885)] - **src**: add option to disable loading native addons (Dominic Elm) [#39977](nodejs/node#39977) - \[[`c1695ac68a`](nodejs/node@c1695ac68a)] - **tools**: update certdata.txt (Richard Lau) [#40280](nodejs/node#40280) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Reviewed-on: https://git.walbeck.it/mwalbeck/docker-jellyfin-livestream/pulls/97 Co-authored-by: renovate-bot <[email protected]> Co-committed-by: renovate-bot <[email protected]>

nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. labels Jan 3, 2022

module: process.exit() should result in exit code 0

b828eac

MoonBall force-pushed the fix/exit-code-13-when-calling-process.exit-in-esm-file branch from 2571c08 to b828eac Compare January 3, 2022 16:14

aduh95 reviewed Jan 3, 2022

View reviewed changes

ljharb reviewed Jan 3, 2022

View reviewed changes

lib/internal/modules/esm/handle_process_exit.js Outdated Show resolved Hide resolved

Mesteery reviewed Jan 3, 2022

View reviewed changes

test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs Outdated Show resolved Hide resolved

MoonBall and others added 6 commits January 4, 2022 07:27

Update test/fixtures/es-modules/tla/unresolved-with-worker-process-ex…

5ebf9b0

…it.mjs Co-authored-by: Antoine du Hamel <[email protected]>

Update test/fixtures/es-modules/tla/process-exit.mjs

fab1eed

Co-authored-by: Antoine du Hamel <[email protected]>

Update lib/internal/modules/run_main.js

4e6f2c0

Co-authored-by: Antoine du Hamel <[email protected]>

Update lib/internal/process/per_thread.js

dbf136c

Co-authored-by: Antoine du Hamel <[email protected]>

Update test/fixtures/es-modules/tla/unresolved-with-worker-process-ex…

24bc42c

…it.mjs Co-authored-by: Mestery <[email protected]>

Update lib/internal/modules/esm/handle_process_exit.js

adcf374

Co-authored-by: Jordan Harband <[email protected]>

use isMainThread

73e0b71

MoonBall force-pushed the fix/exit-code-13-when-calling-process.exit-in-esm-file branch from b3d04ad to 73e0b71 Compare January 4, 2022 00:11

GeoffreyBooth reviewed Jan 4, 2022

View reviewed changes

add handle_process_exit to bootstrap modules

d58b5b9

move comment to handle_process_exit file

d7bb21f

aduh95 reviewed Jan 4, 2022

View reviewed changes

lib/internal/modules/esm/handle_process_exit.js Outdated Show resolved Hide resolved

Update lib/internal/modules/esm/handle_process_exit.js

0951516

Co-authored-by: Antoine du Hamel <[email protected]>

MoonBall force-pushed the fix/exit-code-13-when-calling-process.exit-in-esm-file branch from dbbca0f to 0951516 Compare January 4, 2022 13:57

aduh95 approved these changes Jan 7, 2022

View reviewed changes

aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jan 7, 2022

This comment has been minimized.

Sign in to view

aduh95 mentioned this pull request Jan 7, 2022

tools: fix `GitHub actions status when CQ is empty #41193

Merged

guybedford approved these changes Jan 7, 2022

View reviewed changes

aduh95 merged commit 775bfd1 into nodejs:master Jan 14, 2022

aduh95 mentioned this pull request Jan 14, 2022

[v14.x backport] esm: make process.exit() default to exit code 0 #41508

Closed

aduh95 added the backport-open-v14.x label Jan 14, 2022

targos mentioned this pull request Jan 16, 2022

v17.4.0 release proposal #41557

Merged

richardlau mentioned this pull request Jan 25, 2022

v14.19.0 proposal #41696

Merged

danielleadams mentioned this pull request Feb 1, 2022

v16.14.0 proposal #41804

Merged

haoqunjiang mentioned this pull request Jul 24, 2022

Running pnpm run build in Windows fails vitejs/vite#9328

Closed

7 tasks

targos removed the backport-open-v14.x label May 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

module: `process.exit()` should result in exit code 0 #41388

module: `process.exit()` should result in exit code 0 #41388

MoonBall commented Jan 3, 2022 •

edited by aduh95

Loading

nodejs-github-bot commented Jan 3, 2022

aduh95 commented Jan 3, 2022

GeoffreyBooth Jan 4, 2022

aduh95 Jan 4, 2022

MoonBall Jan 4, 2022

aduh95 Jan 4, 2022

MoonBall commented Jan 4, 2022

MylesBorins commented Jan 7, 2022

aduh95 commented Jan 7, 2022 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

nodejs-github-bot commented Jan 7, 2022

nodejs-github-bot commented Jan 14, 2022

aduh95 commented Jan 14, 2022

module: process.exit() should result in exit code 0 #41388

module: process.exit() should result in exit code 0 #41388

Conversation

MoonBall commented Jan 3, 2022 • edited by aduh95 Loading

nodejs-github-bot commented Jan 3, 2022

aduh95 commented Jan 3, 2022

GeoffreyBooth Jan 4, 2022

Choose a reason for hiding this comment

aduh95 Jan 4, 2022

Choose a reason for hiding this comment

MoonBall Jan 4, 2022

Choose a reason for hiding this comment

aduh95 Jan 4, 2022

Choose a reason for hiding this comment

MoonBall commented Jan 4, 2022

MylesBorins commented Jan 7, 2022

aduh95 commented Jan 7, 2022 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

nodejs-github-bot commented Jan 7, 2022

nodejs-github-bot commented Jan 14, 2022

aduh95 commented Jan 14, 2022

module: `process.exit()` should result in exit code 0 #41388

module: `process.exit()` should result in exit code 0 #41388

MoonBall commented Jan 3, 2022 •

edited by aduh95

Loading

aduh95 commented Jan 7, 2022 •

edited

Loading