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

v2 throws with invalid arg type #57

Open
73rhodes opened this issue Mar 16, 2024 · 2 comments
Open

v2 throws with invalid arg type #57

73rhodes opened this issue Mar 16, 2024 · 2 comments

Comments

@73rhodes
Copy link

73rhodes commented Mar 16, 2024

Context

As part of an effort to augment mocha toolchains with standard posix exit codes, we found a key component of our toolchain (nyc) use foreground-child@2.0.0 which manifests this issue.

the error

foreground-child@2.0.0 can throw an invalid arg type error under certain conditions:

throw new ERR_INVALID_ARG_TYPE(name, 'number', value);
^

TypeError: The "code" argument must be of type number. Received type string ('128SIGABRT')
    at process.set [as exitCode] (node:internal/bootstrap/node:123:9)
    at ChildProcess.<anonymous> (/Users/foo/bar/node_modules/nyc/node_modules/foreground-child/index.js:63:22)
    at ChildProcess.emit (node:events:514:28)
    at maybeClose (node:internal/child_process:1105:16)
    at ChildProcess._handle.onexit (node:internal/child_process:305:5) {
  code: 'ERR_INVALID_ARG_TYPE'
}

root cause

This line assigns a value 128 + signal, resulting in a string eg. "128SIGABRT" instead of a numeric value.

process.exitCode = signal ? 128 + signal : code;

suggested fix

Use os.constants.signals for numerical values of signal strings; eg.

if (typeof signal === 'string') {
    process.exitCode = signal ? 128 + require('os').constants.signals[signal] : code;
} else {
    process.exitCode = signal ? 128 + signal : code;
}

reason to back-patch

Since foreground-child: ^2.0.0 is used by the latest version of nyc it would be helpful to patch v2.0.0 to fix the issue for projects that won't upgrade to foreground-child@3.x.x right away.

steps to reproduce

Given the test file oom.unit.js

describe('OOM error', () => {
  it('should cause an oom error', async () => {
    const x = [];
    while (true) {
      x.push({a: '123'.repeat(1000000) });
    }
  });
});

Running the following command reproduces the error

nyc mocha src/test/server/oom.unit.js
@73rhodes
Copy link
Author

73rhodes commented Mar 18, 2024

I can't open a PR against a tag, but the steps would just be:

git co v2.0.0
Make code changes...
git add -A
git commit -m "Fixed signal handling bug."
git tag -a -m "Tag version 2.0.1, a bugfix release" v2.0.1
git push --tags

... then npm publish.

@nwalters512
Copy link

See istanbuljs/nyc#1546, I'm actively working to move nyc to using foreground-child@^3.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants