[11.x] Support retry and throw on async http client request (in a htt…

…p client request pool) (#48906) * Support throw and retry in async request * Small fixes * Fix linting issues * Fix another linting issue * Throw after last retry * Provide attempt and exception to retryDelay closure * formatting --------- Co-authored-by: Taylor Otwell <[email protected]>
laravel · Nov 8, 2023 · be21757 · be21757
1 parent 890ea60
commit be21757
Show file tree

Hide file tree

Showing 2 changed files with 330 additions and 1 deletion.
diff --git a/src/Illuminate/Http/Client/PendingRequest.php b/src/Illuminate/Http/Client/PendingRequest.php
@@ -997,9 +997,10 @@ protected function parseMultipartBodyFormat(array $data)
      * @param  string  $method
      * @param  string  $url
      * @param  array  $options
+     * @param  int  $attempt
      * @return \GuzzleHttp\Promise\PromiseInterface
      */
-    protected function makePromise(string $method, string $url, array $options = [])
+    protected function makePromise(string $method, string $url, array $options = [], int $attempt = 1)
     {
         return $this->promise = $this->sendRequest($method, $url, $options)
             ->then(function (MessageInterface $message) {
@@ -1011,12 +1012,70 @@ protected function makePromise(string $method, string $url, array $options = [])
             ->otherwise(function (TransferException $e) {
                 if ($e instanceof ConnectException) {
                     $this->dispatchConnectionFailedEvent();
+
+                    return new ConnectionException($e->getMessage(), 0, $e);
                 }
 
                 return $e instanceof RequestException && $e->hasResponse() ? $this->populateResponse($this->newResponse($e->getResponse())) : $e;
+            })
+            ->then(function (Response|ConnectionException|TransferException $response) use ($method, $url, $options, $attempt) {
+                return $this->handlePromiseResponse($response, $method, $url, $options, $attempt);
             });
     }
 
+    /**
+     * Handle the response of an asynchronous request.
+     *
+     * @param  \Illuminate\Http\Client\Response  $response
+     * @param  string  $method
+     * @param  string  $url
+     * @param  array  $options
+     * @param  int  $attempt
+     * @return mixed
+     */
+    protected function handlePromiseResponse(Response|ConnectionException|TransferException $response, $method, $url, $options, $attempt)
+    {
+        if ($response instanceof Response && $response->successful()) {
+            return $response;
+        }
+
+        if ($response instanceof RequestException) {
+            $response = $this->populateResponse($this->newResponse($response->getResponse()));
+        }
+
+        try {
+            $shouldRetry = $this->retryWhenCallback ? call_user_func(
+                $this->retryWhenCallback,
+                $response instanceof Response ? $response->toException() : $response,
+                $this
+            ) : true;
+        } catch (Exception $exception) {
+            return $exception;
+        }
+
+        if ($attempt < $this->tries && $shouldRetry) {
+            $options['delay'] = value($this->retryDelay, $attempt, $response->toException());
+
+            return $this->makePromise($method, $url, $options, $attempt + 1);
+        }
+
+        if ($response instanceof Response &&
+            $this->throwCallback &&
+            ($this->throwIfCallback === null || call_user_func($this->throwIfCallback, $response))) {
+            try {
+                $response->throw($this->throwCallback);
+            } catch (Exception $exception) {
+                return $exception;
+            }
+        }
+
+        if ($this->tries > 1 && $this->retryThrow) {
+            return $response instanceof Response ? $response->toException() : $response;
+        }
+
+        return $response;
+    }
+
     /**
      * Send a request either synchronously or asynchronously.
      *

diff --git a/tests/Http/HttpClientTest.php b/tests/Http/HttpClientTest.php
@@ -1837,6 +1837,132 @@ public function testRequestsWillBeWaitingSleepMillisecondsReceivedBeforeRetry()
         ]);
     }
 
+    public function testRequestExceptionReturnedWhenRetriesExhaustedInPool()
+    {
+        $this->factory->fake([
+            '*' => $this->factory->response(['error'], 403),
+        ]);
+
+        [$exception] = $this->factory->pool(fn ($pool) => [
+            $pool->retry(2, 1000, null, true)->get('http://foo.com/get'),
+        ]);
+
+        $this->assertNotNull($exception);
+        $this->assertInstanceOf(RequestException::class, $exception);
+
+        $this->factory->assertSentCount(2);
+    }
+
+    public function testRequestExceptionIsReturnedWithoutRetriesIfRetryNotNecessaryInPool()
+    {
+        $this->factory->fake([
+            '*' => $this->factory->response(['error'], 500),
+        ]);
+
+        $whenAttempts = collect();
+
+        [$exception] = $this->factory->pool(fn ($pool) => [
+            $pool->retry(2, 1000, function ($exception) use ($whenAttempts) {
+                $whenAttempts->push($exception);
+
+                return $exception->response->status() === 403;
+            }, true)->get('http://foo.com/get'),
+        ]);
+
+        $this->assertNotNull($exception);
+        $this->assertInstanceOf(RequestException::class, $exception);
+
+        $this->assertCount(1, $whenAttempts);
+
+        $this->factory->assertSentCount(1);
+    }
+
+    public function testRequestExceptionIsNotReturnedWhenDisabledAndRetriesExhaustedInPool()
+    {
+        $this->factory->fake([
+            '*' => $this->factory->response(['error'], 403),
+        ]);
+
+        [$response] = $this->factory->pool(fn ($pool) => [
+            $pool->retry(2, 1000, null, false)->get('http://foo.com/get'),
+        ]);
+
+        $this->assertNotNull($response);
+        $this->assertInstanceOf(Response::class, $response);
+        $this->assertTrue($response->failed());
+
+        $this->factory->assertSentCount(2);
+    }
+
+    public function testRequestExceptionIsNotReturnedWithoutRetriesIfRetryNotNecessaryInPool()
+    {
+        $this->factory->fake([
+            '*' => $this->factory->response(['error'], 500),
+        ]);
+
+        $whenAttempts = collect();
+
+        [$response] = $this->factory->pool(fn ($pool) => [
+            $pool->retry(2, 1000, function ($exception) use ($whenAttempts) {
+                $whenAttempts->push($exception);
+
+                return $exception->response->status() === 403;
+            }, false)->get('http://foo.com/get'),
+        ]);
+
+        $this->assertNotNull($response);
+        $this->assertInstanceOf(Response::class, $response);
+        $this->assertTrue($response->failed());
+
+        $this->assertCount(1, $whenAttempts);
+
+        $this->factory->assertSentCount(1);
+    }
+
+    public function testRequestCanBeModifiedInRetryCallbackInPool()
+    {
+        $this->factory->fake([
+            '*' => $this->factory->sequence()
+                ->push(['error'], 500)
+                ->push(['ok'], 200),
+        ]);
+
+        [$response] = $this->factory->pool(fn ($pool) => [
+            $pool->retry(2, 1000, function ($exception, $request) {
+                $this->assertInstanceOf(PendingRequest::class, $request);
+
+                $request->withHeaders(['Foo' => 'Bar']);
+
+                return true;
+            }, false)->get('http://foo.com/get'),
+        ]);
+
+        $this->assertTrue($response->successful());
+
+        $this->factory->assertSent(function (Request $request) {
+            return $request->hasHeader('Foo') && $request->header('Foo') === ['Bar'];
+        });
+    }
+
+    public function testExceptionThrownInRetryCallbackIsReturnedWithoutRetryingInPool()
+    {
+        $this->factory->fake([
+            '*' => $this->factory->response(['error'], 500),
+        ]);
+
+        [$exception] = $this->factory->pool(fn ($pool) => [
+            $pool->retry(2, 1000, function ($exception) {
+                throw new Exception('Foo bar');
+            }, false)->get('http://foo.com/get'),
+        ]);
+
+        $this->assertNotNull($exception);
+        $this->assertInstanceOf(Exception::class, $exception);
+        $this->assertEquals('Foo bar', $exception->getMessage());
+
+        $this->factory->assertSentCount(1);
+    }
+
     public function testMiddlewareRunsWhenFaked()
     {
         $this->factory->fake(function (Request $request) {
@@ -2081,6 +2207,150 @@ public function testRequestExceptionIsNotThrownIfTheRequestDoesNotFail()
         $this->assertSame('{"result":{"foo":"bar"}}', $response->body());
     }
 
+    public function testRequestExceptionIsNotReturnedIfThePendingRequestIsSetToThrowOnFailureButTheResponseIsSuccessfulInPool()
+    {
+        $this->factory->fake([
+            '*' => $this->factory->response(['success'], 200),
+        ]);
+
+        [$response] = $this->factory->pool(fn ($pool) => [
+            $pool->throw()->get('http://foo.com/get'),
+        ]);
+
+        $this->assertInstanceOf(Response::class, $response);
+        $this->assertSame(200, $response->status());
+    }
+
+    public function testRequestExceptionIsReturnedIfThePendingRequestIsSetToThrowOnFailureInPool()
+    {
+        $this->factory->fake([
+            '*' => $this->factory->response(['error'], 403),
+        ]);
+
+        [$exception] = $this->factory->pool(fn ($pool) => [
+            $pool->throw()->get('http://foo.com/get'),
+        ]);
+
+        $this->assertNotNull($exception);
+        $this->assertInstanceOf(RequestException::class, $exception);
+    }
+
+    public function testRequestExceptionIsReturnedIfTheThrowIfOnThePendingRequestIsSetToTrueOnFailureInPool()
+    {
+        $this->factory->fake([
+            '*' => $this->factory->response(['error'], 403),
+        ]);
+
+        [$exception] = $this->factory->pool(fn ($pool) => [
+            $pool->throwIf(true)->get('http://foo.com/get'),
+        ]);
+
+        $this->assertNotNull($exception);
+        $this->assertInstanceOf(RequestException::class, $exception);
+    }
+
+    public function testRequestExceptionIsNotReturnedIfTheThrowIfOnThePendingRequestIsSetToFalseOnFailureInPool()
+    {
+        $this->factory->fake([
+            '*' => $this->factory->response(['error'], 403),
+        ]);
+
+        [$response] = $this->factory->pool(fn ($pool) => [
+            $pool->throwIf(false)->get('http://foo.com/get'),
+        ]);
+
+        $this->assertInstanceOf(Response::class, $response);
+        $this->assertSame(403, $response->status());
+    }
+
+    public function testRequestExceptionIsReturnedIfTheThrowIfClosureOnThePendingRequestReturnsTrueInPool()
+    {
+        $this->factory->fake([
+            '*' => $this->factory->response(['error'], 403),
+        ]);
+
+        $hitThrowCallback = collect();
+
+        [$exception] = $this->factory->pool(fn ($pool) => [
+            $pool->throwIf(function ($response) {
+                $this->assertInstanceOf(Response::class, $response);
+                $this->assertSame(403, $response->status());
+
+                return true;
+            }, function ($response, $e) use (&$hitThrowCallback) {
+                $this->assertInstanceOf(Response::class, $response);
+                $this->assertSame(403, $response->status());
+
+                $this->assertInstanceOf(RequestException::class, $e);
+
+                $hitThrowCallback->push(true);
+            })->get('http://foo.com/get'),
+        ]);
+
+        $this->assertNotNull($exception);
+        $this->assertInstanceOf(RequestException::class, $exception);
+        $this->assertCount(1, $hitThrowCallback);
+    }
+
+    public function testRequestExceptionIsNotReturnedIfTheThrowIfClosureOnThePendingRequestReturnsFalseInPool()
+    {
+        $this->factory->fake([
+            '*' => $this->factory->response(['error'], 403),
+        ]);
+
+        $hitThrowCallback = collect();
+
+        [$response] = $this->factory->pool(fn ($pool) => [
+            $pool->throwIf(function ($response) {
+                $this->assertInstanceOf(Response::class, $response);
+                $this->assertSame(403, $response->status());
+
+                return false;
+            }, function ($response, $e) use (&$hitThrowCallback) {
+                $hitThrowCallback->push(true);
+            })->get('http://foo.com/get'),
+        ]);
+
+        $this->assertCount(0, $hitThrowCallback);
+        $this->assertSame(403, $response->status());
+    }
+
+    public function testRequestExceptionIsReturnedWithCallbackIfThePendingRequestIsSetToThrowOnFailureInPool()
+    {
+        $this->factory->fake([
+            '*' => $this->factory->response(['error'], 403),
+        ]);
+
+        $flag = collect();
+
+        [$exception] = $this->factory->pool(fn ($pool) => [
+            $pool->throw(function ($exception) use (&$flag) {
+                $flag->push(true);
+            })->get('http://foo.com/get'),
+        ]);
+
+        $this->assertCount(1, $flag);
+
+        $this->assertNotNull($exception);
+        $this->assertInstanceOf(RequestException::class, $exception);
+    }
+
+    public function testRequestExceptionIsReturnedAfterLastRetryInPool()
+    {
+        $this->factory->fake([
+            '*' => $this->factory->response(['error'], 403),
+        ]);
+
+        [$exception] = $this->factory->pool(fn ($pool) => [
+            $pool->retry(3)->throw()->get('http://foo.com/get'),
+        ]);
+
+        $this->assertNotNull($exception);
+        $this->assertInstanceOf(RequestException::class, $exception);
+
+        $this->factory->assertSentCount(3);
+    }
+
     public function testRequestExceptionIsThrowIfConditionIsSatisfied()
     {
         $this->factory->fake([