saloonphp / saloon

🤠 Build beautiful API integrations and SDKs with Saloon
https://docs.saloon.dev
MIT License
2.09k stars 107 forks source link

Fix | V3 Concurrency Issues #308

Closed Sammyjo20 closed 1 year ago

Sammyjo20 commented 1 year ago

I think I cracked it! 🤠🚀 This fixes a very tricky issue to get around raised in #306

So it turns out that the way I wrote the promise wrapping before, meant that the Pool was only sending the outer promise in the concurrency specified, and the inner promise would just be invoked synchronously. After spending the last couple of nights learning properly how Guzzle's promise library works, and looking through hundreds of lines of code - I managed to stumble across our golden method. Utils::task().

As described in the docs this method:

Adds a function to run in the task queue when it is next run() and * returns a promise that is fulfilled or rejected with the result.

Looking at the code:

public static function task(callable $task): PromiseInterface
{
    $queue = self::queue();
    $promise = new Promise([$queue, 'run']);
    $queue->add(function () use ($task, $promise): void {
        try {
            if (Is::pending($promise)) {
                $promise->resolve($task());
            }
        } catch (\Throwable $e) {
            $promise->reject($e);
        }
    });

    return $promise;
}

It's exactly what we need!

HUGE thanks to @aidan-casey for spending his Friday night looking at this issue - and helping me get to the bottom of it.

Sammyjo20 commented 1 year ago

I also wanted to prove that it's still wrapping the inner promise as I expect, so I added three dumps - one inside a Guzzle middleware, one inside of Saloon's request middleware and one inside Saloon's response middleware and the output is like this:

Note This is sending 100 requests with a concurrency of 10

Previous Output (Before Fix)

"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"response" // tests/Feature/ExampleTest.php:41
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"response" // tests/Feature/ExampleTest.php:41
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"response" // tests/Feature/ExampleTest.php:41
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"response" // tests/Feature/ExampleTest.php:41
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"response" // tests/Feature/ExampleTest.php:41
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"response" // tests/Feature/ExampleTest.php:41
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"response" // tests/Feature/ExampleTest.php:41
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"response" // tests/Feature/ExampleTest.php:41
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30

Output With Fix

"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"response" // tests/Feature/ExampleTest.php:41
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"response" // tests/Feature/ExampleTest.php:41
"response" // tests/Feature/ExampleTest.php:41
"response" // tests/Feature/ExampleTest.php:41
"response" // tests/Feature/ExampleTest.php:41
"response" // tests/Feature/ExampleTest.php:41
"response" // tests/Feature/ExampleTest.php:41
"response" // tests/Feature/ExampleTest.php:41
"response" // tests/Feature/ExampleTest.php:41
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"response" // tests/Feature/ExampleTest.php:41
"response" // tests/Feature/ExampleTest.php:41
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"Send!" // tests/Feature/ExampleTest.php:37
"Send Guzzle!" // tests/Feature/ExampleTest.php:30
"response" // tests/Feature/ExampleTest.php:41
"response" // tests/Feature/ExampleTest.php:41
"response" // tests/Feature/ExampleTest.php:41
"response" // tests/Feature/ExampleTest.php:41
"response" // tests/Feature/ExampleTest.php:41
"response" // tests/Feature/ExampleTest.php:41
"response" // tests/Feature/ExampleTest.php:41
"response" // tests/Feature/ExampleTest.php:41