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

subprocess.stdout can be undefined instead of null #43905

Closed
unarist opened this issue Jul 20, 2022 · 2 comments · Fixed by #43910
Closed

subprocess.stdout can be undefined instead of null #43905

unarist opened this issue Jul 20, 2022 · 2 comments · Fixed by #43910
Labels
child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations.

Comments

@unarist
Copy link

unarist commented Jul 20, 2022

Affected URL(s)

https://nodejs.org/docs/latest-v16.x/api/child_process.html#subprocessstdout

Description of the problem

The subprocess.stdout property can be null if the child process could not be successfully spawned.

However, it can be undefined in some cases:

$ docker run --rm -it node:16 /bin/bash
root@c12d92a299db:/# ulimit -n 17
root@c12d92a299db:/# node -e 'p=require("child_process").spawn("uname");p.stdout.on("data",console.log)'
[eval]:1
p=require("child_process").spawn("uname");p.stdout.on("data",console.log)
                                                   ^

TypeError: Cannot read properties of undefined (reading 'on')
    at [eval]:1:52
    at Script.runInThisContext (node:vm:129:12)
    at Object.runInThisContext (node:vm:305:38)
    at node:internal/process/execution:76:19
    at [eval]-wrapper:6:22
    at evalScript (node:internal/process/execution:75:60)
    at node:internal/main/eval_string:27:3
root@c12d92a299db:/# node -e 'console.log(require("child_process").spawn("uname"))'
<ref *1> ChildProcess {
  _events: [Object: null prototype] {},
  _eventsCount: 0,
  _maxListeners: undefined,
  _closesNeeded: 1,
  _closesGot: 0,
  connected: false,
  signalCode: null,
  exitCode: null,
  killed: false,
  spawnfile: 'uname',
  _handle: Process {
    onexit: [Function (anonymous)],
    [Symbol(owner_symbol)]: [Circular *1]
  },
  spawnargs: [ 'uname' ],
  [Symbol(kCapture)]: false
}
node:events:505
      throw er; // Unhandled 'error' event
      ^

Error: spawn uname EMFILE
    at Process.ChildProcess._handle.onexit (node:internal/child_process:283:19)
    at onErrorNT (node:internal/child_process:478:16)
    at processTicksAndRejections (node:internal/process/task_queues:83:21)
Emitted 'error' event on ChildProcess instance at:
    at Process.ChildProcess._handle.onexit (node:internal/child_process:289:12)
    at onErrorNT (node:internal/child_process:478:16)
    at processTicksAndRejections (node:internal/process/task_queues:83:21) {
  errno: -24,
  code: 'EMFILE',
  syscall: 'spawn uname',
  path: 'uname',
  spawnargs: []
}

I think this should be documented.

@unarist unarist added the doc Issues and PRs related to the documentations. label Jul 20, 2022
@cola119
Copy link
Member

cola119 commented Jul 20, 2022

This is because ChildProcess.prototype.spawn returns the error before ChildProcess.stdout is initialized to null at L457.

const err = this._handle.spawn(options);
// Run-time errors should emit an error, not throw an exception.
if (err === UV_EACCES ||
err === UV_EAGAIN ||
err === UV_EMFILE ||
err === UV_ENFILE ||
err === UV_ENOENT) {
process.nextTick(onErrorNT, this, err);
// There is no point in continuing when we've hit EMFILE or ENFILE
// because we won't be able to set up the stdio file descriptors.
if (err === UV_EMFILE || err === UV_ENFILE)
return err;

Which is better to document it can be null and undefined, or to fix to initialize ChildProcess.stdout to null as an initial value at the top of the function?

@VoltrexKeyva VoltrexKeyva added the child_process Issues and PRs related to the child_process subsystem. label Jul 20, 2022
@unarist
Copy link
Author

unarist commented Jul 21, 2022

I think it would be better if stdout has same value when the spawn can't create pipes, for consistency and maybe ease to handle.

I'm okay for documenting the behavior, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants