src: consolidate exit codes in the code base#44746

joyeecheung · 2022-09-22T16:07:11Z

Add an ExitCode enum class and use it throughout the code base instead of hard-coding the exit codes everywhere. At the moment, the exit codes used in many places do not actually conform to what the documentation describes. With the new enums (which are also available to the JS land as constants in an internal binding) we could migrate to a more consistent usage of the codes, and eventually expose the constants to the user land when they are stable enough.

nodejs-github-bot · 2022-09-22T16:07:15Z

Review requested:

@nodejs/gyp
@nodejs/modules
@nodejs/startup

lib/internal/cluster/child.js

aduh95

I think this is a neat idea! A few nits:

lib/internal/process/promises.js

lib/internal/main/watch_mode.js

lib/internal/debugger/inspect.js

JakobJingleheimer

Oh sweet! Thanks for this.

src/node.cc

src/env.cc

Add an ExitCode enum class and use it throughout the code base instead of hard-coding the exit codes everywhere. At the moment, the exit codes used in many places do not actually conform to what the documentation describes. With the new enums (which are also available to the JS land as constants in an internal binding) we could migrate to a more consistent usage of the codes, and eventually expose the constants to the user land when they are stable enough.

Trott

Failing tests seem relevant.

nodejs-github-bot · 2022-09-29T08:19:14Z

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

nodejs-github-bot · 2022-09-29T12:17:48Z

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

nodejs-github-bot · 2022-10-03T10:31:33Z

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

… code base

nodejs-github-bot · 2022-10-04T13:10:26Z

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

nodejs-github-bot · 2022-10-04T17:05:32Z

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

joyeecheung · 2022-10-05T11:29:41Z

Fixed the Windows failure @jasnell @Trott @aduh95 @JakobJingleheimer can you take a look again? Thanks!

JakobJingleheimer

👍 I can't speak to the C change aside from I'm fairly sure it's innocuous, and the fix is pretty clearly working.

src/node_exit_code.h

RaisinTen

LGTM

nodejs-github-bot · 2022-10-06T12:41:17Z

Landed in be525d7

danielleadams · 2022-10-11T13:05:25Z

@joyeecheung do you mind backporting this to v18.x? It landed with conflicts.

joyeecheung · 2022-10-14T13:53:19Z

From the results of git node backport preferably these commits should be backported first

7174654 src: create BaseObject with node::Realm, src: create BaseObject with node::Realm #44348
node.gyp
src/api/embed_helpers.cc
src/env-inl.h
src/env.cc
src/env.h
src/node_snapshotable.cc
81d9cdb src: introduce node::Realm, src: introduce node::Realm #44179
node.gyp
src/api/environment.cc
src/env-inl.h
src/env.cc
src/env.h
src/node.cc
src/node_internals.h
src/node_main_instance.cc
src/node_snapshotable.cc
d70aab6 src: support WeakReference in snapshot, src: support WeakReference, diagnostics_channel and net in the snapshot #44193
node.gyp
src/node_snapshotable.cc
7850802 bootstrap: generate bootstrapper arguments in BuiltinLoader, bootstrap: generate bootstrapper arguments in BuiltinLoader #44488
src/api/environment.cc
src/node.cc
4ca398a src: iterate over base objects to prepare for snapshot, src: iterate over base objects to prepare for snapshot #44192
src/env-inl.h
src/env.cc
src/env.h
src/node_process_methods.cc
src/node_snapshotable.cc
5291096 bootstrap: check more metadata when loading the snapshot, bootstrap: check more metadata when loading the snapshot #44132
src/env.h
src/node.cc
src/node_internals.h
src/node_snapshotable.cc
8b0e5b1 src: fix cppgc incompatibility in v8, src: fix cppgc incompatibility in v8 #43521
src/node_process_methods.cc
src/node_snapshotable.cc
dabda03 src: per-environment time origin value, src: per-environment time origin value #43781
src/node_worker.cc

I'll see if I can prepare a PR with these

Refs: nodejs#44746

Refs: #44746 PR-URL: #45841 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Sep 22, 2022

joyeecheung requested a review from Trott September 22, 2022 16:07

ljharb reviewed Sep 22, 2022
View reviewed changes

lib/internal/cluster/child.jsShow resolvedHide resolved

aduh95 approved these changes Sep 23, 2022
View reviewed changes

lib/internal/process/promises.js Outdated Show resolvedHide resolved
lib/internal/main/watch_mode.js Outdated Show resolvedHide resolved
lib/internal/debugger/inspect.js Outdated Show resolvedHide resolved

JakobJingleheimer approved these changes Sep 23, 2022
View reviewed changes

daeyeon reviewed Sep 23, 2022
View reviewed changes

