[9.x] Fix Http Client throw boolean parameter of retry method (#41762)

* Add failing test

* Fix retryThrow behavior

* Fix code when no retry is necessary

Author: gdebrauwer

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;
+        });
     }
 
     /**

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()