Add Promise-based versions for some functions in child_process

[thernstig]

What is the problem this feature will solve?

It would be nice if child_process functionality, such as exec and execFile, had Promised based versions by default. Currently it does not, see https://nodejs.org/api/child_process.html

Note that only child_process functions that can be async/await should be considered. This does not include things such as fork or spawn.

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

A new child_process/promises import path that can be used similar to how fs promises are used:

// Using ESM Module syntax:
import { exec } from 'child_process/promises';

try {
  const { stdout } = await exec(
    'sysctl -n net.ipv4.ip_local_port_range'
  );
  console.log('successfully executed the child process command');
} catch (error) {
  console.error('there was an error:', error.message);
}

The ask is basically that child_process/promises could just return a variant such as const exec = util.promisify(process.exec); as a convenience.

What alternatives have you considered?

I am already using const exec = util.promisify(process.exec); but that is not as nice. And this new proposal follows along what is happening in other Node.js APIs.

(The original ask in https://github.com/nodejs/node/issues/38823 was perceived as asking for the entire module and all APIs to be async/await. This issue here narrows it down to only focus on the functions that can be.)

[mifi]

I would like to see execFile have a promisified counterpart as well, because very often I need to provide dynamic arguments to a command, and in that case, exec can be unsafe and vulnerable to arbitrary command execution.

[bnoordhuis]

FTR, that already exists, it's why you can util.promisify(child_process.execFile).

General expectation management: open a pull request if you want to see this happen. exec and execFile have util.promisify.custom hooks that can be repurposed.

[thernstig]

I would like to see execFile have a promisified counterpart as well, because very often I need to provide dynamic arguments to a command, and in that case, exec can be unsafe and vulnerable to arbitrary command execution.

That was the intention, and I added execFile as well as an example to the main post. Rationale being it is most often more safe to use, so good to highlight it as another example.

[github-actions[bot]]

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

[thernstig]

Great APIs makes devs happy, so bumping.

[avivkeller]

I may be misunderstanding this, but doesn't child_process.execSync / child_process.spawnSync achieve this?

[thernstig]

I may be misunderstanding this, but doesn't child_process.execSync / child_process.spawnSync achieve this?

You are misunderstanding. The functions you list are sync, not async. I think you need to read up a bit more about this in the linked documents in the original post.

[dead-claudia]

Got a (mostly) concrete proposal in https://github.com/nodejs/node/issues/54799.

[thernstig]

@dead-claudia does that proposal solve it with examples like await exec() versions? It was not immediately obvious in the PR description.

[dead-claudia]

@thernstig Here's the example from the initial comment, adapted to my proposal:

// Shell script
import { system } from 'child_process/promises';
import { BufferWriter } from 'stream';

const stderr = new BufferWriter(1000);

try {
  const stdout = new BufferWriter(1000);
  await system(
    'sysctl -n net.ipv4.ip_local_port_range',
    {fds: {1: stdout, 2: stderr}},
  );
  console.log('successfully executed the child process command');
} catch (error) {
  console.error(`there was an error: ${stderr}`);
}

// Direct invocation using `$PATH`
import { exec } from 'child_process/promises';
import { BufferWriter } from 'stream';

const stderr = new BufferWriter(1000);

try {
  const stdout = new BufferWriter(1000);
  await exec(
    'sysctl -n net.ipv4.ip_local_port_range',
    {fds: {1: stdout, 2: stderr}},
  );
  console.log('successfully executed the child process command');
} catch (error) {
  console.error(`there was an error: ${stderr}`);
}

[thernstig]

Ah, but the initial example was using exec and those examples are not, but I see you have exec as part of https://github.com/nodejs/node/issues/54799.

[dead-claudia]

@thernstig Oh, I was toying around with names for exec (started with start and then switched to spawn), forgot to clean up the mess in the issue I linked, and then mentally copypasta'd the mistake here. My bad! 😅

[github-actions[bot]]

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the https://github.com/nodejs/node/labels/never-stale label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document.

[thernstig]

There has been activity, with several new upvotes.

[github-actions[bot]]

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the https://github.com/nodejs/node/labels/never-stale label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document.

[dead-claudia]

Should https://github.com/nodejs/node/issues/54799 be deduped into this? It's a concrete API proposal for this.

[otakutyrant]

So this PR is not merged yet? My AI thought that child_process/promises exists already!

[dead-claudia]

@otakutyrant What PR? The last one I'm aware of was closed over a year ago from being too stale. If you also got that (mis)info from your AI, you should probably look for a better model. 😉