Add Guzzle 8 support while keeping Guzzle 7 compatible - #622
vencakrecl wants to merge 2 commits into
Conversation
|
Thanks for submitting this! I've updated our generator to apply the same edits that you've manually created, and have a PR with them here. You should be able to drop all edits to the |
92047da to
8bbeb40
Compare
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>
8bbeb40 to
24ddc11
Compare
|
Confirmed that this branch includes the current upstream After dropping the original Commit 1dc1768 adds the compatibility edits back only for those 51 remaining clients. Could the generator changes also be applied to these clients so regeneration preserves the fixes? The same commit fixes Guzzle 7 retry handling: cURL errors 55/56 are represented by Validation on PHP 8.5.10:
All 54 changed PHP files pass syntax checks, and |
|
@brandenrodgers Can you take a look at my comment ⬆️ |
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