Sitelet https://web.archive.org/web/20230724162014/https://github.com/nodejs/node/issues/35125
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

Jump to bottom

child_process.execSync should return object with std of plain stdout #35125

Closed
Jokero opened this issue Sep 9, 2020 · 5 comments
Closed

child_process.execSync should return object with std of plain stdout #35125

Jokero opened this issue Sep 9, 2020 · 5 comments
Labels
child_process Issues and PRs related to the child_process subsystem. feature request Issues that request new features to be added to Node.js. stale

Comments

@Jokero
Copy link

Jokero commented Sep 9, 2020

Is your feature request related to a problem? Please describe.
I wanted to run sequentially a series of processes and check their exit code and stderr to make a decision whether the run command failed or not. In my case exit code=0 and anything in stderr having warning substring should be considered as fail.

When I needed to run just one command I happily used the following code:

childProcess.exec(command, (err, stdout, stderr) => {
    if (err || (stderr && stderr.toLowerCase().includes('warning'))) {
        console.error('Failed due to:');
        console.error(stderr);
        process.exit(1);
    }

    console.log('OK\n');
    process.exit(0);
});

When the task changed and I needed to run few commands in a row I decided to use synchronous version of that method:

    try {
        const stdout = childProcess.execSync(command, { encoding: 'utf8' }); // can't get stderr
    } catch (err) {
        const { status, stderr } = err;
        if (status > 0 || (stderr && stderr.toLowerCase().includes('warning'))) {
            console.error('Failed due to:');
            console.error(stderr);
            process.exit(1);
        }
    }

    console.log('OK');

However using that code I can't check stderr when no exception is thrown.

Describe the solution you'd like
I'd like childProcess.execSync to return an object containing status/code, stderr, stdout and other fields similar to returned object in child_process.spawnSync (https://nodejs.org/api/child_process.html#child_process_child_process_spawnsync_command_args_options):

* pid <number> Pid of the child process.
* output <Array> Array of results from stdio output.
* stdout <Buffer> | <string> The contents of output[1].
* stderr <Buffer> | <string> The contents of output[2].
* status <number> | <null> The exit code of the subprocess, or null if the subprocess terminated due to a signal.
* signal <string> | <null> The signal used to kill the subprocess, or null if the subprocess did not terminate due to a signal.
* error <Error> The error object if the child process failed or timed out.

Describe alternatives you've considered
Alternatives:

  1. Use asynchronous version (childProcess.exec) and rewrite the code to work in asynchronous manner
  2. Use childProcess.spawnSync with shell=true
@himself65 himself65 added the child_process Issues and PRs related to the child_process subsystem. label Sep 10, 2020
@himself65
Copy link
Member

I think this might be a good idea, which allows execSync to run silently (without throwing err). But It will have breaking changes. Could we pass options.skipThrow to allow this feature?

example:

const { stdout, stderr, status } = cp.execSync('xxx', { skipThrow: true })

@himself65 himself65 added the feature request Issues that request new features to be added to Node.js. label Sep 12, 2020
@himself65 himself65 changed the title child_process.execSync should return object with stdout/stderr/status/... instead of plain stdout child_process.execSync should return object with std of plain stdout Sep 12, 2020
@Jokero
Copy link
Author

Jokero commented Sep 12, 2020

@himself65 Sounds as a good compromise. With this flag the behaviour will be similar to spawnSync which does not throw. And we also need to take into account execFileSync behaving the same way as execSync (it returns stdout and throws an error)

@ling1726 ling1726 mentioned this issue Feb 8, 2022
Merged
@github-actions
Copy link
Contributor

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.

@github-actions github-actions bot added the stale label Mar 18, 2022
@github-actions
Copy link
Contributor

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

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

@colinparsonsme
Copy link

Leaving a comment because this would still be a useful feature! In my case, I'm building out a test suite for an npm package that runs from the command line, and it's nice being able to run my happy path integration tests with expect(execSync("npm run command -- option1=1 option2=2")).toEqual(something).

But I can't do the same for my sad path integration tests; the same code will just throw error Command failed: command instead of the actual stdout, stderr, or status of command. Since the error message is what I want to test in the first place, I end up using expect(spawnSync('npm', ['run', 'command', '--', 'option1=1', 'option2=2'].stdout.toString()).toEqual(something) for sad path tests, and execSync for my happy path tests. It'd be nice to be able to just use execSync for everything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
child_process Issues and PRs related to the child_process subsystem. feature request Issues that request new features to be added to Node.js. stale
Projects
Node.js feature requests
Status: Stale
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Jokero @himself65 @colinparsonsme