Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] Handle concurrent asynchronous requests in the HTTP client #36948

Merged
merged 8 commits into from Apr 12, 2021

Conversation

cerbero90
Copy link
Contributor

@cerbero90 cerbero90 commented Apr 12, 2021

This PR aims to bring concurrency while sending asynchronous requests with the Laravel HTTP client.

Laravel Octane is not required to make this feature work, however Octane should be compatible with this feature as the latest version of Swoole supports curl_multi operations.

The PR introduces:

  • asynchronous requests
  • a pool to handle asynchronous requests concurrently

Asynchronous requests can be created with the following syntax:

$promise = Http::async()->get($url);

The result is an instance of GuzzleHttp\Promise\Promise.

Based on whether a promise is fulfilled or rejected, the then() method of the promise receives:

  • An instance of Illuminate\Http\Client\Response if the promise is fulfilled
  • An instance of Illuminate\Http\Client\Response if the promise is rejected but a response was provided (4xx, 5xx errors)
  • An instance of GuzzleHttp\Exception\TransferException if the promise is rejected with no response (e.g. timeout)
$promise = Http::async()->get($url)->then(fn (Response|TransferException $result) => $this->handleResult($result));

The pool collects asynchronous requests and send them concurrently, so that the slowest request determines the maximum waiting time for all promises to be fulfilled.

// the waiting time will be ~3 seconds instead of 15

$responses = Http::pool(fn (Pool $pool) => [
    $pool->get('https://httpbin.org/delay/3')->then(/* ... */),
    $pool->get('https://httpbin.org/delay/3')->then(/* ... */),
    $pool->get('https://httpbin.org/delay/3')->then(/* ... */),
    $pool->get('https://httpbin.org/delay/3')->then(/* ... */),
    $pool->get('https://httpbin.org/delay/3')->then(/* ... */),
]);

$responses[0]->ok();
$responses[1]->successful();
// ...

Asynchronous requests can also be added to the pool with a key:

$responses = Http::pool(fn (Pool $pool) => [
    $pool->add('foo')->get('https://httpbin.org/delay/3')->then(/* ... */),
    $pool->add('bar')->get('https://httpbin.org/delay/3')->then(/* ... */),
    $pool->add('baz')->get('https://httpbin.org/delay/3')->then(/* ... */),
]);

$responses['foo']->ok();
$responses['bar']->successful();
$connectionFailed = $responses['baz'] instanceof GuzzleHttp\Exception\ConnectException;

Responses are instances of Illuminate\Http\Client\Response if requests were responded (2xx, 3xx, 4xx, 5xx). Otherwise if no response was received, the exception that provoked the promise rejection is returned.

All requests added to the pool are asynchronous by default, so there is no need to chain the async() method. It may also be worth mentioning that all the fluent methods of PendingRequest are available to the requests in the pool:

$responses = Http::pool(fn (Pool $pool) => [
    $pool->withOptions(['timeout' => 3])->get('https://foo.test')->then(/* ... */),
    $pool->asForm()->post($url, $data)->then(/* ... */),
]);

The pool leverages Illuminate\Http\Client\Factory under the hood, so we can take advantage of all its testing functionalities in our unit tests:

$this->factory->fake([
    '200.com' => $this->factory::response('', 200),
    '400.com' => $this->factory::response('', 400),
    '500.com' => $this->factory::response('', 500),
]);

$responses = $this->factory->pool(fn (Pool $pool) => [
    $pool->get('200.com'),
    $pool->get('400.com'),
    $pool->get('500.com'),
]);

$this->assertSame(200, $responses[0]->status());
$this->assertSame(400, $responses[1]->status());
$this->assertSame(500, $responses[2]->status());

@taylorotwell
Copy link
Member

Thanks for your work on this - renamed add to as.

@taylorotwell taylorotwell merged commit ce69e55 into laravel:8.x Apr 12, 2021
@cerbero90 cerbero90 deleted the feature/http-pool branch April 12, 2021 21:10
@louisgab
Copy link
Contributor

louisgab commented Apr 27, 2021

Awesome work I long awaited, but I was wondering why the pool doesn't take advantage of Guzzle's pool and others utils?
Is the only purpose of Http::pool to add the async() method on all requests ? IMHO, it should be the developer's responsibility...

In terms of DX, I find it easier not to depend on the Pool object and use the intuitive array's "key=>value" notation instead of the pool as() method.

Here is a working equivalent example of the actual pool() :

use Illuminate\Support\Facades\Http;
use GuzzleHttp\Promise\Utils;

$responses = Utils::all([
    'foo' => Http::async()->get('https://httpbin.org/delay/3')->then(/* ... */),
    'bar' => Http::async()->get('https://httpbin.org/delay/3')->then(/* ... */),
    'baz' => Http::async()->get('https://httpbin.org/delay/3')->then(/* ... */),
])->wait();

$responses['foo']->ok();
$responses['bar']->successful();
$connectionFailed = $responses['baz'] instanceof GuzzleHttp\Exception\ConnectException;

I would happily PR something like Http::all(array $requests) as a wrapper of Utils::all()->wait()

That way we can use "preconfigured" instances like I usually do:

$http = Http::baseUrl('https://httpbin.org/')->acceptJson()->timeout(10)->withToken($token);

$responses = Http::all([
    'foo' => $http->async()->get('/delay/3'),
    'bar' => $http->async()->get('/delay/3'),
    'baz' => $http->async()->get('/delay/3'),
]);

@cerbero90
Copy link
Contributor Author

Thanks for your contribution, @louisgab :)

Originally the Pool class was meant to collect requests while making sure they:

  1. are asynchronous
  2. share the same client instance

However you are right, this looks like a limit to use cases like yours.

Adding another method to accomplish the same end result may represent an overhead, an alternative may be altering the pool() method to accept callables and iterables:

/**
 * Send a pool of asynchronous requests concurrently.
 *
 * @param  iterable|callable  $requests
 * @return array
 */
public function pool($requests)
{
    $results = [];

    $requests = is_iterable($requests) ? $requests : tap(new Pool($this->factory), $requests)->getRequests();

    foreach ($requests as $key => $item) {
        $results[$key] = $item instanceof static ? $item->getPromise()->wait() : $item->wait();
    }

    ksort($results);

    return $results;
}

This way the Pool class is skipped if we provide an iterable (array, iterator, generator, etc.)

What do you think?

@louisgab
Copy link
Contributor

louisgab commented Apr 28, 2021

Good enough for me, but we could go further!

I would prefer the all() naming for this because it reminds me of JavaScript's Promise.all(). Then, the name pool kept for a wrapper of Guzzle's Pool object which uses generators (great candidate for Lazy collection!) and has a configurable pool size (concurrency).

Also, I digged inside Guzzle, and found out that Utils:all() wraps the list of iterables as a promise itself.
(Internally it uses GuzzleHttp/Promise/EachPromise that seems to handle edge cases and ensure concurrency. So it could be more reliable than the current loop you implemented in the pool method...)

And it's awesome since it enables us to do things like that :

$promise = Utils:all([
    // Requests
])
->then(/* ... */)
->otherwise(/* ... */);

if($someCondition){
    $promise->cancel();
}

$responses = $promise-wait();

It means we can write logic like "throw if any of the requests went wrong" instead of doing it manually one by one on each response.

It would be nice to hear more opinions, maybe @driesvints or @GrahamCampbell ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants