sometimes dead loop

Version

forground-child@2.0.0
node v20.8.0

When

sometimes, not reproducible

Error

100% CPU, guess somehow dead loop
After Ctrl-C

TypeError [ERR_INVALID_ARG_TYPE]: The "code" argument must be of type number. Received type string ('128SIGINT')
    at process.set [as exitCode] (node:internal/bootstrap/node:123:9)
    at ChildProcess.<anonymous> (/home/vscode/.yarn/berry/cache/foreground-child-npm-2.0.0-80c976b61e-8.zip/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'
}

Node.js v20.8.0

@isaacs I'm willing to open a PR to fix this, I'd like to understand the intention behind the close handler though. I can't see how this ever worked, since (from what I can tell) the second argument to the close handler has always been a string, so this certainly never would have done what it seems like it was supposed to. My guess is this became an error as of nodejs/node#43716.

There are at least two approaches we could take to fix this:

Ignore the value of the signal argument to the close handler; if it's non-null, set exitCode to 128, 1, or some other non-zero value. This is simple, but it'd violate the Unix convention that the existing code was trying to maintain.
Use a mapping of signal names (https://github.com/DefinitelyTyped/DefinitelyTyped/blob/39359c8466846541b5b3298165ef79c509f0cd09/types/node/v16/process.d.ts#L63-L100) to their corresponding number. This could be done with a map directly in this project, or one could use a third-party package like https://www.npmjs.com/package/human-signals.

I lean towards the second option with human-signals, but I'd appreciate input from a maintainer before I open a PR. EDIT: My guess is that if we want maximum backwards-compatibility, we should use the first option and set process.exitCode = 128, since I think that is what 128SIGINT would have been coerced to.

No pull request is needed. This is already working fine on v3, the bug is only in v2. The PR needs to be made to the consumers of fg child.

Thanks for letting me know, I didn't check whether the link in OP's issue corresponded to the latest version of foreground-child 🤦

I'll PR the latest version into nyc. Thanks again (and thanks as always for maintaining so much of the Node ecosystem ❤️)

Version

When

Error

Related