sometimes dead loop

[loynoir]

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

Related

Maybe caused by below line.

foreground-child/index.js

Line 63 in efba8bb

process.exitCode = signal ? 128 + signal : code

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/39359c8466846541b5b3298165ef79c509f0cd09/types/node/child_process.d.ts#L532

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/39359c8466846541b5b3298165ef79c509f0cd09/types/node/v16/process.d.ts#L63-L100

The signal should be null or some const string.

[loynoir]

Although node v21 seems no c8 sometimes hangs forever problem, but according to the types, should leave it open.

[SSANSH]

+1

[nwalters512]

@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.

[isaacs]

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.

[nwalters512]

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 ❤️)