tapjs / foreground-child

Run a child as if it's the foreground process. Give it stdio. Exit when it exits.
http://tapjs.github.io/foreground-child/
ISC License
39 stars 14 forks source link

v2 throws with invalid arg type #57

Open 73rhodes opened 3 months ago

73rhodes commented 3 months ago

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 commented 3 months ago

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 commented 1 month ago

See https://github.com/istanbuljs/nyc/pull/1546, I'm actively working to move nyc to using foreground-child@^3.0.0.