diff --git a/src/Illuminate/Http/Client/PendingRequest.php b/src/Illuminate/Http/Client/PendingRequest.php index b784e35bc9c4..f987f0aed653 100644 --- a/src/Illuminate/Http/Client/PendingRequest.php +++ b/src/Illuminate/Http/Client/PendingRequest.php @@ -707,13 +707,23 @@ public function send(string $method, string $url, array $options = []) return $this->makePromise($method, $url, $options); } - return retry($this->tries ?? 1, function () use ($method, $url, $options) { + $shouldRetry = true; + + return retry($this->tries ?? 1, function ($attempt) use ($method, $url, $options, &$shouldRetry) { try { - return tap(new Response($this->sendRequest($method, $url, $options)), function ($response) { + return tap(new Response($this->sendRequest($method, $url, $options)), function ($response) use ($attempt, &$shouldRetry) { $this->populateResponse($response); - if ($this->tries > 1 && $this->retryThrow && ! $response->successful()) { - $response->throw(); + if (! $response->successful()) { + $shouldRetry = $this->retryWhenCallback ? call_user_func($this->retryWhenCallback, $response->toException()) : true; + + if ($attempt < $this->tries && $shouldRetry) { + $response->throw(); + } + + if ($this->tries > 1 && $this->retryThrow) { + $response->throw(); + } } $this->dispatchResponseReceivedEvent($response); @@ -723,7 +733,9 @@ public function send(string $method, string $url, array $options = []) throw new ConnectionException($e->getMessage(), 0, $e); } - }, $this->retryDelay ?? 100, $this->retryWhenCallback); + }, $this->retryDelay ?? 100, function () use (&$shouldRetry) { + return $shouldRetry; + }); } /** diff --git a/tests/Http/HttpClientTest.php b/tests/Http/HttpClientTest.php index 2509466a9c91..5a528d4445de 100644 --- a/tests/Http/HttpClientTest.php +++ b/tests/Http/HttpClientTest.php @@ -1184,15 +1184,53 @@ public function testRequestIsMacroable() public function testRequestExceptionIsThrownWhenRetriesExhausted() { - $this->expectException(RequestException::class); - $this->factory->fake([ '*' => $this->factory->response(['error'], 403), ]); - $this->factory - ->retry(2, 1000, null, true) - ->get('http://foo.com/get'); + $exception = null; + + try { + $this->factory + ->retry(2, 1000, null, true) + ->get('http://foo.com/get'); + } catch (RequestException $e) { + $exception = $e; + } + + $this->assertNotNull($exception); + $this->assertInstanceOf(RequestException::class, $exception); + + $this->factory->assertSentCount(2); + } + + public function testRequestExceptionIsThrownWithoutRetriesIfRetryNotNecessary() + { + $this->factory->fake([ + '*' => $this->factory->response(['error'], 500), + ]); + + $exception = null; + $whenAttempts = 0; + + try { + $this->factory + ->retry(2, 1000, function ($exception) use (&$whenAttempts) { + $whenAttempts++; + + return $exception->response->status() === 403; + }, true) + ->get('http://foo.com/get'); + } catch (RequestException $e) { + $exception = $e; + } + + $this->assertNotNull($exception); + $this->assertInstanceOf(RequestException::class, $exception); + + $this->assertSame(1, $whenAttempts); + + $this->factory->assertSentCount(1); } public function testRequestExceptionIsNotThrownWhenDisabledAndRetriesExhausted() @@ -1206,6 +1244,31 @@ public function testRequestExceptionIsNotThrownWhenDisabledAndRetriesExhausted() ->get('http://foo.com/get'); $this->assertTrue($response->failed()); + + $this->factory->assertSentCount(2); + } + + public function testRequestExceptionIsNotThrownWithoutRetriesIfRetryNotNecessary() + { + $this->factory->fake([ + '*' => $this->factory->response(['error'], 500), + ]); + + $whenAttempts = 0; + + $response = $this->factory + ->retry(2, 1000, function ($exception) use (&$whenAttempts) { + $whenAttempts++; + + return $exception->response->status() === 403; + }, false) + ->get('http://foo.com/get'); + + $this->assertTrue($response->failed()); + + $this->assertSame(1, $whenAttempts); + + $this->factory->assertSentCount(1); } public function testMiddlewareRunsWhenFaked()