reactphp / promise

Promises/A implementation for PHP.
https://reactphp.org/promise/
MIT License
2.38k stars 146 forks source link

\React\Promise\all($promises) doesn't work as expected #129

Closed suhaboncukcu closed 6 years ago

suhaboncukcu commented 6 years ago

It is awesome to have reactphp namespace and this promise repo. Thank you so much for that.

In the documentation, it says that this function will return a promise which will resolved after all the promises in the array have resolved.

Here is my implementation;

    private function downloadListingImages($contents)
    {
        $response = [];
        $name = 1;
        foreach ($contents['images'] as $image) {
            $response[] = $this->downloadImage($image, $name);
            $name++;
        }
        return $response;
    }

    private function downloadImage($link, $name)
    {
        $guzzle = new Client([
            'handler' => HandlerStack::create(new HttpClientAdapter($this->loop)),
        ]);

        $promise = $guzzle->getAsync($link, [
            'save_to' => 'assets/' . $name . '.jpg',
        ]);

        return $promise;
    }

   $promises = $this->downloadListingImages($contents);

Now, everything is fine till this point. But want I want to do is get $contents from a request to my server. So I have a server implementation;

$server = new React\Http\Server(function (Psr\Http\Message\ServerRequestInterface $request) use ($promises) {

    \React\Promise\all($promises)->always(function($val) {
        file_put_contents('meh.txt', "meh");
    });

    return new React\Http\Response(
        200,
        array('Content-Type' => 'text/plain'),
        "Hello World!\n"
    );

});

What I expect here that $server returns an immediate response (which it does) and after a while see the meh.txt in my repo. However, it never falls to always callback. And even when I don't chain any function on all method, it just resolves itself. Shouldn't it wait until then or something similar to be called?

legionth commented 6 years ago

As long as the objects that passed to \React\Promise\all implement the method then, it works very well. E.g. \React\Promise\all works very fine with with \React\Promise\Promise() and also with \GuzzleHttp\Promise\Promise.

So far I'm not able to reproduce this issue.

As you can see in the documentation the always-method doesn't accept any parameter. Consider to use then instead of always if you in need of a parameter. Also consider that file_put_contents MAY blocking the event-loop.

I hope this helps :+1:

suhaboncukcu commented 6 years ago

Thanks @legionth , I've altered my code to use then instead. I've also realized the reason behind my problem. It seems when I use 'save_to' parameter; it somehow resolves the promise instantly. I'll open a bug to the guzzle repo since I am now able to handle the issue.