Add Guzzle 8 support while keeping Guzzle 7 compatible - #622
Open
vencakrecl wants to merge 1 commit into
Open
Conversation
Allows guzzlehttp/guzzle ^8.0 and guzzlehttp/psr7 ^3.0, and fixes the API removals and behaviour changes that Guzzle 8 introduces. Fixes HubSpot#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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #621.
Allows
guzzlehttp/guzzle: ^7.3 || ^8.0andguzzlehttp/psr7: ^1.7 || ^2.0 || ^3.0, and fixes the API removals and behaviour changes Guzzle 8 introduces. Guzzle 7 stays supported; both majors are covered in CI.What breaks under Guzzle 8
\GuzzleHttp\Utils::jsonEncode()removedRequestException::getResponse()removed — onlyResponseExceptionsubclasses carry a responseNetworkException, notConnectExceptionNetworkExceptionRetryMiddlewareFactorysilently stops retryingRequestException::getHandlerContext()removedlib/RetryMiddlewareFactory.phpapiRequest(['method' => 'post'])sends literalpostThe retry one is the quietest failure:
TRANSIENT_CURL_ERROR_CODES = [52, 55, 56]are allNetworkExceptionin Guzzle 8, soinstanceof ConnectExceptionnever matches and connection-error retries become a no-op with no error. (In Guzzle 7 only 52 was ever aConnectException, so 55 and 56 were never actually retried there either — this fixes that on both majors.)Changes
Generated clients (
codegen/, applied mechanically across 147 files)json_encode(..., JSON_THROW_ON_ERROR).getResponse()before calling it, so a failure without a response produces anApiExceptionwithnullheaders/body instead of a fatalError. The async handler previously blew up on any responseless failure under Guzzle 7 as well.catch (ConnectException)becomescatch (Psr\Http\Client\NetworkExceptionInterface), which both majors implement.Hand-written code (
lib/)RetryMiddlewareFactory::getRetryFunctionByConnectionErrors()matchesNetworkExceptionInterfaceand reads the cURL errno from the exception message.RequestInterface/ResponseInterfaceinstead of only the concreteGuzzleHttp\Psr7classes.apiRequest()uppercases themethodoption.Tests and CI
tests/Unit/GeneratedApiClientTest.phpdrives a generated client over aMockHandler: JSON body encoding, and error handling with and without a response, on both the sync and async paths. Verified as a real regression test by restoring an unpatchedBasicApi.php— 2 errors + 1 failure, includingCall to undefined method GuzzleHttp\Exception\ConnectException::getResponse().RetryMiddlewareFactoryTestno longer relies on the removed handler-context constructor argument.['^7.3', '^8.0']matrix.Verification
23 tests and 139 specs pass against both
guzzle 7.15.2(withpsr7 2.13.0) andguzzle 8.0.1(withpsr7 3.0.0/promises 3.0.0).php-cs-fixerreports no changes.Things checked and found not to be affected
ObjectSerializer::toHeaderValue()always casts to string, so psr7 3.x's string-only header rule is satisfied.Psr7\Query::build(): only referenced in docblocks —ObjectSerializer::buildQuery()is a local implementation, so the new value validation does not apply.getConfig()in the generated API classes is the SDK's own method returningConfiguration, unrelated to the removedClientInterface::getConfig().Psr7\Utils::streamFor()/tryFopen()still exist in psr7 3.x and are called with strings and resources only.Note for maintainers
codegen/is generated outside this repository, and upstreamopenapi-generator'sphp/api.mustachestill emits\GuzzleHttp\Utils::jsonEncode()and the unguardedgetResponse(). The generator templates need the same three changes, or a regeneration will revert all 147 files.🤖 Generated with Claude Code