[4.x] Fix Guzzle 8 transfer exceptions in GuzzleSender

src/Http/Senders/GuzzleSender.php

@@ -20,6 +20,7 @@
 use GuzzleHttp\Exception\ConnectException;
 use GuzzleHttp\Exception\RequestException;
 use GuzzleHttp\Exception\TransferException;
+use GuzzleHttp\Exception\ResponseTransferException;
 use Saloon\Exceptions\Request\FatalRequestException;
 use Saloon\Http\Senders\Factories\GuzzleMultipartBodyFactory;
 
@@ -114,21 +115,14 @@ public function send(PendingRequest $pendingRequest): Response
 
             return $this->createResponse($guzzleResponse, $pendingRequest, $request);
         } catch (ConnectException $exception) {
-            // ConnectException means a network exception has happened, like Guzzle
-            // not being able to connect to the host.
-
             throw new FatalRequestException($exception, $pendingRequest);
         } catch (RequestException $exception) {
-            // Sometimes, Guzzle will throw a RequestException without a response. This
-            // means that it was fatal, so we should still throw a fatal request exception.
-
-            $guzzleResponse = method_exists($exception, 'getResponse') ? $exception->getResponse() : null;
-
-            if (is_null($guzzleResponse)) {
-                throw new FatalRequestException($exception, $pendingRequest);
-            }
+            return $this->responseFromRequestException($exception, $pendingRequest, $request);
+        } catch (TransferException $exception) {
+            // Guzzle 8: timeouts and network errors before a response (NetworkException, etc.)
+            // are no longer ConnectException. Matches processPromise() behaviour.
 
-            return $this->createResponse($guzzleResponse, $pendingRequest, $request, $exception);
+            throw new FatalRequestException($exception, $pendingRequest);
         }
     }
 
@@ -167,26 +161,59 @@ function (TransferException $guzzleException) use ($pendingRequest, $psrRequest)
                         throw new FatalRequestException($guzzleException, $pendingRequest);
                     }
 
-                    // Sometimes, Guzzle will throw a RequestException without a response. This
-                    // means that it was fatal, so we should still throw a fatal request exception.
+                    $response = $this->responseFromRequestException($guzzleException, $pendingRequest, $psrRequest);
 
-                    $guzzleResponse = method_exists($guzzleException, 'getResponse') ? $guzzleException->getResponse() : null;
+                    return ($exception = $response->toException()) ? throw $exception : $response;
+                }
+            );
+    }
 
-                    if (is_null($guzzleResponse)) {
-                        throw new FatalRequestException($guzzleException, $pendingRequest);
-                    }
+    /**
+     * Build a Saloon response from a Guzzle RequestException, or throw when the transfer was fatal.
+     *
+     * @throws FatalRequestException
+     */
+    protected function responseFromRequestException(
+        RequestException $exception,
+        PendingRequest $pendingRequest,
+        RequestInterface $psrRequest,
+    ): Response {
+        if ($this->isIncompleteResponseTransfer($exception)) {
+            throw new FatalRequestException($exception, $pendingRequest);
+        }
 
-                    // Otherwise we'll create a response to convert into an exception.
-                    // This will run the exception through the exception handlers
-                    // which allows the user to handle their own exceptions.
+        $guzzleResponse = $this->guzzleResponseFromRequestException($exception);
 
-                    $response = $this->createResponse($guzzleResponse, $pendingRequest, $psrRequest, $guzzleException);
+        if ($guzzleResponse === null) {
+            throw new FatalRequestException($exception, $pendingRequest);
+        }
 
-                    // Throw the exception our way
+        return $this->createResponse($guzzleResponse, $pendingRequest, $psrRequest, $exception);
+    }
 
