tty: stdio properties are undefined inside exec() child

[silverwind]

I'm wondering if this is intentional or a bug:

parent.js

var exec = require("child_process").exec;

exec("iojs child.js", function (err, stdout, stderr) {
  process.stdout.write(stdout);
});

child.js

console.log(process.stdin.isTTY);
console.log(process.stdin.isRaw);
console.log(process.stdin.setRawMode);
console.log(process.stdout.columns);

output:

undefined
undefined
undefined
undefined

ref: sindresorhus/grunt-shell#95 raineorshine/npm-check-updates#119

[thefourtheye]

When the new process is created, the process.stdin is actually a PIPE and it doesn't have isTTY, isRaw and setRawMode defined. We can actually fix them like this, I think

diff --git a/src/node.js b/src/node.js
index af66b17..87121bc 100644
--- a/src/node.js
+++ b/src/node.js
@@ -717,6 +717,12 @@
         stdin._handle.readStop();
       });

+      stdin.isTTY = true;
+      stdin.isRaw = false;
+      stdin.setRawMode = function setRawMode(mode) {
+        throw new Error('Not a Raw device');
+      };
+
       return stdin;
     });

process.stdout object is actually a Socket object, so columns and rows will not make sense in this case.

[silverwind]

Doing this unconditionally worries me a bit. For the isTTY case, I wonder why the code path in tty_wrap.cc that sets it isn't triggered.

[thefourtheye]

The isTTY is set in tty module's ReadStream constructor function. When the new process is created, guessHandleType calls uv_guess_handle with fd as 0 and than returns PIPE, instead of TTY. So, a net.Socket is created and returned. If uv_guess_handle returned TTY, tty.ReadStream would have been invoked, and isTTY and isRaw would be set properly.

[silverwind]

Hmm, I don't see the process.stdin getter invoked at all when accessing these properties, so your patch doesn't seem to change anything. I'm guessing we have to check out ttywrap.

[thefourtheye]

@silverwind Actually, I tested the changes and it is working fine. Shall I submit a PR with the code in this issue as a test?

[silverwind]

Go ahead, it's good to have a PR so we can run CI anyways.

[thefourtheye]

@silverwind I am not making changes for the columns and rows, as they will be undefined anyway.

[silverwind]

I'm primarily after stdin.isTTY, but I wasn't seeing any difference, it looked as if startup.processStdio was never invoked.

[brendanashworth]

Mmm #2160?

[silverwind]

@sabrehagen is it possible that your script was being piped to or was being called by exec when you observed that issue?

[silverwind]

Been thinking about this some more. The docs are pretty clear that these ReadStream properties are only defined when an actual TTY is attached:

When io.js detects that it is being run inside a TTY context, then process.stdin will be a tty.ReadStream

I think I'll just leave it at that. A possible change to introduce these properties would be a breaking change if done right, and I don't think it's worth doing so, it doesn't really solve any issues.

[thefourtheye]

But stdin will be inconsistent when we exec, I am not sure if that is okay

[silverwind]

True. I don't understand completely what exec really does yet. When doing process.stdin.pipe(child.stdin) after exec, I don't think isTTY and isRaw do update correctly, but probably should.

[thefourtheye]

I believe the same problem should be with spawn as well. I'll check that today

[silverwind]

I gave up on doing the rework of your PR because there'd be a tiny change of breakage when someone does process.stdin.isTTY === undefined and it's kind of a pointless breaking change if we intialize these values when stdin isn't actually a ReadStream.

The issue I was trying to solve is this:

var child = exec("iojs child.js", function (err, stdout, stderr) {
  process.stdout.write(stdout);
});
process.stdin.pipe(child.stdin);

After this pipe is made, the child's stdin should upgrade to a ReadStream instance (and have these properties defined). Ideally, this should happen before the child executes its first line of code.

[Trott]

@silverwind wrote:

Ideally, this should happen before the child executes its first line of code.

The child is a separate node process, right? Is there realistically even a way to guarantee order like that when there are two separate processes?

[59naga]

FYI
when i encountered this issue, wanted the following requirements:

• execution of a command that can't be run using spawn.
• non-blocking (not sync)
• stdio inheritance

In other words, by executing a shell in the spawn, it has been resolved.

// parent.js
const spawn = require('child_process').spawn;
spawn('NODE_ENV=production node ./child.js', { shell: true, stdio: 'inherit' });

// child.js
const chalk = require('chalk');
process.stdout.write(`${chalk.magenta('child')}\n`);

the above doesn't work with node 5.6 or less

[jasnell]

I'm wondering if there's really anything to do here other than simply expand the documentation to account for this.