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

child_process.execFile returns strings where doc says it should return a Buffer #3389

Closed
nixar opened this issue Oct 15, 2015 · 6 comments
Closed
Labels
child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations.

Comments

@nixar
Copy link

nixar commented Oct 15, 2015

Doc says:

callback Function called with the output when process terminates

  • error Error
  • stdout Buffer
  • stderr Buffer

This was known back in 2013 already

This points out the options parameter's encoding field can force a buffer response. However the default value is supposed to be 'utf8' according to the doc, which strongly implies that it means character encoding, not the type of the return value. The name of this option is terribly misleading.

Demo code:

"use strict";
var child_process = require('child_process');
console.log('node version=%s',process.version);

for (let options of [{}, {encoding:'buffer'}]) {
  child_process.execFile(
    'uname', ['-o'], options,
    (error, stdout, stderr) => {
      console.log(
        'options is %s → stdout is a %s', 
        JSON.stringify(options), 
        typeof stdout
      )
    }
  );
}

Output:

node version=v4.2.1
options is {"encoding":"buffer"} → stdout is a object
options is {} → stdout is a string

The behavior or the documentation need to be changed. If the current behavior is kept, the semantic of 'encoding' should be clarified, and the name of the property should be changed (to 'return_type' for example).

@ChALkeR ChALkeR added the child_process Issues and PRs related to the child_process subsystem. label Oct 15, 2015
@cjihrig
Copy link
Contributor

cjihrig commented Oct 15, 2015

If you think there is a problem with the documentation, feel free to submit a PR improving it :-)

@nixar
Copy link
Author

nixar commented Oct 15, 2015

I considered it but I'm trying to understand the logic first. How do you explain the baffling yet ostensibly deliberate semantic of 'encoding'?

@cjihrig
Copy link
Contributor

cjihrig commented Oct 16, 2015

encoding specifies how the stderr and stdout data is interpreted. The default is utf8, which is why stdout is a string in your example. When you explicitly set it to a buffer, you get a Buffer object. Does that help?

@nixar
Copy link
Author

nixar commented Oct 16, 2015

No, that does not, because I long figured it out, since that's precisely
what my code is meant to show. I'm trying to understand the logic of
calling something "encoding" when you mean return value type, and "utf8" to
mean string.

Le ven. 16 oct. 2015 02:01, Colin Ihrig notifications@github.com a écrit :

encoding specifies how the stderr and stdout data is interpreted. The
default is utf8, which is why stdout is a string in your example. When
you explicitly set it to a buffer, you get a Buffer object. Does that
help?


Reply to this email directly or view it on GitHub
#3389 (comment).

@cjihrig
Copy link
Contributor

cjihrig commented Oct 16, 2015

utf8 does not mean string. It's one of several types of string encoding supported by Node.

@Fishrock123 Fishrock123 added the doc Issues and PRs related to the documentations. label Oct 16, 2015
@Fishrock123
Copy link
Contributor

It should probably say like Buffer | String (in which ever format we use throughout the docs).

nixar pushed a commit to nixar/DefinitelyTyped that referenced this issue Oct 16, 2015
nixar added a commit to nixar/DefinitelyTyped that referenced this issue Oct 16, 2015
jasnell added a commit to jasnell/node that referenced this issue Apr 3, 2016
Clarify that the arguments to child_process.execFile
and child_process.exec callback can be Buffer or strings.

Fixes: nodejs#3389
@jasnell jasnell closed this as completed in 6fd26dc Apr 3, 2016
MylesBorins pushed a commit that referenced this issue Apr 5, 2016
Clarify that the arguments to child_process.execFile
and child_process.exec callback can be Buffer or strings.

Fixes: #3389
PR-URL: #6015
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Apr 11, 2016
Clarify that the arguments to child_process.execFile
and child_process.exec callback can be Buffer or strings.

Fixes: #3389
PR-URL: #6015
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
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

No branches or pull requests

4 participants