-                    return ($exception = $response->toException()) ? throw $exception : $response;
-                }
-            );
+    /**
+     * Guzzle 7: getResponse() is on RequestException. Guzzle 8: only on ResponseException subclasses.
+     */
+    protected function guzzleResponseFromRequestException(RequestException $exception): ?ResponseInterface
+    {
+        if (class_exists('GuzzleHttp\Exception\ResponseException') && $exception instanceof \GuzzleHttp\Exception\ResponseException) {
+            return $exception->getResponse();
+        }
+
+        if (! method_exists($exception, 'getResponse')) {
+            return null;
+        }
+
+        return $exception->getResponse();
+    }
+
+    /**
+     * Guzzle 8 may return a partial response when the body transfer fails (ResponseTransferException).
+     */
+    protected function isIncompleteResponseTransfer(RequestException $exception): bool
+    {
+        return class_exists(ResponseTransferException::class)
+            && $exception instanceof ResponseTransferException;
     }
 
     /**

tests/Feature/Guzzle8SenderExceptionTest.php

@@ -0,0 +1,120 @@
+<?php
+
+declare(strict_types=1);
+
+use Saloon\Enums\Method;
+use Saloon\Http\Request;
+use Saloon\Http\Connector;
+use GuzzleHttp\Handler\MockHandler;
+use GuzzleHttp\Exception\ConnectException;
+use GuzzleHttp\Exception\NetworkException;
+use GuzzleHttp\Psr7\Request as PsrRequest;
+use GuzzleHttp\Psr7\Response as PsrResponse;
+use GuzzleHttp\Exception\NetworkTimeoutException;
+use GuzzleHttp\Exception\ResponseTimeoutException;
+use Saloon\Exceptions\Request\FatalRequestException;
+
+beforeEach(function () {
+    if (! class_exists(NetworkException::class)) {
+        $this->markTestSkipped('Requires Guzzle 8 (NetworkException).');
+    }
+});
+
+test('guzzle 8 transfer failures are converted to fatal request exceptions', function (callable $makeException) {
+    $psrRequest = new PsrRequest('GET', 'https://example.test/resource');
+
+    $connector = new class extends Connector {
+        public function resolveBaseUrl(): string
+        {
+            return 'https://example.test';
+        }
+    };
+
+    $request = new class extends Request {
+        protected Method $method = Method::GET;
+
+        public function resolveEndpoint(): string
+        {
+            return '/resource';
+        }
+    };
+
+    $mock = new MockHandler([$makeException($psrRequest)]);
+    $connector->sender()->getHandlerStack()->setHandler($mock);
+
+    $connector->send($request);
+})->throws(FatalRequestException::class)->with([
+    'ConnectException' => fn (PsrRequest $psrRequest) => new ConnectException('connect failed', $psrRequest),
+    'NetworkTimeoutException' => fn (PsrRequest $psrRequest) => new NetworkTimeoutException('timed out before headers', $psrRequest),
+    'NetworkException' => fn (PsrRequest $psrRequest) => new NetworkException('empty reply', $psrRequest),
+    'ResponseTimeoutException' => fn (PsrRequest $psrRequest) => new ResponseTimeoutException(
+        'timed out after headers',
+        $psrRequest,
+        new PsrResponse(200, [], '{"partial":'),
+    ),
+]);
+
+test('guzzle 8 transfer failures are converted to fatal request exceptions when using sendAsync', function () {
+    $psrRequest = new PsrRequest('GET', 'https://example.test/resource');
+
+    $connector = new class extends Connector {
+        public function resolveBaseUrl(): string
+        {
+            return 'https://example.test';
+        }
+    };
+
+    $request = new class extends Request {
+        protected Method $method = Method::GET;
+
+        public function resolveEndpoint(): string
+        {
+            return '/resource';
+        }
+    };
+
+    $mock = new MockHandler([
+        new ResponseTimeoutException(
+            'timed out after headers',
+            $psrRequest,
+            new PsrResponse(200, [], '{"partial":'),
+        ),
+    ]);
+    $connector->sender()->getHandlerStack()->setHandler($mock);
+
+    $connector->sendAsync($request)->wait();
+})->throws(FatalRequestException::class);
+
+test('guzzle 8 network failures participate in connector retries when using send', function () {
+    $psrRequest = new PsrRequest('GET', 'https://example.test/resource');
+    $makeException = fn () => new NetworkException('empty reply', $psrRequest);
+
+    $connector = new class extends Connector {
+        public ?int $tries = 3;
+
+        public function resolveBaseUrl(): string
+        {
+            return 'https://example.test';
+        }
+    };
+
+    $request = new class extends Request {
+        protected Method $method = Method::GET;
+
+        public function resolveEndpoint(): string
+        {
+            return '/resource';
+        }
+    };
+
+    $mock = new MockHandler([$makeException(), $makeException(), $makeException()]);
+    $connector->sender()->getHandlerStack()->setHandler($mock);
+
+    try {
+        $connector->send($request);
+    } catch (FatalRequestException) {
+        //
+    }
+
+    expect($mock)->toHaveCount(0);
+});