diff --git a/src/Http/Senders/GuzzleSender.php b/src/Http/Senders/GuzzleSender.php index b3c6870a..8662d90a 100644 --- a/src/Http/Senders/GuzzleSender.php +++ b/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; } /** diff --git a/tests/Feature/Guzzle8SenderExceptionTest.php b/tests/Feature/Guzzle8SenderExceptionTest.php new file mode 100644 index 00000000..e58a2ec2 --- /dev/null +++ b/tests/Feature/Guzzle8SenderExceptionTest.php @@ -0,0 +1,120 @@ +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); +});