GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — module: `process.exit()` should result in exit code 0 by MoonBall · Pull Request #41388 · nodejs/node

Conversation

@MoonBall

@MoonBall MoonBall commented Jan 3, 2022 •
edited by aduh95
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Fixes: #40808

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@nodejs-github-bot

nodejs-github-bot commented Jan 3, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/modules
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot 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
@MoonBall
module: process.exit() should result in exit code 0
b828eac
@MoonBall
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
aduh95 reviewed Jan 3, 2022
View reviewed changes
Comment thread test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread test/fixtures/es-modules/tla/process-exit.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/modules/run_main.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/process/per_thread.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

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

Uh oh!

There was an error while loading. Please reload this page.

ljharb
ljharb reviewed Jan 3, 2022
View reviewed changes
Comment thread lib/internal/modules/esm/handle_process_exit.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Mesteery
Mesteery reviewed Jan 3, 2022
View reviewed changes
Comment thread test/fixtures/es-modules/tla/unresolved-with-worker-process-exit.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.

MoonBall and others added 6 commits January 4, 2022 07:27
@MoonBall @aduh95
Update test/fixtures/es-modules/tla/unresolved-with-worker-process-ex…
5ebf9b0
…it.mjs

Co-authored-by: Antoine du Hamel <[email protected]>
@MoonBall @aduh95
Update test/fixtures/es-modules/tla/process-exit.mjs
fab1eed
Co-authored-by: Antoine du Hamel <[email protected]>
@MoonBall @aduh95
Update lib/internal/modules/run_main.js
4e6f2c0
Co-authored-by: Antoine du Hamel <[email protected]>
@MoonBall @aduh95
Update lib/internal/process/per_thread.js
dbf136c
Co-authored-by: Antoine du Hamel <[email protected]>
@MoonBall @Mesteery
Update test/fixtures/es-modules/tla/unresolved-with-worker-process-ex…
24bc42c
…it.mjs

Co-authored-by: Mestery <[email protected]>
@MoonBall @ljharb
Update lib/internal/modules/esm/handle_process_exit.js
adcf374
Co-authored-by: Jordan Harband <[email protected]>
@aduh95

aduh95 commented Jan 3, 2022

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@MoonBall
use isMainThread
73e0b71
@MoonBall
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
GeoffreyBooth reviewed Jan 4, 2022
View reviewed changes
Comment thread lib/internal/modules/esm/handle_process_exit.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/process/per_thread.js

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

@GeoffreyBooth GeoffreyBooth Jan 4, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@aduh95 aduh95 Jan 4, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@MoonBall MoonBall Jan 4, 2022

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@aduh95 aduh95 Jan 4, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

MoonBall reacted with heart emoji
All reactions
  • ❤️ 1 reaction
@MoonBall
add handle_process_exit to bootstrap modules
d58b5b9
@MoonBall

MoonBall commented Jan 4, 2022

Copy link
Copy Markdown
Member Author

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

fixed

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@MoonBall
move comment to handle_process_exit file
d7bb21f
aduh95
aduh95 reviewed Jan 4, 2022
View reviewed changes
Comment thread lib/internal/modules/esm/handle_process_exit.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@MoonBall @aduh95
Update lib/internal/modules/esm/handle_process_exit.js
0951516
Co-authored-by: Antoine du Hamel <[email protected]>
@MoonBall
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
@MylesBorins

MylesBorins commented Jan 7, 2022

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@aduh95

