From 41125540cd7bd72aa7721009f728cac2d4752657 Mon Sep 17 00:00:00 2001 From: Bob Soukup Date: Sun, 25 Oct 2026 15:34:33 +0100 Subject: [PATCH 1/2] test: pin retry delays and which network failures are retried Guzzle 8 removes RetryMiddleware::exponentialDelay() and reclassifies network failures: a timeout before the response headers is NetworkTimeoutException, no longer a ConnectException, and send or receive errors become NetworkException instead of RequestException. Pin today's behaviour on Guzzle 7 first, so the Guzzle 8 support can show it keeps it: - a timeout without a response is retried; - a receive error after the request was sent is not, because the provider may already have run (and billed) it; - without Retry-After, the delays are 1 s, 2 s, 4 s. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/Http/HttpClientFactoryTest.php | 81 ++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/tests/Http/HttpClientFactoryTest.php b/tests/Http/HttpClientFactoryTest.php index c41556f..5194df3 100644 --- a/tests/Http/HttpClientFactoryTest.php +++ b/tests/Http/HttpClientFactoryTest.php @@ -7,12 +7,16 @@ use GuzzleHttp\Client; use GuzzleHttp\Exception\ClientException; use GuzzleHttp\Exception\ConnectException; +use GuzzleHttp\Exception\NetworkException; +use GuzzleHttp\Exception\RequestException; +use GuzzleHttp\Exception\NetworkTimeoutException; use GuzzleHttp\Exception\ServerException; use GuzzleHttp\Handler\MockHandler; use GuzzleHttp\HandlerStack; use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\RequestInterface; use Soukicz\Llm\Cache\CacheInterface; use Soukicz\Llm\Http\HttpClientFactory; use Soukicz\Llm\Tests\Cache\InMemoryCache; @@ -131,6 +135,83 @@ public function testRetriesConnectExceptions(): void { $this->assertSame('ok', (string) $response->getBody()); } + /** + * A timeout before any response arrives must be retried like a refused connection. + * Guzzle 7 reports it as ConnectException, Guzzle 8 as NetworkTimeoutException + * (a NetworkException that is not a ConnectException) + */ + public function testRetriesNetworkTimeoutsWithoutResponse(): void { + $client = $this->createClient(); + $this->recordDelays($client, $delays); + $request = new Request('GET', 'https://example.com/api'); + $timeout = class_exists(NetworkTimeoutException::class) + ? new NetworkTimeoutException('cURL error 28: Operation timed out', $request) + : new ConnectException('cURL error 28: Operation timed out', $request); + $this->mockHandler->append($timeout, new Response(200, [], 'ok')); + + $response = $client->get('https://example.com/api'); + + $this->assertSame('ok', (string) $response->getBody()); + $this->assertSame(0, $this->mockHandler->count()); + } + + /** + * A receive error after the request was sent is not retried: the provider may already have run + * (and billed) it. Guzzle 7 reports it as RequestException, Guzzle 8 as NetworkException + */ + public function testDoesNotRetryReceiveErrorsAfterTheRequestWasSent(): void { + $client = $this->createClient(); + $this->recordDelays($client, $delays); + $request = new Request('POST', 'https://example.com/api'); + $recvError = class_exists(NetworkException::class) + ? new NetworkException('cURL error 56: Recv failure: Connection reset by peer', $request) + : new RequestException('cURL error 56: Recv failure: Connection reset by peer', $request); + $this->mockHandler->append($recvError, new Response(200)); + + try { + $client->post('https://example.com/api'); + $this->fail('Expected the receive error'); + } catch (RequestException|NetworkException $e) { + $this->assertSame($recvError, $e); + } + $this->assertSame(1, $this->mockHandler->count(), 'The request must not be sent again'); + } + + public function testRetryDelayDoublesWithoutRetryAfterHeader(): void { + $client = $this->createClient(); + $this->recordDelays($client, $delays); + $this->mockHandler->append( + new Response(500), + new Response(502), + new Response(503), + new Response(200), + ); + + $client->get('https://example.com/api'); + + $this->assertSame([null, 1000, 2000, 4000], $delays); + } + + /** + * Records the delay the retry middleware asks for on each attempt and drops it, so the + * MockHandler does not sleep. Pushed last, so it runs between the retry middleware and the transport + * + * @param list|null $delays + */ + private function recordDelays(Client $client, ?array &$delays): void { + $delays = []; + /** @var HandlerStack $stack */ + $stack = $client->getConfig('handler'); + $stack->push(static function (callable $handler) use (&$delays): callable { + return static function (RequestInterface $request, array $options) use ($handler, &$delays) { + $delays[] = $options['delay'] ?? null; + unset($options['delay']); + + return $handler($request, $options); + }; + }); + } + public function testSuccessfulResponsesAreCachedAndReplayed(): void { $cache = new InMemoryCache(); $client = $this->createClient($cache); From 7bf3cc15dfc573062c546a907caf7682ba17f8a5 Mon Sep 17 00:00:00 2001 From: Bob Soukup Date: Sun, 25 Oct 2026 15:34:33 +0100 Subject: [PATCH 2/2] feat: support Guzzle 8 alongside Guzzle 7 Applications on Guzzle 8 cannot install this library: it pins guzzlehttp/guzzle ^7.9, promises ^2.0 and psr7 ^2.7. Allowing the new majors alone is not enough, because two things break on Guzzle 8: - RetryMiddleware::exponentialDelay() is gone (deprecated since 7.11), so every retry without a Retry-After header died with "Call to undefined method". The delay is now computed here, with the same formula (1 s, 2 s, 4 s). - A timeout before the response headers is NetworkTimeoutException, no longer a ConnectException, so it was not retried any more. The decider now also retries that class. It keeps everything else as on Guzzle 7: Guzzle 8's other NetworkExceptions (send and receive errors) are not retried, because the request may already have been processed. The only narrowing is the empty reply (cURL 52), which Guzzle 7 reported as ConnectException and Guzzle 8 does not. Also: - PHPStan's generics.notGeneric ignore only matches with Promises 2, so it no longer reports when it is unmatched. - A CI job runs the tests and PHPStan on the newest Guzzle 7, now that a plain install picks Guzzle 8. The prefer-lowest job keeps covering the oldest Guzzle 7. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/php-tests.yml | 24 ++++++++++++++++++++++++ composer.json | 6 +++--- phpstan.neon | 2 ++ src/Http/HttpClientFactory.php | 13 ++++++++++--- 4 files changed, 39 insertions(+), 6 deletions(-) diff --git a/.github/workflows/php-tests.yml b/.github/workflows/php-tests.yml index 781a83a..9ca5415 100644 --- a/.github/workflows/php-tests.yml +++ b/.github/workflows/php-tests.yml @@ -63,6 +63,30 @@ jobs: - name: Run tests run: XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-text --testsuite default + test-guzzle-7: + name: Tests and PHPStan with Guzzle 7 (PHP 8.3) + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v3 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: '8.3' + extensions: mbstring, xml, json, zlib + coverage: none + + - name: Install dependencies (newest Guzzle 7) + run: composer update --prefer-dist --no-progress --with-all-dependencies --with "guzzlehttp/guzzle:^7.9" + + - name: Run tests + run: vendor/bin/phpunit --testsuite default + + - name: Run PHPStan + run: vendor/bin/phpstan analyse src + test-lowest-dependencies: name: Test with lowest dependencies (PHP 8.3) runs-on: ubuntu-latest diff --git a/composer.json b/composer.json index 8ed6d3e..ab0b240 100644 --- a/composer.json +++ b/composer.json @@ -23,9 +23,9 @@ "require": { "php": ">=8.3", "psr/http-message": "^1.1 || ^2.0", - "guzzlehttp/guzzle": "^7.9", - "guzzlehttp/promises": "^2.0", - "guzzlehttp/psr7": "^2.7", + "guzzlehttp/guzzle": "^7.9 || ^8.0", + "guzzlehttp/promises": "^2.0 || ^3.0", + "guzzlehttp/psr7": "^2.7 || ^3.0", "ext-zlib": "*", "swaggest/json-schema": "^0.12.43", "ramsey/uuid": "^4.8" diff --git a/phpstan.neon b/phpstan.neon index f5a8a58..7f8659b 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -4,4 +4,6 @@ parameters: - src ignoreErrors: - + # Guzzle Promises 2 (Guzzle 7) has no generic PromiseInterface; with Promises 3 (Guzzle 8) it does identifier: generics.notGeneric + reportUnmatched: false diff --git a/src/Http/HttpClientFactory.php b/src/Http/HttpClientFactory.php index 3b3737b..0ddf690 100644 --- a/src/Http/HttpClientFactory.php +++ b/src/Http/HttpClientFactory.php @@ -8,13 +8,14 @@ use GuzzleHttp\Middleware; use GuzzleHttp\Promise\Create; use GuzzleHttp\Promise\PromiseInterface; -use GuzzleHttp\RetryMiddleware; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; use Soukicz\Llm\Cache\CacheInterface; class HttpClientFactory { private const MAX_RETRIES = 3; + /** Guzzle 8 only; a string so the class may be missing on Guzzle 7 */ + private const NETWORK_TIMEOUT_EXCEPTION = 'GuzzleHttp\\Exception\\NetworkTimeoutException'; public static function createClient(?callable $customMiddleware = null, ?CacheInterface $cache = null, array $headers = []): Client { $options = [ @@ -71,7 +72,12 @@ private static function addRetryMiddleware(HandlerStack $handler): void { if ($retries >= self::MAX_RETRIES) { return false; } - if ($exception instanceof ConnectException) { + // Retry only failures without a response that cannot have been processed or that Guzzle 7 + // already retried: a failed connection (DNS, TCP, TLS - nothing was sent) and a timeout before + // the response. Guzzle 8 reports that timeout as NetworkTimeoutException, which is not a + // ConnectException. Send/receive errors after the request went out stay unretried, as on Guzzle 7: + // the provider may already have run (and billed) the request + if ($exception instanceof ConnectException || is_a($exception, self::NETWORK_TIMEOUT_EXCEPTION)) { return true; } @@ -82,7 +88,8 @@ private static function addRetryMiddleware(HandlerStack $handler): void { $delay = static function (int $retries, ?ResponseInterface $response = null): int { if ($response === null || !$response->hasHeader('Retry-After')) { - return RetryMiddleware::exponentialDelay($retries); + // 1 s, 2 s, 4 s; RetryMiddleware::exponentialDelay() is deprecated in Guzzle 7.11 and removed in 8 + return (int) 2 ** ($retries - 1) * 1000; } $retryAfter = $response->getHeaderLine('Retry-After');