Silence stream_isatty and posix_isatty

[ostrolucky]

Q	A
Branch?	4.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets
License	MIT
Doc PR

After #36696, I've got report that users get XX bytes of buffered data lost during stream conversion errors when validation of their first question fails. This can be seen at https://travis-ci.org/github/symfony/maker-bundle/jobs/693200010#L332

Simple reproducer:

(new Application())
    ->register('app')
    ->setCode(function(InputInterface $input, OutputInterface $output) {
        (new QuestionHelper())->ask($input, $output, (new Question('Foo?'))->setValidator(function () {
            throw new InvalidArgumentException('Foo!');
        }));
    })
    ->getApplication()
    ->setDefaultCommand('app', true)
    ->run()
;

Runnning as:
echo "foo\n" | php console-app.php

These calls were actually silenced before already (and still are in 3.4), but we removed this later, thinking it's not needed anymore.

I would actually prefer to fix this differently if possible, but I would have to be able to reproduce issue mentioned in https://github.com/symfony/symfony/pull/36696/files#r420063328 first. I was unable to reproduce that issue with code similar as in https://github.com/symfony/symfony/blob/8ddaa20b29caa7aeb505681f12e86a7a444ae8b4/src/Symfony/Component/Console/Tests/phpt/uses_stdin_as_interactive_input.phpt, which should match scenario that referenced comment describes

cc @weaverryan @nicolas-grekas

[nicolas-grekas]

Coincidentally, we discussed this with @weaverryan yesterday.

The warning is legit: data lost during stream conversion means we're ignoring part of the user input.

The best fix I can think of is to do this instead:

-$attempts = $question->getMaxAttempts();
+$attempts = $question->getMaxAttempts() ?? 100;

[ostrolucky]

Wouldn't that reopen problem 2 that was fixed in #36590? In stdin mode, only one attempt to display question should happen.

[nicolas-grekas]

We have little options here, having to choose between losing user input and asking more that one time...
The issue is calling isatty on stdin to decide to go with interactive mode or not. Can't we check somewhere on stdout instead?

[ostrolucky]

As mentioned in PR description, I can't help further if I don't understand issue that was fixed in #36696. Because after reverting that patch I cannot reproduce the issue patch is meant to fix.

Can't we check somewhere on stdout instead?

No, stdout is same in both cases of bin/console and echo foo | bin/console.

[nicolas-grekas]

The patch in #36696 fixes exactly the same situation as reported here, but when answers are correct: when the response is correct, we don't care about tty or not. That's what the patch does: dong the tty check only when the answer fails.

Now, we have the same problem but only when a wrong answer is given.

[ostrolucky]

Yes you did say that in https://github.com/symfony/symfony/pull/36696/files#r420063328 and I said in this PR description that according that description code like such

symfony/src/Symfony/Component/Console/Tests/phpt/uses_stdin_as_interactive_input.phpt

Lines 18 to 27 in 8ddaa20

	(new Application())
	->register('app')
	->setCode(function(InputInterface $input, OutputInterface $output) {
	$output->writeln((new QuestionHelper())->ask($input, $output, new Question('Foo?', 'foo')));
	$output->writeln((new QuestionHelper())->ask($input, $output, new Question('Bar?', 'bar')));
	})
	->getApplication()
	->setDefaultCommand('app', true)
	->run()
	;

should reproduce it, but doesn't do that for me.

Can you reproduce it in attached project? Composer.json is setup in a way that it doesn't contain #36696 but does contain #36590. I am running it with with echo "sup\nyo\n" | bin/console app:list-users. What am I missing if I cannot reproduce it?

symfony-demo.zip

[nicolas-grekas]

I just discovered that this is enough to fix the warning:
$inputStream = null === $this->inputStream || (\defined('STDIN') && STDIN === $this->inputStream) ? fopen('php://input', 'r') : $this->inputStream;

[weaverryan]

Unfortunately, this trades one error for another. This DOES fix the "bytes of buffered data lost during stream conversion" error. But now, as soon as an answer fails validation, instead of asking again, it ALWAYS fails immediately - it falls out of the loop and hits the throw at the end

symfony/src/Symfony/Component/Console/Helper/QuestionHelper.php

Line 479 in 5a74790

throw $error;

Here is a SUPER simple reproduced - https://github.com/weaverryan/console-question-reproducer - I tried the current code on that and your proposed patch. But this PR doesn't fix the issue either - I think this PR sort of fixes it, but then I think we're still hitting #37046.

The only thing that fully works is reverting back to 5.0.8 - you can try all of this pretty quickly with the reproducer.

[nicolas-grekas]

We settled on another way, isn't it?

[ostrolucky]

yep