Add Promise-based closed(), exited(), and succeeded() to ChildProcess

[dfabulich]

What is the problem this feature will solve?

When working with the spawn API, you need to add listeners for the error event, as well as the close event and/or the exit event.

The code can be quite verbose:

async function spawnPromise(command, args, options) {
    await new Promise((resolve, reject) => {
        const child = spawn(command, args, options);
        let errored = false;
        child.once('error', e => {
            errored = true;
            reject(e);
        });
        child.once('close', (code) => {
            if (errored) return;
            if (code === 0) {
                resolve();
            } else {
                const error = new Error('spawn failed with error code: ' + code);
                reject(error);
            }
        })
    })
}

What is the feature you are proposing to solve the problem?

close and exit are one-time events, so it would be appropriate to make Promise-based helpers for them:

• closed(): Promise<ChildProcess>: Resolves to the child process when the close event fires, rejects with an error if the error event fires.
• exited(): Promise<ChildProcess>: Resolves to the child process when the exit event fires, rejects with an error if the error event fires.
• succeeded(): Promise<void>: Resolves void when the child process closes with 0 exit code; rejects with an error otherwise. (As I'm imagining it, succeeded() would resolve to void, because there's nothing left to do with a known-successful child process. You know its exit code (0). None of its methods or properties are meaningful. It's finished.)

Then, you could write one of these:

await spawn(command, arguments, { stdio: 'inherit' }).succeeded();
const {exitCode, signalCode} = await spawn(command, arguments, { stdio: 'inherit' }).closed();
const {exitCode, signalCode} = await spawn(command, arguments, { stdio: 'inherit' }).exited();

What alternatives have you considered?

• EDIT: @Renegade334 suggested using events.once to clean up the sample code in my problem statement.

  import {spawn} from 'node:child_process'
  import {once} from 'node:events'
  
  async function spawnPromise(command, args, options) {
      const [code] = await once(spawn(command, args, options), 'close');
      if (code !== 0) throw new Error('spawn failed with error code: ' + code);
  }

  This saps a lot of my enthusiasm for .closed() and .exited(), but I think .succeded() could still be really good.

• EDIT: If we only wanted to add .succeeded(), maybe it would be better to simply make ChildProcess thenable. You could then simply await a child process. I've provided a prollyfill.

  import {spawn, ChildProcess} from 'node:child_process'
  import {once} from 'node:events'
  
  if (!ChildProcess.prototype.then) {
      Object.defineProperty(ChildProcess.prototype, 'then', {
          value: function then(onFulfilled, onRejected) {
              const p = (async () => {
                  const exitCode = this.exitCode ?? (await once(this, 'close'))[0];
  
                  if (exitCode === 0) return;
                  throw new Error("Process failed with exit code: " + exitCode);
              })();
  
              return p.then(onFulfilled, onRejected);
          },
      });
  }
  
  // usage
  await spawn(command, {stdio: 'inherit'});
  
  // with stdout
  let ls = spawn('ls');
  ls.stdout.on('data', (data) => {
    console.log(`stdout: ${data}`);
  });
  await ls;

• We could add a custom promisifier to spawn, like we have for exec and execFile. But spawn has more than one way to end (closing vs. exiting) and no Node errback parameter, so it's not a clean fit.

• Add a cleaned up child_processes API that cleans the API up for child process spawning, like how fs/promises cleaned up fs #54799 documents a thorough "cleanup" of the API. This sounds difficult to build consensus around; it's gotten kinda stale, and cleans up all sorts of things.

• In the thread on the issue above, @benjamingr suggested adding a .text() convenience to ChildProcess.

  const child = spawn('ls', ['-lh', '/usr']); // reasonable, creates a child process
  // Now I want to read its stdout and wait for it to close, this is verbose:
  const output = Buffer.concat(await child.stdout.toArray()).toString();
  // What if instead we could do:
  const output2 = await child.text();
  // Or as a one liner
  console.log(await spawn('ls', ['-lh', '/usr']).text());

  In my opinion, the promisified execFile is already "good enough" for buffered output.

  const {stdout} = await promisify(execFile)('ls', ['-lh', '/usr']);

  It's spawn that I care about, because that's the only asynchronous version of the API that supports stdio: 'inherit'. Today, If I want stdio: inherit, I have to go fiddling around with spawn callbacks.

[dfabulich]

I'm prepared to file a PR for this, but, before I do, I'd appreciate some words of wisdom from a Node maintainer about whether this PR would be controversial.

(I've had bad experiences filing small PRs on Node that require months of consensus-building, escalation to TSC, etc.)

[Renegade334]

It's worth pointing out that "give me a promise that resolves if a given event fires, or rejects if "error" is emitted first" is the exact functionality that events.once currently offers.

[dfabulich]

TIL that events.once handles error events! I'm going to file a PR to make the documentation recommend events.once.

I guess my example function at the top could be rewritten:

import {spawn} from 'node:child_process'
import {once} from 'node:events'

async function spawnPromise(command, args, options) {
    const [code] = await once(spawn(command, args, options), 'close');
    if (code !== 0) throw new Error('spawn failed with error code: ' + code);
}

That's about as good as promisify, IMO, and saps most of my enthusiasm for .closed() and .exited().

I think it would still be nice to add .succeeded() to ChildProcess. But, if we're talking about just one promise-returning method, maybe we could get away with making ChildProcess thenable.

Here's a little prollyfill:

if (!ChildProcess.prototype.then) {
    Object.defineProperty(ChildProcess.prototype, 'then', {
        value: function then(onFulfilled, onRejected) {
            const p = (async () => {
                const exitCode = this.exitCode ?? (await once(this, 'close'))[0];

                if (exitCode === 0) return;
                throw new Error("Process failed with exit code: " + exitCode);
            })();

            return p.then(onFulfilled, onRejected);
        },
    });
}

Then you could simply await spawn('ls', {stdio: inherit}). Or, if you want stdout:

const ls = spawn('ls', ['-lh', '/usr']);
ls.stdout.on('data', (data) => {
  console.log(`stdout: ${data}`);
});
await ls;