ariya / phantomjs

Scriptable Headless Browser
http://phantomjs.org
BSD 3-Clause "New" or "Revised" License
29.46k stars 5.76k forks source link

callback method of "execFile" function inside "child_process" module should add exitCode as a parameter #12689

Closed machinewu closed 4 years ago

machinewu commented 9 years ago

I have a case of using "child_process" module performs the shell command "zip" to compress files. If the file "/tmp/test.dat" not exists (For the time being without "fs" module to detect if the file exists):

var execFile = require("child_process").execFile;
execFile("/usr/bin/zip", ["-j", "test.zip", "/tmp/test.dat"], null, function (err, stdout, stderr) {
    console.log("execFileSTDOUT:", JSON.stringify(stdout));
    console.log("execFileSTDERR:", JSON.stringify(stderr));
});

It will show

execFileSTDOUT: "\tzip warning: name not matched: /tmp/test.dat\n\nzip error: Nothing to do! (test.zip)\n" execFileSTDERR: ""

But if I run the following command:

zip -j test.zip /tmp/test.dat
echo returnCode=$?

It will show

returnCode=12

So, this case cannot use only stdout or stderr to detect errors occurred.(Although I can match the "error" or "warn" or other keywords, but I think This is not a good solution.)

I think "execFile" function's "callback" method should add exitCode as a parameter for slove this case. I found the source code in child_process.js at line 91:

    ctx.on("exit", function (code) {
      return cb(null, stdout, stderr)
    })

"callback" method's first parameter is set null! This is a waste of an argument! Change the first parameter to exitcode, I think it would be better.

stale[bot] commented 4 years ago

Due to our very limited maintenance capacity, we need to prioritize our development focus on other tasks. Therefore, this issue will be automatically closed (see #15395 for more details). In the future, if we see the need to attend to this issue again, then it will be reopened. Thank you for your contribution!