await() doesn't work with streaming HTTP response body

[bileslav]

Hello,

The code below doesn't work as expected.
At the end, strlen() should return a non-zero.

Am I missing something maybe?

<?php
require __DIR__ . '/vendor/autoload.php';

React\Async\async(function (): void {
	$browser = new React\Http\Browser();
	$response = $browser->requestStreaming(
		'GET',
		'https://file-examples-com.github.io/uploads/2017/04/file_example_MP4_1920_18MG.mp4',
		[
			'Range' => 'bytes=0-499',
		],
	);

	/** @var Psr\Http\Message\ResponseInterface */
	$response = React\Async\await($response);

	/** @var React\Stream\ReadableStreamInterface */
	$body = $response->getBody();

	$body = React\Promise\Stream\buffer($body);
	$body = React\Async\await($body);

	echo strlen($body), "\n"; // 0
})();

[clue]

@bileslaw Thank you for reporting, this is an interesting one!

I could indeed reproduce the problem you're seeing and have managed to solve it like this:

$browser = new React\Http\Browser();
$promise = $browser->requestStreaming(
    'GET',
    'https://file-examples-com.github.io/uploads/2017/04/file_example_MP4_1920_18MG.mp4',
    [
        'Range' => 'bytes=0-499',
    ],
)->then(function (Psr\Http\Message\ResponseInterface $response) {
    return React\Promise\Stream\buffer($response->getBody());
});

$body = React\Async\await($promise);

echo strlen($body), "\n"; // 0

This example should work just fine. That said, I consider your example to be reasonable and I would have expected this to work as well. I'll keep this ticket open for the reference and will look into addressing this in the underlying await() implementation.

The underlying problem is that await() only resumes in the "next tick" while the HTTP response headers and the body return in the same "same tick". This means once you start awaiting the body buffer, the data has already been emitted (into the void) and the body is already closed.

I've already patched this locally, but will do more tests first. Expect a PR in the next week or so 👍

[bileslav]

@clue Great, thank you!

[WyriHaximus]

@bileslaw Thank you! For reporting this 👍 . Really neat edge case you found there 👍

[clue]

@bileslaw Again thanks for reporting! I've just filed #32 which should address this and make this work as expected, both within an async() call (the easy part) and outside of async() (the hard part). You're looking at 8+ hours of work for #32 alone, plus work in #30 and related tickets. This has definitely been an interesting challenge and I'm glad we've been able to improve this somewhat, enjoy!

[clue]

@bileslaw Thanks for giving this another try and confirming this works as expected now that your other problem has been addressed! 👍