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..42be33e67 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` 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`. 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`. + ## [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..4c26b13bd 100644 --- a/lib/RetryMiddlewareFactory.php +++ b/lib/RetryMiddlewareFactory.php @@ -2,10 +2,12 @@ namespace HubSpot; -use GuzzleHttp\Exception\ConnectException; +use GuzzleHttp\Exception\RequestException; +use GuzzleHttp\Exception\ResponseTransferException; 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 +94,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 +132,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 +153,26 @@ 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 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]; + } + + if (!static::isTransferFailure($exception, $context, $errno)) { return false; } @@ -167,18 +180,32 @@ 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; - } + return is_numeric($errno) && in_array((int) $errno, $curlErrorCodes, true); + }; + } - if (1 === preg_match('/cURL error\s+(\d+):/i', $exception->getMessage(), $matches)) { - return in_array((int) $matches[1], $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/GeneratedApiClientTest.php b/tests/Unit/GeneratedApiClientTest.php new file mode 100644 index 000000000..554987e11 --- /dev/null +++ b/tests/Unit/GeneratedApiClientTest.php @@ -0,0 +1,190 @@ +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->assertSame($async ? [] : null, $error->getResponseHeaders()); + $this->assertSame($async ? '' : null, $error->getResponseBody()); + } + } + } + } + + /** @test */ + public function itSendsAJsonEncodedBody(): void + { + $sent = []; + $api = $this->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..7dd0a974a 100644 --- a/tests/Unit/RetryMiddlewareFactoryTest.php +++ b/tests/Unit/RetryMiddlewareFactoryTest.php @@ -3,7 +3,11 @@ namespace Hubspot\Tests\Unit; 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; @@ -14,61 +18,149 @@ */ class RetryMiddlewareFactoryTest extends TestCase { + public function testRetriesTransferFailuresUsingActualGuzzleExceptionTypes(): void + { + $request = new Request('GET', 'https://api.hubapi.com/test'); + 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}"); + $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)); + } + } + + public function testRetriesTransferFailuresThatHappenAfterResponseHeaders(): void + { + $request = new Request('GET', 'https://api.hubapi.com/test'); + foreach (RetryMiddlewareFactory::TRANSIENT_CURL_ERROR_CODES as $errno) { + $exception = $this->responseTransferException($errno, $request); + + $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)); + } + } + + public function testRetriesCurlErrorCodesRequestedByTheCaller(): void + { + $request = new Request('GET', 'https://api.hubapi.com/test'); + // 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 */ - public function itRetriesRetriableConnectionErrorsByErrno(): void + public function itDoesNotRetryNonRetriableConnectionErrors(): 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] + 'cURL error 60: SSL certificate problem', + $request ); - $this->assertTrue($retry(0, $request, null, $exception)); + $this->assertFalse($retry(0, $request, null, $exception)); } /** @test */ - public function itRetriesRetriableConnectionErrorsByMessageWhenErrnoMissing(): void + 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( - 'cURL error 55: Send failure: Broken pipe', - $request - ); + $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 itDoesNotRetryNonRetriableConnectionErrors(): void + public function itDoesNotRetryExceptionsThatAreNotNetworkFailures(): void { $retry = RetryMiddlewareFactory::getRetryFunctionByConnectionErrors(RetryMiddlewareFactory::TRANSIENT_CURL_ERROR_CODES, 3); $request = new Request('GET', 'https://api.hubapi.com/test'); - $exception = new ConnectException( - 'cURL error 60: SSL certificate problem', - $request, - null, - ['errno' => 60] - ); + $exception = new RequestException('cURL error 56: something else', $request); $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, - null, - ['errno' => 56] - ); + $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]); + } }