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

When executable is spawned from node if there is no stdin getStdin() never resolves #13

Open
lukechilds opened this issue Jun 5, 2016 · 13 comments

Comments

@lukechilds
Copy link
Sponsor Contributor

lukechilds commented Jun 5, 2016

If there is no file option set my CLI app falls back to getStdin().

I check if the result of the file read or getStdin() is falsey and if so I show the help message.

This works fine when used for real in a shell but when I try to write a test for it, it just hangs. It seems if I spawn my CLI app from node, get-stdin() returns a promise that never resolves. For example:

$ node dist/cli.js

  Usage: htconvert [options]

  Options:

    -h, --help              output usage information
    -V, --version           output the version number
    -f, --file [.htaccess]  File containing .htaccess redirects

But if I run this from node:

var exec = require('child_process').exec;
exec('node dist/cli.js').stdout.pipe(process.stdout);

I get nothing.

Not sure if possibly related to #1. Is this expected behaviour or a bug?

@lukechilds lukechilds changed the title When executable is spawned from node if there is no stdin get-stdin() never resolves When executable is spawned from node if there is no stdin getStdin() never resolves Jun 6, 2016
@sindresorhus
Copy link
Owner

That's expected. Since you've executed getStdin(), stdin is waiting to get written to. You're not doing that in your example.

@lukechilds
Copy link
Sponsor Contributor Author

But I'm not writing to stdin in the first example from the shell and it doesn't hang. It's only when it's spawned from node via exec that it hangs.

Is there a way I can simulate the first example from exec so I can test for help output in AVA?

@sindresorhus
Copy link
Owner

That's because in the first example the process is interactive, in the second, it's spawned, so it's not. get-stream has a shortcut to resolve when it's interactive. When get-stdin subscribe to stdin, it blocks the process until something is written to stdin. I don't really know all the details, but I think this is how the stdin works on a system level, not just Node.js. @Qix- can probably elaborate on all this.

Is there a way I can simulate the first example from exec so I can test for help output in AVA?

Write to stdin, or make the process interactive. The only way to make it interactive as far as I know is to childProcess.spawn('foo', {stdio: 'inherit'});.

@lukechilds
Copy link
Sponsor Contributor Author

The thing is if I write to stdin it'll work with that data and won't show the help output 😢

I'll look in to making the process interactive, thanks for the explanation 👍

@Qix-
Copy link

Qix- commented Jun 6, 2016

Without knowing exactly how get-stdin works, my first suspicion is the isatty() function returning false - which is the usual case.

isatty(int) returns 1 or 0 (true or false, respectively) when you pass it a file descriptor. Without getting too much into details, essentially if the file descriptor is interactive then isatty() returns 1. That means if stdin is interactive, isatty(0) will return 1 (true). Same thing with piping stdout - if you're piping the output of process A to process B, isatty(1) in process A will return 0 (false).

I'm assuming that's what's going on here: spawning a new process in Node has a high potential to munge the I/O descriptors of the child process (either turning them off or piping to them instead of forwarding them or marking them as interactive). If you're not inheriting I'm pretty sure they are marked as non-interactive, thus isatty(0) will return 0.

Since we can only "best guess" as to what's going on, we assume there is data going to be piped in.

Hopefully that helps.

@sindresorhus
Copy link
Owner

@Qix- get-stdin works by just listening to stdin, gathering all the data, and resolving when getting the end event. The main logic is barely 10 LOC:

get-stdin/index.js

Lines 15 to 25 in e89bea4

stdin.on('readable', function () {
var chunk;
while ((chunk = stdin.read())) {
ret += chunk;
}
});
stdin.on('end', function () {
resolve(ret);
});

@sindresorhus
Copy link
Owner

@Qix- My question is. Why does stdin need to block? It's very annoying that you can't both have an stdin listener and write to stdout, without anything coming into stdin.

@Qix-
Copy link

Qix- commented Jun 6, 2016

It's very annoying that you can't both have an stdin listener and write to stdout

You can; could you explain exactly what you mean? Writing to stdout while reading stdin is perfectly possible - at least, on the unix side of things. Not sure if listening in Node blocks stdout, but that definitely shouldn't be the case.

@lukechilds
Copy link
Sponsor Contributor Author

Just tried and it works fine: lukechilds@545a9fc want a PR?

AVA tests pass but the npm test command fails because of an xo rule, nothing to do with what I've changed.

sindresorhus added a commit that referenced this issue Dec 2, 2016
For this module, we only really care about stdin that is written right away, like when the process is piped.

This has the benefit of letting us easily add a fallback when there's no stdin.

Fixes #13
@ulion
Copy link

ulion commented Jan 23, 2017

encountered same problem when using https://github.com/silverwind/oui

which use get-stdin but hang there if the stdin is not closed by parent process.

@silverwind
Copy link

It seems that all async spawn methods need to explicitely close stdin. This should work for OP:

var exec = require('child_process').exec;
var child = exec('node dist/cli.js');
child.stdout.pipe(process.stdout);
child.stdin.end();

Ongoing discussion in: nodejs/node#2339 (comment)

@Qix-
Copy link

Qix- commented Feb 12, 2017

Re @ulion: yep, that's right; there's no way for a process to know when stdin is finished until the pipe itself is closed.

@lukechilds
Copy link
Sponsor Contributor Author

@silverwind Thanks so much, have a gold star! 🌟

All working now: lukechilds/htconvert@a8f0476

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants