[rush] Fix issue where package.json script hangs when accessing STDIN on Windows

[octogonz]

Summary

The problem I encountered:

1. clone [api-extractor] Fix an issue where destructured parameters were handled incorrectly #5559 on Windows
2. rush install && rush test from cmd.exe

The process hangs while building api-documenter-scenarios, which uses a Heft "run-script-plugin" to invoke node.exe api-documenter/lib/start.js from runScenarios.ts. The actually hang is somewhere in API Documenter, but if you simply add console.log(process.stdin.isTTY) to the top of start.js it will hang forever. Presumably something goes wrong during lazy initialization of the process.stdin property.

Details

Probably this is ultimately another deep Node.js bug on Windows, since Windows TTY/streams has unusual semantics and the Node maintainers don't put as much effort into fixing those issues. I found several GitHub issues about stdin hangs that were opened and later closed without any code change.

A simple way to avoid the hang is to change this code:

rushstack/build-tests/api-documenter-scenarios/src/runScenarios.ts

Lines 46 to 59 in be05af7

	const childProcess: ChildProcess = Executable.spawn(
	process.argv0,
	[
	'node_modules/@microsoft/api-documenter/lib/start',
	'generate',
	`--input-folder`,
	`temp/etc/${scenarioFolderName}`,
	'--output-folder',
	`temp/etc/${scenarioFolderName}/markdown`
	],
	{
	stdio: 'inherit'
	}
	);

...to use stdio: ['ignore', 'inherit', 'inherit'].

But I did not make that fix, because it seems fundamentally unreasonable. What's wrong with inheriting STDIN? And why does it only repro when invoked by Rush?

After a bunch of debugging, I realized Rush is doing something wrong:

When spawning a child process, in the case where it binds STDOUT/STDERR to pipe to redirect the output to the stream collator, Rush also seems to bind STDIN to pipe. BUT Rush never actually attaches any handlers to the STDIN stream, nor does it close it properly. This doesn't justify the specific stdin.isTTY hang, but it does open the possibility of a hang if the child process tries to read STDIN and expects it to get closed later, since it's not a TTY.

Looking around in Utilities.ts, there seem to be other copy+pastes of this same mistake, however I didn't have time to analyze them deeply. If someone else wants to suggest other fixes, that would be very welcome.

How it was tested

Hang repros without the fix. Build does not hang with this fix.

[dmichon-msft]

We should ABSOLUTELY not be inheriting stdin, that would imply that anything the user types would get unexpectedly forwarded to some arbitrary set of child processes.

[dmichon-msft]

In fact, in general, the else case that inherits stdin is also not a sensible default; the caller should have to expressly specify if they want stdin, otherwise we should default to ignore.