src/node.cc Outdated Show resolvedHide resolved
src/env.ccShow resolvedHide resolved

joyeecheung force-pushed the exit-codes branch from dc81538 to c49c504Compare September 28, 2022 12:12

fixup! src: consolidate exit codes in the code base
b439af8

joyeecheung force-pushed the exit-codes branch from c49c504 to b439af8Compare September 28, 2022 12:13

github-actionsbot mentioned this pull request Sep 29, 2022
CI Reliability 2022-09-29 nodejs/reliability#388
Open
16 tasks

Trott reviewed Sep 29, 2022
View reviewed changes

fixup! fixup! src: consolidate exit codes in the code base
00e91e3

fixup! fixup! fixup! src: consolidate exit codes in the code base
dbb1e5a

This was referenced Sep 30, 2022
CI Reliability 2022-09-30 nodejs/reliability#390
Open
CI Reliability 2022-10-01 nodejs/reliability#391
Open
CI Reliability 2022-10-02 nodejs/reliability#392
Open

jasnell approved these changes Oct 2, 2022
View reviewed changes

github-actionsbot mentioned this pull request Oct 3, 2022
CI Reliability 2022-10-03 nodejs/reliability#393
Open
9 tasks

fixup! fixup! fixup! fixup! src: consolidate exit codes in the code base
15d3fde

github-actionsbot mentioned this pull request Oct 4, 2022
CI Reliability 2022-10-04 nodejs/reliability#394
Open
16 tasks

fixup! fixup! fixup! fixup! fixup! src: consolidate exit codes in the…
bd027f1
… code base

github-actionsbot mentioned this pull request Oct 5, 2022
CI Reliability 2022-10-05 nodejs/reliability#395
Open
19 tasks

JakobJingleheimer approved these changes Oct 5, 2022
View reviewed changes

github-actionsbot mentioned this pull request Oct 6, 2022
CI Reliability 2022-10-06 nodejs/reliability#396
Open
25 tasks

RaisinTen reviewed Oct 6, 2022
View reviewed changes

src/node_exit_code.hShow resolvedHide resolved
src/node_exit_code.hShow resolvedHide resolved

RaisinTen approved these changes Oct 6, 2022
View reviewed changes

joyeecheung added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Oct 6, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 6, 2022

nodejs-github-bot merged commit be525d7 into nodejs:mainOct 6, 2022

This was referenced Oct 7, 2022
CI Reliability 2022-10-07 nodejs/reliability#397
Open
CI Reliability 2022-10-08 nodejs/reliability#398
Open
CI Reliability 2022-10-09 nodejs/reliability#399
Open
CI Reliability 2022-10-10 nodejs/reliability#400
Open

danielleadams added the backport-requested-v18.x label Oct 11, 2022

RafaelGSS mentioned this pull request Oct 14, 2022
2022-10-18, Version 19.0.0 (Current) #44626
Merged

VoltrexKeyva mentioned this pull request Nov 1, 2022
test_runner: avoid swallowing of asynchronously thrown errors #45264
Merged

debadree25 added a commit to debadree25/node that referenced this pull request Dec 13, 2022
lib,src: update exit codes as per todos
bc5f1de
Refs: nodejs#44746

debadree25 mentioned this pull request Dec 13, 2022
lib,src: update exit codes #45841
Merged

debadree25 added a commit to debadree25/node that referenced this pull request Jan 3, 2023
lib,src: update exit codes as per todos
e979977
Refs: nodejs#44746

debadree25 added a commit to debadree25/node that referenced this pull request Feb 19, 2023
lib,src: update exit codes as per todos
057da6e
Refs: nodejs#44746

jakepruitt mentioned this pull request Mar 13, 2024
Handle new ProcessGlobalArgsInternal function in node >19 nexe/nexe#1086
Merged

Uh oh!

src: consolidate exit codes in the code base#44746

src: consolidate exit codes in the code base #44746

Uh oh!

Conversation

joyeecheung commented Sep 22, 2022

Uh oh!

nodejs-github-bot commented Sep 22, 2022

Uh oh!

Uh oh!

aduh95 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JakobJingleheimer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Trott left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Sep 29, 2022

Uh oh!

nodejs-github-bot commented Sep 29, 2022

Uh oh!

nodejs-github-bot commented Oct 3, 2022

Uh oh!

nodejs-github-bot commented Oct 4, 2022

Uh oh!

nodejs-github-bot commented Oct 4, 2022

Uh oh!

joyeecheung commented Oct 5, 2022

Uh oh!

JakobJingleheimer left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

RaisinTen left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Oct 6, 2022

Uh oh!

danielleadams commented Oct 11, 2022

Uh oh!

joyeecheung commented Oct 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

Trott left a comment •
edited
Loading

JakobJingleheimer left a comment •
edited
Loading