all() function fails when a specified event doesn't pass any data

[seregazhuk]

Hi!
For all() function the docs say:

The promise will resolve with an array of whatever all events emitted or null if the events do not pass any data.

Here is a simple example. I create a writable stream and then try to collect data from all drain events. These events don't provide any data. As a result the promise should resolve with null:

$writable = new \React\Stream\WritableResourceStream(fopen('php://stdout', 'w'), $loop, 1);
$writable->write('Hello world');
React\Promise\Stream\all($writable, 'drain')->then(function() {
    echo 'drained' . PHP_EOL;
});

But I receive

Notice: Undefined variable: data in vendor/react/promise-stream/src/functions.php on line 129

Looks like in source code for all() function we should add default null value for variable $data?

$bufferer = function ($data = null) use (&$buffer) {
        $buffer []= $data;
    };

[clue]

Good catch and thanks for reporting! I agree that this looks like a small bug.

Do you feel like filing a PR to address this?

[seregazhuk]

Yes, sure! 😎

[seregazhuk]

@clue I'm not sure if it is a bug. I've forgotten to call close() on the stream. So, with this code everything woks fine:

$loop = \React\EventLoop\Factory::create();

$writable = new \React\Stream\WritableResourceStream(fopen('php://stdout', 'w'), $loop, 1);
$writable->write('Hello world');

React\Promise\Stream\all($writable, 'drain')->then(function() {
    echo 'drained' . PHP_EOL;
});
$writable->close();

$loop->run();

Or is it better to provide this fix, so the function doesn't fail even if the stream wasn't closed?

[seregazhuk]

Close, because it wasn't actually a bug.

[clue]

This actually is a bug and I'm currently looking into this :-)