From 4846d1d5fbd78e8c57271fe975695cedb981114c Mon Sep 17 00:00:00 2001 From: Venca Krecl Date: Mon, 27 Jul 2026 13:27:46 +0200 Subject: [PATCH 1/4] Add Guzzle 8 support while keeping Guzzle 7 compatible Allows guzzlehttp/guzzle ^8.0 and guzzlehttp/psr7 ^3.0, and fixes the API removals and behaviour changes that Guzzle 8 introduces. Fixes #621. Generated clients (codegen/, applied mechanically across 147 files): - \GuzzleHttp\Utils::jsonEncode() was removed, so request bodies are built with json_encode(..., JSON_THROW_ON_ERROR) instead. - RequestException::getResponse() was removed; in Guzzle 8 only ResponseException subclasses carry a response. Both the sync catch block and the async rejection handler now check for the method, so a failure without a response produces an ApiException with null headers and body instead of a fatal error. The async handler previously blew up on any responseless failure under Guzzle 7 as well. - Guzzle 8 reports no-response network failures as NetworkException rather than ConnectException, so the generated clients catch Psr\Http\Client\NetworkExceptionInterface, which both versions implement. Hand written code (lib/): - RetryMiddlewareFactory::getRetryFunctionByConnectionErrors() matches NetworkExceptionInterface and reads the cURL errno from the exception message, because Guzzle 8 removed RequestException::getHandlerContext() and reclassified cURL errors 52, 55 and 56 as NetworkException. Without this, connection error retries become a silent no-op on Guzzle 8. Errors 55 and 56 are now retried on Guzzle 7 too, which is what TRANSIENT_CURL_ERROR_CODES documented. - The retry deciders accept any PSR-7 RequestInterface/ResponseInterface instead of only the concrete GuzzleHttp\Psr7 classes. - apiRequest() uppercases the method option. Guzzle 7 uppercased request methods, Guzzle 8 sends them verbatim. Tests and CI: - New tests/Unit/GeneratedApiClientTest.php drives a generated client over a MockHandler and covers JSON body encoding plus error handling with and without a response, on the sync and async paths. - RetryMiddlewareFactoryTest no longer relies on the removed handler context constructor argument. - phpunit and phpspec run against Guzzle 7 and Guzzle 8. Note that codegen/ is generated outside this repository and upstream openapi-generator still emits the removed APIs, so the generator templates need the same changes to keep these fixes on regeneration. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/phpspec.yml | 13 +- .github/workflows/phpunit.yml | 13 +- CHANGELOG.md | 10 ++ README.md | 2 + composer.json | 4 +- lib/Delay.php | 2 +- lib/Http/Request.php | 3 +- lib/RetryMiddlewareFactory.php | 36 +++--- tests/Unit/GeneratedApiClientTest.php | 150 ++++++++++++++++++++++ tests/Unit/Http/RequestTest.php | 11 ++ tests/Unit/RetryMiddlewareFactoryTest.php | 47 +++++-- 11 files changed, 255 insertions(+), 36 deletions(-) create mode 100644 tests/Unit/GeneratedApiClientTest.php diff --git a/.github/workflows/phpspec.yml b/.github/workflows/phpspec.yml index ba0fb1aeb..479f7d309 100644 --- a/.github/workflows/phpspec.yml +++ b/.github/workflows/phpspec.yml @@ -6,11 +6,22 @@ jobs: build: runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + guzzle: ['^7.3', '^8.0'] + + name: "phpspec (guzzle ${{ matrix.guzzle }})" + + env: + GUZZLE_CONSTRAINT: ${{ matrix.guzzle }} + steps: - uses: actions/checkout@v2 - name: Setup PHP uses: shivammathur/setup-php@v2 with: php-version: '8.2' - - run: composer install + - run: composer require "guzzlehttp/guzzle:$GUZZLE_CONSTRAINT" --no-update --no-interaction + - run: composer update --no-interaction --no-progress - run: vendor/bin/phpspec run diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index c1dac1f7d..471390f43 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -6,11 +6,22 @@ jobs: build: runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + guzzle: ['^7.3', '^8.0'] + + name: "phpunit (guzzle ${{ matrix.guzzle }})" + + env: + GUZZLE_CONSTRAINT: ${{ matrix.guzzle }} + steps: - uses: actions/checkout@v2 - name: Setup PHP uses: shivammathur/setup-php@v2 with: php-version: '8.2' - - run: composer install + - run: composer require "guzzlehttp/guzzle:$GUZZLE_CONSTRAINT" --no-update --no-interaction + - run: composer update --no-interaction --no-progress - run: vendor/bin/phpunit ./tests diff --git a/CHANGELOG.md b/CHANGELOG.md index bcd6dc2a1..1ba377939 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/HubSpot/hubspot-api-php/compare/14.1.0...HEAD) +### Guzzle 8 support + +- `guzzlehttp/guzzle` `^8.0` and `guzzlehttp/psr7` `^3.0` are now allowed; Guzzle 7 remains supported. +- Generated clients no longer call the removed `\GuzzleHttp\Utils::jsonEncode()`; they use `json_encode(..., JSON_THROW_ON_ERROR)`. +- Generated clients no longer assume `RequestException::getResponse()` exists (removed in Guzzle 8, where only `ResponseException` subclasses carry a response). Failures without a response now produce an `ApiException` with `null` headers/body instead of a fatal error, on both the sync and async paths. +- Generated clients catch `Psr\Http\Client\NetworkExceptionInterface` instead of `GuzzleHttp\Exception\ConnectException`, so Guzzle 8's `NetworkException` (send/receive errors, HTTP/2 and HTTP/3 failures) is still converted to an `ApiException`. +- `RetryMiddlewareFactory::getRetryFunctionByConnectionErrors()` now matches `Psr\Http\Client\NetworkExceptionInterface` and reads the cURL errno from the exception message, because Guzzle 8 removed `RequestException::getHandlerContext()` and reclassified cURL errors 52, 55 and 56 as `NetworkException` rather than `ConnectException`. As a side effect, cURL errors 55 and 56 are now retried on Guzzle 7 as well, which is what `TRANSIENT_CURL_ERROR_CODES` always documented. +- `apiRequest()` uppercases the `method` option. Guzzle 7 uppercased request methods, Guzzle 8 sends them verbatim. +- The retry decider callbacks accept any PSR-7 `RequestInterface`/`ResponseInterface` instead of only `GuzzleHttp\Psr7\Request`/`Response`. + ## [14.1.0](https://github.com/HubSpot/hubspot-api-php/releases/tag/14.1.0) - 2026-05-12 ### Retry Middleware diff --git a/README.md b/README.md index ec4b34783..339629d36 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,8 @@ The current package requirements are: PHP >= 8.1 +Guzzle 7 and Guzzle 8 are both supported (`guzzlehttp/guzzle: ^7.3 || ^8.0`). + ### Sample apps Please, take a look at our [Sample apps](https://github.com/HubSpot/sample-apps-list) diff --git a/composer.json b/composer.json index 757b50d1c..c421ca739 100644 --- a/composer.json +++ b/composer.json @@ -22,8 +22,8 @@ "ext-curl": "*", "ext-json": "*", "ext-mbstring": "*", - "guzzlehttp/guzzle": "^7.3", - "guzzlehttp/psr7": "^1.7 || ^2.0" + "guzzlehttp/guzzle": "^7.3 || ^8.0", + "guzzlehttp/psr7": "^1.7 || ^2.0 || ^3.0" }, "require-dev": { "friendsofphp/php-cs-fixer": "^3.94", diff --git a/lib/Delay.php b/lib/Delay.php index 25e1a2ed5..3ebae62e2 100644 --- a/lib/Delay.php +++ b/lib/Delay.php @@ -19,7 +19,7 @@ public static function getLinearDelayFunction() } /** - * @deprecated pass null as the delay function instead — Guzzle will apply its built-in exponential delay via \GuzzleHttp\RetryMiddleware::exponentialDelay + * @deprecated pass null as the delay function instead - \GuzzleHttp\RetryMiddleware then applies its built-in exponential delay */ public static function getExponentialDelayFunction(int $base) { diff --git a/lib/Http/Request.php b/lib/Http/Request.php index 3323d9e2c..fb8c7c59b 100644 --- a/lib/Http/Request.php +++ b/lib/Http/Request.php @@ -39,7 +39,8 @@ public function __construct(Config $config, array $options = []) if (array_key_exists('defaultJson', $this->options)) { $this->defaultJson = $this->options['defaultJson']; } - $this->method = $this->options['method'] ?? 'GET'; + // Guzzle 7 uppercased request methods, Guzzle 8 sends them verbatim. + $this->method = strtoupper($this->options['method'] ?? 'GET'); $this->initHeaders(); $this->applyAuth(); diff --git a/lib/RetryMiddlewareFactory.php b/lib/RetryMiddlewareFactory.php index 45ccc0e43..53047871e 100644 --- a/lib/RetryMiddlewareFactory.php +++ b/lib/RetryMiddlewareFactory.php @@ -2,10 +2,10 @@ namespace HubSpot; -use GuzzleHttp\Exception\ConnectException; use GuzzleHttp\Middleware; -use GuzzleHttp\Psr7\Request; -use GuzzleHttp\Psr7\Response; +use Psr\Http\Client\NetworkExceptionInterface; +use Psr\Http\Message\RequestInterface; +use Psr\Http\Message\ResponseInterface; class RetryMiddlewareFactory { @@ -92,14 +92,14 @@ public static function getRetryFunctionByRanges( ): callable { return function ( $retries, - Request $request, - ?Response $response = null + RequestInterface $request, + ?ResponseInterface $response = null ) use ($ranges, $maxRetries) { if ($retries >= $maxRetries) { return false; } - if (!$response instanceof Response) { + if (!$response instanceof ResponseInterface) { return false; } @@ -130,14 +130,14 @@ public static function getRetryFunction( ): callable { return function ( $retries, - Request $request, - ?Response $response = null + RequestInterface $request, + ?ResponseInterface $response = null ) use ($codes, $maxRetries) { if ($retries >= $maxRetries) { return false; } - if (($response instanceof Response) && in_array($response->getStatusCode(), $codes)) { + if (($response instanceof ResponseInterface) && in_array($response->getStatusCode(), $codes)) { return true; } @@ -151,15 +151,18 @@ public static function getRetryFunctionByConnectionErrors( ): callable { return function ( $retries, - Request $request, - ?Response $response = null, + RequestInterface $request, + ?ResponseInterface $response = null, $exception = null ) use ($maxRetries, $curlErrorCodes) { if ($retries >= $maxRetries) { return false; } - if (!$exception instanceof ConnectException) { + // Guzzle 7 reports connection failures as ConnectException, Guzzle 8 + // splits them across ConnectException and NetworkException. Both + // implement PSR-18's NetworkExceptionInterface. + if (!$exception instanceof NetworkExceptionInterface) { return false; } @@ -167,13 +170,8 @@ public static function getRetryFunctionByConnectionErrors( return true; } - $handlerContext = $exception->getHandlerContext(); - $errno = $handlerContext['errno'] ?? null; - - if (is_numeric($errno) && in_array((int) $errno, $curlErrorCodes, true)) { - return true; - } - + // RequestException::getHandlerContext() was removed in Guzzle 8, so the + // cURL errno is read from the exception message in both major versions. if (1 === preg_match('/cURL error\s+(\d+):/i', $exception->getMessage(), $matches)) { return in_array((int) $matches[1], $curlErrorCodes, true); } diff --git a/tests/Unit/GeneratedApiClientTest.php b/tests/Unit/GeneratedApiClientTest.php new file mode 100644 index 000000000..6ca8cdd19 --- /dev/null +++ b/tests/Unit/GeneratedApiClientTest.php @@ -0,0 +1,150 @@ +apiWithMock([new Response(201, ['Content-Type' => 'application/json'], '{"id":"1","properties":{}}')], $sent); + + $input = new SimplePublicObjectInputForCreate(); + $input->setProperties(['email' => 'test@example.com']); + + $api->create($input); + + $this->assertCount(1, $sent); + $this->assertSame('POST', $sent[0]->getMethod()); + $this->assertSame('application/json', $sent[0]->getHeaderLine('Content-Type')); + $this->assertSame( + ['properties' => ['email' => 'test@example.com']], + json_decode((string) $sent[0]->getBody(), true) + ); + } + + /** @test */ + public function itConvertsAnErrorResponseIntoAnApiExceptionWithHeadersAndBody(): void + { + $sent = []; + $api = $this->apiWithMock([new Response(400, ['X-Trace' => 'abc'], '{"message":"bad request"}')], $sent); + + try { + $api->getById('1'); + $this->fail('Expected ApiException to be thrown'); + } catch (ApiException $e) { + $this->assertSame(400, $e->getCode()); + $this->assertSame('{"message":"bad request"}', $e->getResponseBody()); + $this->assertSame(['abc'], $e->getResponseHeaders()['X-Trace']); + } + } + + /** @test */ + public function itConvertsAResponselessRequestExceptionIntoAnApiException(): void + { + $sent = []; + $api = $this->apiWithMock([ + new RequestException('Body could not be read', new Request('GET', 'https://api.hubapi.com')), + ], $sent); + + try { + $api->getById('1'); + $this->fail('Expected ApiException to be thrown'); + } catch (ApiException $e) { + $this->assertStringContainsString('Body could not be read', $e->getMessage()); + $this->assertNull($e->getResponseBody()); + $this->assertNull($e->getResponseHeaders()); + } + } + + /** @test */ + public function itConvertsANetworkFailureIntoAnApiException(): void + { + $sent = []; + $api = $this->apiWithMock([ + new ConnectException('cURL error 7: Failed to connect', new Request('GET', 'https://api.hubapi.com')), + ], $sent); + + try { + $api->getById('1'); + $this->fail('Expected ApiException to be thrown'); + } catch (ApiException $e) { + $this->assertStringContainsString('Failed to connect', $e->getMessage()); + $this->assertNull($e->getResponseBody()); + } + } + + /** @test */ + public function itConvertsAResponselessFailureIntoAnApiExceptionOnTheAsyncPath(): void + { + $sent = []; + $api = $this->apiWithMock([ + new ConnectException('cURL error 7: Failed to connect', new Request('GET', 'https://api.hubapi.com')), + ], $sent); + + $this->expectException(ApiException::class); + + $api->getByIdAsync('1')->wait(); + } + + /** @test */ + public function itConvertsAnErrorResponseIntoAnApiExceptionOnTheAsyncPath(): void + { + $sent = []; + $api = $this->apiWithMock([new Response(404, ['X-Trace' => 'abc'], '{"message":"not found"}')], $sent); + + try { + $api->getByIdAsync('1')->wait(); + $this->fail('Expected ApiException to be thrown'); + } catch (ApiException $e) { + $this->assertSame(404, $e->getCode()); + $this->assertSame('{"message":"not found"}', $e->getResponseBody()); + } + } + + /** + * @param array $queue + * @param array|mixed $sent + */ + private function apiWithMock(array $queue, array &$sent): BasicApi + { + $stack = HandlerStack::create(new MockHandler($queue)); + $stack->push(function (callable $handler) use (&$sent) { + return function (RequestInterface $request, array $options) use ($handler, &$sent) { + $sent[] = $request; + + return $handler($request, $options); + }; + }); + + $config = new Configuration(); + $config->setAccessToken('test-token'); + + return new BasicApi(new Client(['handler' => $stack]), $config); + } +} diff --git a/tests/Unit/Http/RequestTest.php b/tests/Unit/Http/RequestTest.php index 028ea1abb..33b8b611a 100644 --- a/tests/Unit/Http/RequestTest.php +++ b/tests/Unit/Http/RequestTest.php @@ -93,6 +93,17 @@ public function createContactsDefaultJsonFasle(): void ], $request->getOptionsForSending()); } + /** @test */ + public function itNormalizesTheRequestMethodCasing(): void + { + $request = new Request(new Config(), [ + 'path' => '/crm/v3/objects/contacts', + 'method' => 'post', + ]); + + $this->assertSame('POST', $request->getMethod()); + } + protected function getHeaders(Config $config, bool $defaultJson = true): array { $headers = [ diff --git a/tests/Unit/RetryMiddlewareFactoryTest.php b/tests/Unit/RetryMiddlewareFactoryTest.php index 92b137793..de5f66968 100644 --- a/tests/Unit/RetryMiddlewareFactoryTest.php +++ b/tests/Unit/RetryMiddlewareFactoryTest.php @@ -3,6 +3,7 @@ namespace Hubspot\Tests\Unit; use GuzzleHttp\Exception\ConnectException; +use GuzzleHttp\Exception\RequestException; use GuzzleHttp\Psr7\Request; use HubSpot\RetryMiddlewareFactory; use PHPUnit\Framework\TestCase; @@ -15,22 +16,20 @@ class RetryMiddlewareFactoryTest extends TestCase { /** @test */ - public function itRetriesRetriableConnectionErrorsByErrno(): void + public function itRetriesRetriableConnectionErrorsByMessage(): void { $retry = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors(RetryMiddlewareFactory::TRANSIENT_CURL_ERROR_CODES, 3); $request = new Request('GET', 'https://api.hubapi.com/test'); $exception = new ConnectException( 'cURL error 56: OpenSSL SSL_read unexpected eof while reading', - $request, - null, - ['errno' => 56] + $request ); $this->assertTrue($retry(0, $request, null, $exception)); } /** @test */ - public function itRetriesRetriableConnectionErrorsByMessageWhenErrnoMissing(): void + public function itRetriesRetriableSendErrors(): void { $retry = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors(RetryMiddlewareFactory::TRANSIENT_CURL_ERROR_CODES, 3); $request = new Request('GET', 'https://api.hubapi.com/test'); @@ -49,14 +48,42 @@ public function itDoesNotRetryNonRetriableConnectionErrors(): void $request = new Request('GET', 'https://api.hubapi.com/test'); $exception = new ConnectException( 'cURL error 60: SSL certificate problem', - $request, - null, - ['errno' => 60] + $request ); $this->assertFalse($retry(0, $request, null, $exception)); } + /** @test */ + public function itDoesNotRetryConnectionErrorsWithoutACurlErrno(): void + { + $retry = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors(RetryMiddlewareFactory::TRANSIENT_CURL_ERROR_CODES, 3); + $request = new Request('GET', 'https://api.hubapi.com/test'); + $exception = new ConnectException('Connection failed', $request); + + $this->assertFalse($retry(0, $request, null, $exception)); + } + + /** @test */ + public function itRetriesAnyConnectionErrorWhenNoCurlErrorCodesAreGiven(): void + { + $retry = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors([], 3); + $request = new Request('GET', 'https://api.hubapi.com/test'); + $exception = new ConnectException('Connection failed', $request); + + $this->assertTrue($retry(0, $request, null, $exception)); + } + + /** @test */ + public function itDoesNotRetryExceptionsThatAreNotNetworkFailures(): void + { + $retry = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors(RetryMiddlewareFactory::TRANSIENT_CURL_ERROR_CODES, 3); + $request = new Request('GET', 'https://api.hubapi.com/test'); + $exception = new RequestException('cURL error 56: something else', $request); + + $this->assertFalse($retry(0, $request, null, $exception)); + } + /** @test */ public function itStopsRetryingWhenMaxRetriesReached(): void { @@ -64,9 +91,7 @@ public function itStopsRetryingWhenMaxRetriesReached(): void $request = new Request('GET', 'https://api.hubapi.com/test'); $exception = new ConnectException( 'cURL error 56: OpenSSL SSL_read unexpected eof while reading', - $request, - null, - ['errno' => 56] + $request ); $this->assertFalse($retry(1, $request, null, $exception)); From 64abbb171abd565d1d1e5b6ad7962c6135a690e0 Mon Sep 17 00:00:00 2001 From: Venca Krecl Date: Thu, 10 Sep 2026 08:12:39 +0200 Subject: [PATCH 2/4] Fix remaining generated clients and Guzzle 7 transfer retries --- CHANGELOG.md | 2 +- codegen/Crm/Objects/Api/AdvancedApi.php | 6 +-- codegen/Crm/Objects/Api/BasicApi.php | 30 +++++++------- codegen/Crm/Objects/Api/BatchApi.php | 30 +++++++------- codegen/Crm/Objects/Api/SearchApi.php | 6 +-- codegen/Webhooks/Api/BasicApi.php | 48 +++++++++++------------ lib/RetryMiddlewareFactory.php | 28 +++++++++---- tests/Unit/GeneratedApiClientTest.php | 40 +++++++++++++++++++ tests/Unit/RetryMiddlewareFactoryTest.php | 23 +++++++++++ 9 files changed, 144 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ba377939..2ee95970f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Generated clients no longer call the removed `\GuzzleHttp\Utils::jsonEncode()`; they use `json_encode(..., JSON_THROW_ON_ERROR)`. - Generated clients no longer assume `RequestException::getResponse()` exists (removed in Guzzle 8, where only `ResponseException` subclasses carry a response). Failures without a response now produce an `ApiException` with `null` headers/body instead of a fatal error, on both the sync and async paths. - Generated clients catch `Psr\Http\Client\NetworkExceptionInterface` instead of `GuzzleHttp\Exception\ConnectException`, so Guzzle 8's `NetworkException` (send/receive errors, HTTP/2 and HTTP/3 failures) is still converted to an `ApiException`. -- `RetryMiddlewareFactory::getRetryFunctionByConnectionErrors()` now matches `Psr\Http\Client\NetworkExceptionInterface` and reads the cURL errno from the exception message, because Guzzle 8 removed `RequestException::getHandlerContext()` and reclassified cURL errors 52, 55 and 56 as `NetworkException` rather than `ConnectException`. As a side effect, cURL errors 55 and 56 are now retried on Guzzle 7 as well, which is what `TRANSIENT_CURL_ERROR_CODES` always documented. +- `RetryMiddlewareFactory::getRetryFunctionByConnectionErrors()` now matches `Psr\Http\Client\NetworkExceptionInterface` and reads the cURL errno from handler context when available, falling back to the exception message because Guzzle 8 removed `RequestException::getHandlerContext()` and reclassified cURL errors 52, 55 and 56 as `NetworkException` rather than `ConnectException`. Guzzle 7 send/receive failures (cURL errors 55 and 56) are also retried by recognizing `RequestException` with matching cURL handler context. - `apiRequest()` uppercases the `method` option. Guzzle 7 uppercased request methods, Guzzle 8 sends them verbatim. - The retry decider callbacks accept any PSR-7 `RequestInterface`/`ResponseInterface` instead of only `GuzzleHttp\Psr7\Request`/`Response`. diff --git a/codegen/Crm/Objects/Api/AdvancedApi.php b/codegen/Crm/Objects/Api/AdvancedApi.php index 67a450bb7..7ad357b89 100644 --- a/codegen/Crm/Objects/Api/AdvancedApi.php +++ b/codegen/Crm/Objects/Api/AdvancedApi.php @@ -303,7 +303,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -311,8 +311,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); diff --git a/codegen/Crm/Objects/Api/BasicApi.php b/codegen/Crm/Objects/Api/BasicApi.php index 707f5fa42..fcff2cedd 100644 --- a/codegen/Crm/Objects/Api/BasicApi.php +++ b/codegen/Crm/Objects/Api/BasicApi.php @@ -259,7 +259,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -267,8 +267,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); @@ -565,7 +565,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -573,8 +573,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); @@ -890,7 +890,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -898,8 +898,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); @@ -1271,7 +1271,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -1279,8 +1279,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); @@ -1635,7 +1635,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -1643,8 +1643,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); diff --git a/codegen/Crm/Objects/Api/BatchApi.php b/codegen/Crm/Objects/Api/BatchApi.php index 6d5471535..db17670a2 100644 --- a/codegen/Crm/Objects/Api/BatchApi.php +++ b/codegen/Crm/Objects/Api/BatchApi.php @@ -259,7 +259,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -267,8 +267,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); @@ -578,7 +578,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -586,8 +586,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); @@ -897,7 +897,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -905,8 +905,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); @@ -1220,7 +1220,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -1228,8 +1228,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); @@ -1550,7 +1550,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -1558,8 +1558,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); diff --git a/codegen/Crm/Objects/Api/SearchApi.php b/codegen/Crm/Objects/Api/SearchApi.php index 42e637103..75300829c 100644 --- a/codegen/Crm/Objects/Api/SearchApi.php +++ b/codegen/Crm/Objects/Api/SearchApi.php @@ -303,7 +303,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -311,8 +311,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); diff --git a/codegen/Webhooks/Api/BasicApi.php b/codegen/Webhooks/Api/BasicApi.php index 9fde87fb6..9ea9b637a 100644 --- a/codegen/Webhooks/Api/BasicApi.php +++ b/codegen/Webhooks/Api/BasicApi.php @@ -268,7 +268,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -276,8 +276,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); @@ -515,7 +515,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -523,8 +523,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); @@ -806,7 +806,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -814,8 +814,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); @@ -1112,7 +1112,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -1120,8 +1120,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); @@ -1414,7 +1414,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -1422,8 +1422,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); @@ -1701,7 +1701,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -1709,8 +1709,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); @@ -1992,7 +1992,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -2000,8 +2000,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); @@ -2303,7 +2303,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); + $statusCode = $response ? $response->getStatusCode() : 0; throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -2311,8 +2311,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : [], - $response ? (string) $response->getBody() : '' + $response ? $response->getHeaders() : null, + $response ? (string) $response->getBody() : null ); } ); diff --git a/lib/RetryMiddlewareFactory.php b/lib/RetryMiddlewareFactory.php index 53047871e..6a077422f 100644 --- a/lib/RetryMiddlewareFactory.php +++ b/lib/RetryMiddlewareFactory.php @@ -2,6 +2,7 @@ namespace HubSpot; +use GuzzleHttp\Exception\RequestException; use GuzzleHttp\Middleware; use Psr\Http\Client\NetworkExceptionInterface; use Psr\Http\Message\RequestInterface; @@ -159,10 +160,23 @@ public static function getRetryFunctionByConnectionErrors( return false; } - // Guzzle 7 reports connection failures as ConnectException, Guzzle 8 - // splits them across ConnectException and NetworkException. Both - // implement PSR-18's NetworkExceptionInterface. - if (!$exception instanceof NetworkExceptionInterface) { + // Guzzle 7 exposes cURL context; Guzzle 8 exposes only the message. + $context = ($exception instanceof RequestException || $exception instanceof NetworkExceptionInterface) + && method_exists($exception, 'getHandlerContext') + ? $exception->getHandlerContext() + : []; + $errno = $context['errno'] ?? null; + if (!is_numeric($errno) && $exception instanceof \Throwable + && 1 === preg_match('/cURL error\s+(\d+):/i', $exception->getMessage(), $matches)) { + $errno = $matches[1]; + } + + // Guzzle 7 uses RequestException for send/receive errors (55/56). + // Require cURL context to distinguish them from other request failures. + $legacyTransferError = $exception instanceof RequestException + && is_numeric($context['errno'] ?? null) + && in_array((int) $context['errno'], self::TRANSIENT_CURL_ERROR_CODES, true); + if (!$exception instanceof NetworkExceptionInterface && !$legacyTransferError) { return false; } @@ -170,10 +184,8 @@ public static function getRetryFunctionByConnectionErrors( return true; } - // RequestException::getHandlerContext() was removed in Guzzle 8, so the - // cURL errno is read from the exception message in both major versions. - if (1 === preg_match('/cURL error\s+(\d+):/i', $exception->getMessage(), $matches)) { - return in_array((int) $matches[1], $curlErrorCodes, true); + if (is_numeric($errno)) { + return in_array((int) $errno, $curlErrorCodes, true); } return false; diff --git a/tests/Unit/GeneratedApiClientTest.php b/tests/Unit/GeneratedApiClientTest.php index 6ca8cdd19..5fae026de 100644 --- a/tests/Unit/GeneratedApiClientTest.php +++ b/tests/Unit/GeneratedApiClientTest.php @@ -4,6 +4,7 @@ use GuzzleHttp\Client; use GuzzleHttp\Exception\ConnectException; +use GuzzleHttp\Exception\NetworkException; use GuzzleHttp\Exception\RequestException; use GuzzleHttp\Handler\MockHandler; use GuzzleHttp\HandlerStack; @@ -13,6 +14,8 @@ use HubSpot\Client\Crm\Contacts\ApiException; use HubSpot\Client\Crm\Contacts\Configuration; use HubSpot\Client\Crm\Contacts\Model\SimplePublicObjectInputForCreate; +use HubSpot\Client\Webhooks\Api\BasicApi as WebhooksBasicApi; +use HubSpot\Client\Webhooks\Model\SubscriptionCreateRequest; use PHPUnit\Framework\TestCase; use Psr\Http\Message\RequestInterface; @@ -28,6 +31,43 @@ */ class GeneratedApiClientTest extends TestCase { + public function testWebhooksRequestBodyIsCompatibleWithBothGuzzleVersions(): void + { + $api = new WebhooksBasicApi(); + $input = new SubscriptionCreateRequest(); + $input->setActive(true); + $request = $api->createRequest(123, $input); + $this->assertSame(['active' => true], json_decode((string) $request->getBody(), true)); + } + + public function testWebhooksHandlesResponselessExceptionsOnBothPaths(): void + { + $request = new Request('GET', 'https://api.hubapi.com'); + $exceptions = [new RequestException('No response', $request), new ConnectException('Connection failed', $request)]; + if (class_exists(NetworkException::class)) { + $exceptions[] = new NetworkException('Network failed', $request); + } + foreach ($exceptions as $exception) { + foreach ([false, true] as $async) { + $client = new Client(['handler' => HandlerStack::create(new MockHandler([$exception]))]); + $api = new WebhooksBasicApi($client); + + try { + if ($async) { + $api->getAllAsync(123)->wait(); + } else { + $api->getAll(123); + } + $this->fail('Expected ApiException'); + } catch (\HubSpot\Client\Webhooks\ApiException $error) { + $this->assertSame(0, $error->getCode()); + $this->assertNull($error->getResponseHeaders()); + $this->assertNull($error->getResponseBody()); + } + } + } + } + /** @test */ public function itSendsAJsonEncodedBody(): void { diff --git a/tests/Unit/RetryMiddlewareFactoryTest.php b/tests/Unit/RetryMiddlewareFactoryTest.php index de5f66968..20be71ed1 100644 --- a/tests/Unit/RetryMiddlewareFactoryTest.php +++ b/tests/Unit/RetryMiddlewareFactoryTest.php @@ -3,6 +3,7 @@ namespace Hubspot\Tests\Unit; use GuzzleHttp\Exception\ConnectException; +use GuzzleHttp\Exception\NetworkException; use GuzzleHttp\Exception\RequestException; use GuzzleHttp\Psr7\Request; use HubSpot\RetryMiddlewareFactory; @@ -15,6 +16,28 @@ */ class RetryMiddlewareFactoryTest extends TestCase { + public function testRetriesTransferFailuresUsingActualGuzzleExceptionTypes(): void + { + $request = new Request('GET', 'https://api.hubapi.com/test'); + foreach ([52, 55, 56] as $errno) { + if (class_exists(NetworkException::class)) { + $exception = new NetworkException("cURL error {$errno}: transfer failed", $request); + } elseif (52 === $errno) { + $exception = new ConnectException('Transfer failed', $request, null, ['errno' => $errno]); + } else { + $exception = new RequestException('Transfer failed', $request, null, null, ['errno' => $errno]); + } + + $retry = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors(); + $this->assertTrue($retry(0, $request, null, $exception), "cURL error {$errno}"); + $this->assertFalse($retry(5, $request, null, $exception)); + $restricted = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors([7]); + $this->assertFalse($restricted(0, $request, null, $exception)); + $unrestricted = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors([]); + $this->assertTrue($unrestricted(0, $request, null, $exception)); + } + } + /** @test */ public function itRetriesRetriableConnectionErrorsByMessage(): void { From 594058a3e318b2024977e5670ee0ef72e153208d Mon Sep 17 00:00:00 2001 From: Venca Krecl Date: Thu, 24 Sep 2026 18:09:03 +0200 Subject: [PATCH 3/4] Restore generated clients to master versions --- codegen/Crm/Objects/Api/AdvancedApi.php | 6 ++-- codegen/Crm/Objects/Api/BasicApi.php | 30 ++++++++-------- codegen/Crm/Objects/Api/BatchApi.php | 30 ++++++++-------- codegen/Crm/Objects/Api/SearchApi.php | 6 ++-- codegen/Webhooks/Api/BasicApi.php | 48 ++++++++++++------------- tests/Unit/GeneratedApiClientTest.php | 4 +-- 6 files changed, 62 insertions(+), 62 deletions(-) diff --git a/codegen/Crm/Objects/Api/AdvancedApi.php b/codegen/Crm/Objects/Api/AdvancedApi.php index 7ad357b89..67a450bb7 100644 --- a/codegen/Crm/Objects/Api/AdvancedApi.php +++ b/codegen/Crm/Objects/Api/AdvancedApi.php @@ -303,7 +303,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -311,8 +311,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); diff --git a/codegen/Crm/Objects/Api/BasicApi.php b/codegen/Crm/Objects/Api/BasicApi.php index fcff2cedd..707f5fa42 100644 --- a/codegen/Crm/Objects/Api/BasicApi.php +++ b/codegen/Crm/Objects/Api/BasicApi.php @@ -259,7 +259,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -267,8 +267,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); @@ -565,7 +565,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -573,8 +573,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); @@ -890,7 +890,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -898,8 +898,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); @@ -1271,7 +1271,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -1279,8 +1279,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); @@ -1635,7 +1635,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -1643,8 +1643,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); diff --git a/codegen/Crm/Objects/Api/BatchApi.php b/codegen/Crm/Objects/Api/BatchApi.php index db17670a2..6d5471535 100644 --- a/codegen/Crm/Objects/Api/BatchApi.php +++ b/codegen/Crm/Objects/Api/BatchApi.php @@ -259,7 +259,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -267,8 +267,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); @@ -578,7 +578,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -586,8 +586,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); @@ -897,7 +897,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -905,8 +905,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); @@ -1220,7 +1220,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -1228,8 +1228,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); @@ -1550,7 +1550,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -1558,8 +1558,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); diff --git a/codegen/Crm/Objects/Api/SearchApi.php b/codegen/Crm/Objects/Api/SearchApi.php index 75300829c..42e637103 100644 --- a/codegen/Crm/Objects/Api/SearchApi.php +++ b/codegen/Crm/Objects/Api/SearchApi.php @@ -303,7 +303,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -311,8 +311,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); diff --git a/codegen/Webhooks/Api/BasicApi.php b/codegen/Webhooks/Api/BasicApi.php index 9ea9b637a..9fde87fb6 100644 --- a/codegen/Webhooks/Api/BasicApi.php +++ b/codegen/Webhooks/Api/BasicApi.php @@ -268,7 +268,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -276,8 +276,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); @@ -515,7 +515,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -523,8 +523,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); @@ -806,7 +806,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -814,8 +814,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); @@ -1112,7 +1112,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -1120,8 +1120,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); @@ -1414,7 +1414,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -1422,8 +1422,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); @@ -1701,7 +1701,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -1709,8 +1709,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); @@ -1992,7 +1992,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -2000,8 +2000,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); @@ -2303,7 +2303,7 @@ function ($response) use ($returnType) { }, function ($exception) { $response = method_exists($exception, 'getResponse') ? $exception->getResponse() : null; - $statusCode = $response ? $response->getStatusCode() : 0; + $statusCode = $response ? $response->getStatusCode() : $exception->getCode(); throw new ApiException( sprintf( '[%d] Error connecting to the API (%s)', @@ -2311,8 +2311,8 @@ function ($exception) { $exception->getRequest()->getUri() ), $statusCode, - $response ? $response->getHeaders() : null, - $response ? (string) $response->getBody() : null + $response ? $response->getHeaders() : [], + $response ? (string) $response->getBody() : '' ); } ); diff --git a/tests/Unit/GeneratedApiClientTest.php b/tests/Unit/GeneratedApiClientTest.php index 5fae026de..554987e11 100644 --- a/tests/Unit/GeneratedApiClientTest.php +++ b/tests/Unit/GeneratedApiClientTest.php @@ -61,8 +61,8 @@ public function testWebhooksHandlesResponselessExceptionsOnBothPaths(): void $this->fail('Expected ApiException'); } catch (\HubSpot\Client\Webhooks\ApiException $error) { $this->assertSame(0, $error->getCode()); - $this->assertNull($error->getResponseHeaders()); - $this->assertNull($error->getResponseBody()); + $this->assertSame($async ? [] : null, $error->getResponseHeaders()); + $this->assertSame($async ? '' : null, $error->getResponseBody()); } } } From 397040ea668d71011ab0cc7486c9ee6a04cd30dd Mon Sep 17 00:00:00 2001 From: Venca Krecl Date: Thu, 24 Sep 2026 19:26:32 +0200 Subject: [PATCH 4/4] Fix transfer failure retries across Guzzle versions --- CHANGELOG.md | 4 +- lib/RetryMiddlewareFactory.php | 37 +++++--- tests/Unit/RetryMiddlewareFactoryTest.php | 102 ++++++++++++++++------ 3 files changed, 102 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ee95970f..42be33e67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,9 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `guzzlehttp/guzzle` `^8.0` and `guzzlehttp/psr7` `^3.0` are now allowed; Guzzle 7 remains supported. - Generated clients no longer call the removed `\GuzzleHttp\Utils::jsonEncode()`; they use `json_encode(..., JSON_THROW_ON_ERROR)`. -- Generated clients no longer assume `RequestException::getResponse()` exists (removed in Guzzle 8, where only `ResponseException` subclasses carry a response). Failures without a response now produce an `ApiException` with `null` headers/body instead of a fatal error, on both the sync and async paths. +- Generated clients no longer assume `RequestException::getResponse()` exists (removed in Guzzle 8, where only `ResponseException` subclasses carry a response). Failures without a response now produce an `ApiException` instead of a fatal error: with `null` headers/body on the sync path, and with empty headers/body (`[]` and `''`) on the async path, which is unchanged. - Generated clients catch `Psr\Http\Client\NetworkExceptionInterface` instead of `GuzzleHttp\Exception\ConnectException`, so Guzzle 8's `NetworkException` (send/receive errors, HTTP/2 and HTTP/3 failures) is still converted to an `ApiException`. -- `RetryMiddlewareFactory::getRetryFunctionByConnectionErrors()` now matches `Psr\Http\Client\NetworkExceptionInterface` and reads the cURL errno from handler context when available, falling back to the exception message because Guzzle 8 removed `RequestException::getHandlerContext()` and reclassified cURL errors 52, 55 and 56 as `NetworkException` rather than `ConnectException`. Guzzle 7 send/receive failures (cURL errors 55 and 56) are also retried by recognizing `RequestException` with matching cURL handler context. +- `RetryMiddlewareFactory::getRetryFunctionByConnectionErrors()` now matches `Psr\Http\Client\NetworkExceptionInterface` and reads the cURL errno from handler context when available, falling back to the exception message because Guzzle 8 removed `RequestException::getHandlerContext()` and reclassified cURL errors 52, 55 and 56 as `NetworkException` rather than `ConnectException`. Failures that happen after the response headers arrived are also retried, whether they are reported as Guzzle 8's `ResponseTransferException` or as Guzzle 7's `RequestException` with cURL handler context. Which cURL errors are retried always follows the `$curlErrorCodes` argument, on both majors. - `apiRequest()` uppercases the `method` option. Guzzle 7 uppercased request methods, Guzzle 8 sends them verbatim. - The retry decider callbacks accept any PSR-7 `RequestInterface`/`ResponseInterface` instead of only `GuzzleHttp\Psr7\Request`/`Response`. diff --git a/lib/RetryMiddlewareFactory.php b/lib/RetryMiddlewareFactory.php index 6a077422f..4c26b13bd 100644 --- a/lib/RetryMiddlewareFactory.php +++ b/lib/RetryMiddlewareFactory.php @@ -3,6 +3,7 @@ namespace HubSpot; use GuzzleHttp\Exception\RequestException; +use GuzzleHttp\Exception\ResponseTransferException; use GuzzleHttp\Middleware; use Psr\Http\Client\NetworkExceptionInterface; use Psr\Http\Message\RequestInterface; @@ -171,12 +172,7 @@ public static function getRetryFunctionByConnectionErrors( $errno = $matches[1]; } - // Guzzle 7 uses RequestException for send/receive errors (55/56). - // Require cURL context to distinguish them from other request failures. - $legacyTransferError = $exception instanceof RequestException - && is_numeric($context['errno'] ?? null) - && in_array((int) $context['errno'], self::TRANSIENT_CURL_ERROR_CODES, true); - if (!$exception instanceof NetworkExceptionInterface && !$legacyTransferError) { + if (!static::isTransferFailure($exception, $context, $errno)) { return false; } @@ -184,11 +180,32 @@ public static function getRetryFunctionByConnectionErrors( return true; } - if (is_numeric($errno)) { - return in_array((int) $errno, $curlErrorCodes, true); - } + return is_numeric($errno) && in_array((int) $errno, $curlErrorCodes, true); + }; + } + + /** + * Identifies transfer failures; the caller filters cURL error codes. + * Guzzle 7 uses handler context; Guzzle 8 uses specific exception types. + * + * @param mixed $exception rejection reason, not necessarily a Throwable + * @param array $context cURL handler context, empty on Guzzle 8 + * @param mixed $errno cURL errno from the context or the message + */ + protected static function isTransferFailure($exception, array $context, $errno): bool + { + if ($exception instanceof NetworkExceptionInterface) { + return true; + } + if (!$exception instanceof RequestException) { return false; - }; + } + + if (is_numeric($context['errno'] ?? null)) { + return true; + } + + return $exception instanceof ResponseTransferException && is_numeric($errno); } } diff --git a/tests/Unit/RetryMiddlewareFactoryTest.php b/tests/Unit/RetryMiddlewareFactoryTest.php index 20be71ed1..7dd0a974a 100644 --- a/tests/Unit/RetryMiddlewareFactoryTest.php +++ b/tests/Unit/RetryMiddlewareFactoryTest.php @@ -5,7 +5,9 @@ use GuzzleHttp\Exception\ConnectException; use GuzzleHttp\Exception\NetworkException; use GuzzleHttp\Exception\RequestException; +use GuzzleHttp\Exception\ResponseTransferException; use GuzzleHttp\Psr7\Request; +use GuzzleHttp\Psr7\Response; use HubSpot\RetryMiddlewareFactory; use PHPUnit\Framework\TestCase; @@ -19,14 +21,8 @@ class RetryMiddlewareFactoryTest extends TestCase public function testRetriesTransferFailuresUsingActualGuzzleExceptionTypes(): void { $request = new Request('GET', 'https://api.hubapi.com/test'); - foreach ([52, 55, 56] as $errno) { - if (class_exists(NetworkException::class)) { - $exception = new NetworkException("cURL error {$errno}: transfer failed", $request); - } elseif (52 === $errno) { - $exception = new ConnectException('Transfer failed', $request, null, ['errno' => $errno]); - } else { - $exception = new RequestException('Transfer failed', $request, null, null, ['errno' => $errno]); - } + foreach (RetryMiddlewareFactory::TRANSIENT_CURL_ERROR_CODES as $errno) { + $exception = $this->connectionException($errno, $request); $retry = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors(); $this->assertTrue($retry(0, $request, null, $exception), "cURL error {$errno}"); @@ -38,30 +34,34 @@ public function testRetriesTransferFailuresUsingActualGuzzleExceptionTypes(): vo } } - /** @test */ - public function itRetriesRetriableConnectionErrorsByMessage(): void + public function testRetriesTransferFailuresThatHappenAfterResponseHeaders(): void { - $retry = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors(RetryMiddlewareFactory::TRANSIENT_CURL_ERROR_CODES, 3); $request = new Request('GET', 'https://api.hubapi.com/test'); - $exception = new ConnectException( - 'cURL error 56: OpenSSL SSL_read unexpected eof while reading', - $request - ); + foreach (RetryMiddlewareFactory::TRANSIENT_CURL_ERROR_CODES as $errno) { + $exception = $this->responseTransferException($errno, $request); - $this->assertTrue($retry(0, $request, null, $exception)); + $retry = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors(); + $this->assertTrue($retry(0, $request, null, $exception), "cURL error {$errno}"); + $this->assertFalse($retry(5, $request, null, $exception)); + $restricted = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors([7]); + $this->assertFalse($restricted(0, $request, null, $exception)); + $unrestricted = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors([]); + $this->assertTrue($unrestricted(0, $request, null, $exception)); + } } - /** @test */ - public function itRetriesRetriableSendErrors(): void + public function testRetriesCurlErrorCodesRequestedByTheCaller(): void { - $retry = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors(RetryMiddlewareFactory::TRANSIENT_CURL_ERROR_CODES, 3); $request = new Request('GET', 'https://api.hubapi.com/test'); - $exception = new ConnectException( - 'cURL error 55: Send failure: Broken pipe', - $request - ); - - $this->assertTrue($retry(0, $request, null, $exception)); + // CURLE_PARTIAL_FILE, not part of the default set. + $exception = $this->responseTransferException(18, $request); + + $requested = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors([18]); + $this->assertTrue($requested(0, $request, null, $exception)); + $default = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors(); + $this->assertFalse($default(0, $request, null, $exception)); + $unrestricted = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors([]); + $this->assertTrue($unrestricted(0, $request, null, $exception)); } /** @test */ @@ -107,16 +107,60 @@ public function itDoesNotRetryExceptionsThatAreNotNetworkFailures(): void $this->assertFalse($retry(0, $request, null, $exception)); } + public function testDoesNotRetryHttpErrorsContainingCurlErrorMessages(): void + { + $request = new Request('POST', 'https://api.hubapi.com/test'); + foreach ([400, 500] as $statusCode) { + $response = new Response($statusCode, ['Content-Type' => 'application/json'], '{"message":"cURL error 56: upstream failure"}'); + $exception = RequestException::create($request, $response); + $this->assertStringContainsString('cURL error 56:', $exception->getMessage()); + + foreach ([RetryMiddlewareFactory::TRANSIENT_CURL_ERROR_CODES, [56], []] as $curlErrorCodes) { + $retry = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors($curlErrorCodes); + $this->assertFalse($retry(0, $request, null, $exception)); + } + } + } + /** @test */ public function itStopsRetryingWhenMaxRetriesReached(): void { $retry = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors(RetryMiddlewareFactory::TRANSIENT_CURL_ERROR_CODES, 1); $request = new Request('GET', 'https://api.hubapi.com/test'); - $exception = new ConnectException( - 'cURL error 56: OpenSSL SSL_read unexpected eof while reading', - $request - ); + $exception = $this->connectionException(56, $request); $this->assertFalse($retry(1, $request, null, $exception)); } + + /** + * Builds a failure before response headers using the installed Guzzle version. + */ + private function connectionException(int $errno, Request $request): \Exception + { + $message = sprintf('cURL error %d: transfer failed', $errno); + + if (class_exists(NetworkException::class)) { + return new NetworkException($message, $request); + } + + if (in_array($errno, [6, 7, 28, 35, 52], true)) { + return new ConnectException($message, $request, null, ['errno' => $errno]); + } + + return new RequestException($message, $request, null, null, ['errno' => $errno]); + } + + /** + * Builds a failure after response headers using the installed Guzzle version. + */ + private function responseTransferException(int $errno, Request $request): \Exception + { + $message = sprintf('cURL error %d: transfer failed', $errno); + + if (class_exists(ResponseTransferException::class)) { + return new ResponseTransferException($message, $request, new Response(200)); + } + + return new RequestException($message, $request, new Response(200), null, ['errno' => $errno]); + } }