From 00cc0631a5f41b2998ba2f66e3700b6bdde77fa1 Mon Sep 17 00:00:00 2001 From: Paul Young Date: Wed, 3 Jul 2024 14:04:02 +1000 Subject: [PATCH 1/4] Update HttpClientTest.php --- tests/Http/HttpClientTest.php | 139 ++++++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/tests/Http/HttpClientTest.php b/tests/Http/HttpClientTest.php index 9904e9165c01..2df9284a189d 100644 --- a/tests/Http/HttpClientTest.php +++ b/tests/Http/HttpClientTest.php @@ -1711,6 +1711,28 @@ public function testRequestExceptionIsThrownWhenRetriesExhausted() $this->factory->assertSentCount(2); } + public function testRequestExceptionIsThrownWhenRetriesExhaustedWithBackoffArray() + { + $this->factory->fake([ + '*' => $this->factory->response(['error'], 403), + ]); + + $exception = null; + + try { + $this->factory + ->retry([1], 0, 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([ @@ -1740,6 +1762,35 @@ public function testRequestExceptionIsThrownWithoutRetriesIfRetryNotNecessary() $this->factory->assertSentCount(1); } + public function testRequestExceptionIsThrownWithoutRetriesIfRetryNotNecessaryWithBackoffArray() + { + $this->factory->fake([ + '*' => $this->factory->response(['error'], 500), + ]); + + $exception = null; + $whenAttempts = 0; + + try { + $this->factory + ->retry([1000,1000], 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() { $this->factory->fake([ @@ -1755,6 +1806,21 @@ public function testRequestExceptionIsNotThrownWhenDisabledAndRetriesExhausted() $this->factory->assertSentCount(2); } + public function testRequestExceptionIsNotThrownWhenDisabledAndRetriesExhaustedWithBackoffArray() + { + $this->factory->fake([ + '*' => $this->factory->response(['error'], 403), + ]); + + $response = $this->factory + ->retry([1,2], throw: false) + ->get('http://foo.com/get'); + + $this->assertTrue($response->failed()); + + $this->factory->assertSentCount(3); + } + public function testRequestExceptionIsNotThrownWithoutRetriesIfRetryNotNecessary() { $this->factory->fake([ @@ -1778,6 +1844,29 @@ public function testRequestExceptionIsNotThrownWithoutRetriesIfRetryNotNecessary $this->factory->assertSentCount(1); } + public function testRequestExceptionIsNotThrownWithoutRetriesIfRetryNotNecessaryWithBackoffArray() + { + $this->factory->fake([ + '*' => $this->factory->response(['error'], 500), + ]); + + $whenAttempts = 0; + + $response = $this->factory + ->retry([1,2], 0, 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 testRequestCanBeModifiedInRetryCallback() { $this->factory->fake([ @@ -1803,6 +1892,31 @@ public function testRequestCanBeModifiedInRetryCallback() }); } + public function testRequestCanBeModifiedInRetryCallbackWithBackoffArray() + { + $this->factory->fake([ + '*' => $this->factory->sequence() + ->push(['error'], 500) + ->push(['ok'], 200), + ]); + + $response = $this->factory + ->retry([2], when: function ($exception, $request) { + $this->assertInstanceOf(PendingRequest::class, $request); + + $request->withHeaders(['Foo' => 'Bar']); + + return true; + }, throw: 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 testExceptionThrownInRetryCallbackWithoutRetrying() { $this->factory->fake([ @@ -1828,6 +1942,31 @@ public function testExceptionThrownInRetryCallbackWithoutRetrying() $this->factory->assertSentCount(1); } + public function testExceptionThrownInRetryCallbackWithoutRetryingWithBackoffArray() + { + $this->factory->fake([ + '*' => $this->factory->response(['error'], 500), + ]); + + $exception = null; + + try { + $this->factory + ->retry([1,2,3], when: function ($exception) use (&$whenAttempts) { + throw new Exception('Foo bar'); + }, throw: false) + ->get('http://foo.com/get'); + } catch (Exception $e) { + $exception = $e; + } + + $this->assertNotNull($exception); + $this->assertInstanceOf(Exception::class, $exception); + $this->assertEquals('Foo bar', $exception->getMessage()); + + $this->factory->assertSentCount(1); + } + public function testRequestsWillBeWaitingSleepMillisecondsReceivedBeforeRetry() { Sleep::fake(); From ee29d901654abab41b717cadfedad5c282edaa6f Mon Sep 17 00:00:00 2001 From: Paul Young Date: Wed, 3 Jul 2024 14:12:45 +1000 Subject: [PATCH 2/4] Fix PendingRequest.php to deal with tries as an array --- src/Illuminate/Http/Client/PendingRequest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Http/Client/PendingRequest.php b/src/Illuminate/Http/Client/PendingRequest.php index 5dbb9dad8da2..0d619fd17da4 100644 --- a/src/Illuminate/Http/Client/PendingRequest.php +++ b/src/Illuminate/Http/Client/PendingRequest.php @@ -911,11 +911,13 @@ public function send(string $method, string $url, array $options = []) $response->throw($this->throwCallback); } - if ($attempt < $this->tries && $shouldRetry) { + $tryCount = is_array($this->tries) ? count($this->tries) + 1 : $this->tries; + + if ($attempt < $tryCount && $shouldRetry) { $response->throw(); } - if ($this->tries > 1 && $this->retryThrow) { + if ($tryCount > 1 && $this->retryThrow) { $response->throw(); } } From 6bec42cd2e6580811f69562e8a3827fda76744c8 Mon Sep 17 00:00:00 2001 From: Paul Young Date: Wed, 3 Jul 2024 14:49:37 +1000 Subject: [PATCH 3/4] Style CI spaces --- tests/Http/HttpClientTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Http/HttpClientTest.php b/tests/Http/HttpClientTest.php index 2df9284a189d..ea204c8c3f02 100644 --- a/tests/Http/HttpClientTest.php +++ b/tests/Http/HttpClientTest.php @@ -1773,7 +1773,7 @@ public function testRequestExceptionIsThrownWithoutRetriesIfRetryNotNecessaryWit try { $this->factory - ->retry([1000,1000], 1000, function ($exception) use (&$whenAttempts) { + ->retry([1000, 1000], 1000, function ($exception) use (&$whenAttempts) { $whenAttempts++; return $exception->response->status() === 403; @@ -1813,7 +1813,7 @@ public function testRequestExceptionIsNotThrownWhenDisabledAndRetriesExhaustedWi ]); $response = $this->factory - ->retry([1,2], throw: false) + ->retry([1, 2], throw: false) ->get('http://foo.com/get'); $this->assertTrue($response->failed()); @@ -1853,7 +1853,7 @@ public function testRequestExceptionIsNotThrownWithoutRetriesIfRetryNotNecessary $whenAttempts = 0; $response = $this->factory - ->retry([1,2], 0, function ($exception) use (&$whenAttempts) { + ->retry([1, 2], 0, function ($exception) use (&$whenAttempts) { $whenAttempts++; return $exception->response->status() === 403; @@ -1952,7 +1952,7 @@ public function testExceptionThrownInRetryCallbackWithoutRetryingWithBackoffArra try { $this->factory - ->retry([1,2,3], when: function ($exception) use (&$whenAttempts) { + ->retry([1, 2, 3], when: function ($exception) use (&$whenAttempts) { throw new Exception('Foo bar'); }, throw: false) ->get('http://foo.com/get'); From 28d53e22e5118b24033ee246721bf6e1309a0600 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Wed, 3 Jul 2024 16:09:48 -0500 Subject: [PATCH 4/4] formatting --- src/Illuminate/Http/Client/PendingRequest.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Http/Client/PendingRequest.php b/src/Illuminate/Http/Client/PendingRequest.php index 0d619fd17da4..7eb8b0e47b6c 100644 --- a/src/Illuminate/Http/Client/PendingRequest.php +++ b/src/Illuminate/Http/Client/PendingRequest.php @@ -911,13 +911,15 @@ public function send(string $method, string $url, array $options = []) $response->throw($this->throwCallback); } - $tryCount = is_array($this->tries) ? count($this->tries) + 1 : $this->tries; + $potentialTries = is_array($this->tries) + ? count($this->tries) + 1 + : $this->tries; - if ($attempt < $tryCount && $shouldRetry) { + if ($attempt < $potentialTries && $shouldRetry) { $response->throw(); } - if ($tryCount > 1 && $this->retryThrow) { + if ($potentialTries > 1 && $this->retryThrow) { $response->throw(); } }