keypress events not working in windows cmd.exe & powershell

[egoroof]

• Version: 5.7.0
• Platform: windows 10 x64
• Subsystem: streams?

I make commits using cz-cli (it uses arrow keys for selection). Just updated node to 5.7.0 (from 5.6.0) on windows 10 and now arrow keys (upd also Ctrl+C and maybe something other) not working in command line. Pressing arrow keys does nothing (like keypress event doesn't happen).

Guess it related #2996.

[Fishrock123]

@egoroof What terminal are you using?

[egoroof]

CMD
upd: PowerShell the same results

[rvagg]

possibly related report from nodeschool nodeschool/discussions#1641

someone needs to bisect this one, @nodejs/platform-windows can anyone repro?

[egoroof]

someone needs to bisect this one

Can I help somehow? Maybe I should run some tests to get more info about the bug?

[rvagg]

@egoroof if you can set yourself up with a development environment to compile and test Node on your Windows machine then you could help track down exactly where the bug was introduced. git bisect is a great tool to learn if you haven't before, you need to come up with a good test, manual or automatic, to tell you if a version of Node is broken or not then find a spot in the git history where it's not broken then use git bisect to identify the actual commit where the breakage started and we can go from there.

Unless anyone in here has a better idea of where we can narrow this down to.

[eljefedelrodeodeljefe]

@egoroof if you could reproduce without external dependencies like commitizen that would be great. In the referenced issue, there are already examples. I can reproduce some. Trying to get gdb running on windows and hunt this bug down, too.

[egoroof]

Here is the test:

// pressing any key should get the proccess exit
// without the fix you will have to press enter to do it
// it can show you false positives sometimes
// run the test many times to get high probability that it is true passing
process.stdin.setRawMode(true);
process.stdin.on('data', function (char) {
    console.log('Exiting on any key press...');
    process.exit();
});

Here is the results (which I tested with cmd.exe):

works:

• 5.5.0
• 5.4.1
• 5.3.0
• 5.2.0
• 5.1.1
• 5.1.0
• 4.3.1

doesn't work:

• 5.7.0
• 5.6.0
• 5.0.0

It seems it was originally fixed in 5.1.0 (af46112). Then it returned back in 5.6.0.

[rvagg]

if it is to do with streams then it may be in one of the commits that have touched the streams code since v5.5.0:

7684b0f stream: fix no data on partial decode
f706cb0 streams: 5% throughput gain when sending small chunks
ee8d4bb stream: prevent object map change in TransformState
c8b6de2 stream: refactor redeclared variables

Only the last two in that list were in between v5.5.0 and v5.6.0. You could try backing them each out and see if it makes a difference.

Also, /cc @nodejs/streams

[calvinmetcalf]

the second to last one is the only one that actually changes anything so I somewhat doubt the problem lies there

[egoroof]

Ok. I find this bug is complicated. The test is manual and it can show you false positives sometimes. So I run the test many times to get high probability that it is true passing.
Moreover I find that this test have more chances to be passed if you machine uses lots of resouces. I just run PhpStorm IDE and during the starting of the IDE I run test many times and in most cases the test was passed. Then when the IDE is loaded the test doesn't pass in most cases.

Bisect result is: f1a0827 deps: sync with upstream c-ares/c-ares@2bae2d5.

To be sure that this is the cause:
git checkout nodejs/v5.x
git revert -n f0bd176d6d5523626efb3005cf41f11663bde280
git revert -n b46f3b84d4a35c9bf75a30dcfd68d8a3317713db
git revert -n f1a082741743f51112bbf14d4d300f30e99d54d3
vcbuild nosign

And do testing. Seems to work (I didn't find failures with the test).
See #5090 about these commits.

Can someone create automated test for it? I have no idea how.

[rvagg]

Thanks for investigating @egoroof, what we first need to do is figure out how on earth c-ares could be implicated on processing stdin on Windows, and if that is indeed the cause of the bug, how do we fix it?

/cc @indutny @saghul (since you merged and reviewed that patch ... just in case you might have a clue here)