aduh95 commented Jan 7, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aduh95
aduh95 approved these changes Jan 7, 2022
View reviewed changes
@aduh95 aduh95 added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Jan 7, 2022
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@aduh95 aduh95 mentioned this pull request Jan 7, 2022
tools: fix `GitHub actions status when CQ is empty #41193
Merged
guybedford
guybedford approved these changes Jan 7, 2022
View reviewed changes
@nodejs-github-bot

nodejs-github-bot commented Jan 7, 2022

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Jan 8, 2022
CI Reliability 2022-01-08 nodejs/reliability#169
Open
CI Reliability 2022-01-09 nodejs/reliability#170
Open
CI Reliability 2022-01-12 nodejs/reliability#171
Open
CI Reliability 2022-01-13 nodejs/reliability#172
Open
CI Reliability 2022-01-14 nodejs/reliability#173
Open
@nodejs-github-bot

nodejs-github-bot commented Jan 14, 2022

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

aduh95 commented Jan 14, 2022

Copy link
Copy Markdown
Contributor

Landed in 775bfd1.

MoonBall reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aduh95 pushed a commit to aduh95/node that referenced this pull request Jan 14, 2022
@MoonBall @aduh95
esm: make process.exit() default to exit code 0
968960d
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]>
@aduh95 aduh95 mentioned this pull request Jan 14, 2022
[v14.x backport] esm: make process.exit() default to exit code 0 #41508
Closed
@aduh95 aduh95 added the backport-open-v14.x label Jan 14, 2022
targos pushed a commit that referenced this pull request Jan 14, 2022
@MoonBall @targos
esm: make process.exit() default to exit code 0
c9a4603
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]>
@targos targos mentioned this pull request Jan 16, 2022
v17.4.0 release proposal #41557
Merged
richardlau pushed a commit that referenced this pull request Jan 25, 2022
@MoonBall @richardlau
esm: make process.exit() default to exit code 0
a90defe
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]>
@richardlau richardlau mentioned this pull request Jan 25, 2022
v14.19.0 proposal #41696
Merged
danielleadams pushed a commit that referenced this pull request Feb 1, 2022
@MoonBall @danielleadams
esm: make process.exit() default to exit code 0
3cfe856
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]>
@danielleadams danielleadams mentioned this pull request Feb 1, 2022
v16.14.0 proposal #41804
Merged
mwalbeck pushed a commit to mwalbeck/docker-jellyfin-livestream that referenced this pull request Mar 16, 2022
renovate-bot @mwalbeck
Update Node.js to v14.19.0 (#97)
7fa000e
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 - [#&#8203;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 - [#&#8203;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 - [#&#8203;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 - [#&#8203;40280](nodejs/node#40280)

##### Other Notable Changes

-   \[[`0d448eaab5`](nodejs/node@0d448eaab5)] - **(SEMVER-MINOR)** **crypto**: make FIPS related options always available (Vít Ondruch) [#&#8203;36341](nodejs/node#36341)
-   \[[`004eafbebf`](nodejs/node@004eafbebf)] - **(SEMVER-MINOR)** **lib**: add unsubscribe method to non-active DC channels (simon-id) [#&#8203;40433](nodejs/node#40433)
-   \[[`625be7585d`](nodejs/node@625be7585d)] - **(SEMVER-MINOR)** **lib**: add return value for DC channel.unsubscribe (simon-id) [#&#8203;40433](nodejs/node#40433)
-   \[[`607bc74eae`](nodejs/node@607bc74eae)] - **(SEMVER-MINOR)** **module**: support pattern trailers (Guy Bedford) [#&#8203;39635](nodejs/node#39635)
-   \[[`f74fe2a59c`](nodejs/node@f74fe2a59c)] - **(SEMVER-MINOR)** **src**: make napi_create_reference accept symbol (JckXia) [#&#8203;39926](nodejs/node#39926)

##### Commits

-   \[[`0231ffa501`](nodejs/node@0231ffa501)] - **build**: add `--without-corepack` (Jonah Snider) [#&#8203;41060](nodejs/node#41060)
-   \[[`5389b8ab05`](nodejs/node@5389b8ab05)] - **crypto**: update root certificates (Richard Lau) [#&#8203;40280](nodejs/node#40280)
-   \[[`0d448eaab5`](nodejs/node@0d448eaab5)] - **(SEMVER-MINOR)** **crypto**: make FIPS related options always available (Vít Ondruch) [#&#8203;36341](nodejs/node#36341)
-   \[[`cd20ecc7cb`](nodejs/node@cd20ecc7cb)] - **deps**: upgrade Corepack to 0.10 (Maël Nison) [#&#8203;40374](nodejs/node#40374)
-   \[[`737df75e17`](nodejs/node@737df75e17)] - **(SEMVER-MINOR)** **deps**: add corepack (Maël Nison) [#&#8203;39608](nodejs/node#39608)
-   \[[`b85aa5a143`](nodejs/node@b85aa5a143)] - **deps**: upgrade npm to 6.14.16 (Ruy Adorno) [#&#8203;41603](nodejs/node#41603)
-   \[[`2755d391a5`](nodejs/node@2755d391a5)] - **deps**: update ICU to 70.1 (Michaël Zasso) [#&#8203;40658](nodejs/node#40658)
-   \[[`3089326d89`](nodejs/node@3089326d89)] - **deps**: update archs files for OpenSSL-1.1.1m (Richard Lau) [#&#8203;41173](nodejs/node#41173)
-   \[[`59da7c12aa`](nodejs/node@59da7c12aa)] - **deps**: upgrade openssl sources to 1.1.1m (Richard Lau) [#&#8203;41173](nodejs/node#41173)
-   \[[`cede1f26f6`](nodejs/node@cede1f26f6)] - **deps**: add -fno-strict-aliasing flag to libuv (Daniel Bevenius) [#&#8203;40631](nodejs/node#40631)
-   \[[`4477da858f`](nodejs/node@4477da858f)] - **doc**: fix corepack grammar for `--force` flag (Steven) [#&#8203;40762](nodejs/node#40762)
-   \[[`5971d58600`](nodejs/node@5971d58600)] - **doc**: add missing YAML tag in `esm.md` (Antoine du Hamel) [#&#8203;41516](nodejs/node#41516)
-   \[[`e903798ae1`](nodejs/node@e903798ae1)] - **doc**: add note regarding unfinished TLA (Antoine du Hamel) [#&#8203;41434](nodejs/node#41434)
-   \[[`a90defebcf`](nodejs/node@a90defebcf)] - **esm**: make `process.exit()` default to exit code 0 (Gang Chen) [#&#8203;41388](nodejs/node#41388)
-   \[[`fc328f1ab0`](nodejs/node@fc328f1ab0)] - **fs**: nullish coalescing to respect zero positional reads (Omar El-Mihilmy) [#&#8203;40716](nodejs/node#40716)
-   \[[`004eafbebf`](nodejs/node@004eafbebf)] - **(SEMVER-MINOR)** **lib**: add unsubscribe method to non-active DC channels (simon-id) [#&#8203;40433](nodejs/node#40433)
-   \[[`625be7585d`](nodejs/node@625be7585d)] - **(SEMVER-MINOR)** **lib**: add return value for DC channel.unsubscribe (simon-id) [#&#8203;40433](nodejs/node#40433)
-   \[[`2c365961d0`](nodejs/node@2c365961d0)] - **module**: support pattern trailers for imports field (Guy Bedford) [#&#8203;40041](nodejs/node#40041)
-   \[[`607bc74eae`](nodejs/node@607bc74eae)] - **(SEMVER-MINOR)** **module**: support pattern trailers (Guy Bedford) [#&#8203;39635](nodejs/node#39635)
-   \[[`f74fe2a59c`](nodejs/node@f74fe2a59c)] - **(SEMVER-MINOR)** **src**: make napi_create_reference accept symbol (JckXia) [#&#8203;39926](nodejs/node#39926)
-   \[[`b050c65885`](nodejs/node@b050c65885)] - **src**: add option to disable loading native addons (Dominic Elm) [#&#8203;39977](nodejs/node#39977)
-   \[[`c1695ac68a`](nodejs/node@c1695ac68a)] - **tools**: update certdata.txt (Richard Lau) [#&#8203;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.

---

 - [ ] <!-- rebase-check -->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]>
@haoqunjiang haoqunjiang mentioned this pull request Jul 24, 2022
Running pnpm run build in Windows fails vitejs/vite#9328
Closed
7 tasks done
@targos targos removed the backport-open-v14.x label May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ljharb ljharb ljharb left review comments
@GeoffreyBooth GeoffreyBooth GeoffreyBooth left review comments
@guybedford guybedford guybedford approved these changes
@aduh95 aduh95 aduh95 approved these changes
+1 more reviewer
@Mesteery Mesteery Mesteery left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. esm Issues and PRs related to the ECMAScript Modules implementation. experimental Issues and PRs related to experimental features. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

process.exit() in ESM results in exit code 13

Uh oh!

There was an error while loading. Please reload this page.

9 participants

@MoonBall @nodejs-github-bot @aduh95 @MylesBorins @ljharb @GeoffreyBooth @guybedford @Mesteery @targos
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.