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.

Sign up for GitHub

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

Jump to bottom

unexpected breaking changes with process.exitCode/process.exit() #45683

Open
nlf opened this issue Nov 29, 2022 · 2 comments · May be fixed by #49579
Open

unexpected breaking changes with process.exitCode/process.exit() #45683

nlf opened this issue Nov 29, 2022 · 2 comments · May be fixed by #49579
Labels
confirmed-bug Issues with confirmed bugs.

Comments

@nlf
Copy link
Contributor

nlf commented Nov 29, 2022 •
edited

Version

20.0.0-pre (nightly)

Platform

Darwin dib.lan 22.1.0 Darwin Kernel Version 22.1.0: Sun Oct 9 20:14:30 PDT 2022; root:xnu-8792.41.9~2/RELEASE_ARM64_T8103 arm64

Subsystem

process

What steps will reproduce the bug?

const assert = require('assert')

process.exitCode = 2
delete process.exitCode // REMOVE
assert(process.exitCode === undefined) // REMOVE
process.exit(undefined)

How often does it reproduce? Is there a required condition?

reproduces on all node versions built after landing #44711

What is the expected behavior?

the assertion should pass, and if the two lines marked REMOVE are commented out the process should exit with code 2

What do you see instead?

the assertion throws, and if the two lines marked REMOVE are commented out the process exits with code 0

Additional information

No response
@nlf nlf mentioned this issue Nov 29, 2022
Merged
@ljharb ljharb mentioned this issue Nov 29, 2022
Merged
@daeyeon
Copy link
Member

daeyeon commented Nov 30, 2022

This is a missing use case when the PR landed.

@daeyeon daeyeon added the confirmed-bug Issues with confirmed bugs. label Nov 30, 2022
@daeyeon daeyeon mentioned this issue Nov 30, 2022
Closed
ljharb added a commit to ljharb/node that referenced this issue Sep 9, 2023
@ljharb
process: make process.exitCode configurable again
50e7c3b 
This change was done in nodejs#44711, and it's not clear it was intentional. It caused nodejs#45683, and also makes it impossible to mock out the exitCode in tests.

Filing this PR per nodejs#44711 (comment)

Fixes nodejs#45683.
@ljharb ljharb mentioned this issue Sep 9, 2023
Closed
ljharb added a commit to ljharb/node that referenced this issue Sep 9, 2023
@ljharb
process: make exitCode configurable again
6bff7d9 
This change was done in nodejs#44711, and it's not clear it was intentional. It caused nodejs#45683, and also makes it impossible to mock out the exitCode in tests.

Filing this PR per nodejs#44711 (comment)

Fixes nodejs#45683.
@ljharb ljharb linked a pull request Sep 9, 2023 that will close this issue
Open
@ljharb
Copy link
Member

ljharb commented Sep 9, 2023

Filed #49579 to fix this.

ljharb added a commit to ljharb/node that referenced this issue Sep 9, 2023
@ljharb
process: make exitCode configurable again
f7f938b 
This change was done in nodejs#44711, and it's not clear it was intentional.
It caused nodejs#45683, and also makes it impossible to mock out the exitCode
in tests.

Filing this PR per nodejs#44711 (comment)

Fixes nodejs#45683.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
confirmed-bug Issues with confirmed bugs.
Projects
None yet
Milestone
No milestone
3 participants
@ljharb @nlf @daeyeon