Skip to content

Add Guzzle 8 support while keeping Guzzle 7 compatible - #622

Open
vencakrecl wants to merge 2 commits into
HubSpot:masterfrom
vencakrecl:guzzle-8
Open

vencakrecl wants to merge 2 commits into
HubSpot:masterfrom
vencakrecl:guzzle-8

Conversation

@vencakrecl

Copy link
Copy Markdown

Fixes #621.

Allows guzzlehttp/guzzle: ^7.3 || ^8.0 and guzzlehttp/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

Break Where
\GuzzleHttp\Utils::jsonEncode() removed 1007 call sites across 147 generated files
RequestException::getResponse() removed — only ResponseException subclasses carry a response 649 sync catch blocks + 649 async rejection handlers
No-response network failures are now NetworkException, not ConnectException 649 generated catch blocks
cURL errors 52, 55, 56 reclassified as NetworkException RetryMiddlewareFactory silently stops retrying
RequestException::getHandlerContext() removed lib/RetryMiddlewareFactory.php
Request method casing preserved verbatim apiRequest(['method' => 'post']) sends literal post

The retry one is the quietest failure: TRANSIENT_CURL_ERROR_CODES = [52, 55, 56] are all NetworkException in Guzzle 8, so instanceof ConnectException never matches and connection-error retries become a no-op with no error. (In Guzzle 7 only 52 was ever a ConnectException, 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)

  • Request bodies use json_encode(..., JSON_THROW_ON_ERROR).
  • Sync catch block and async rejection handler check for getResponse() before calling it, so a failure without a response produces an ApiException with null headers/body instead of a fatal Error. The async handler previously blew up on any responseless failure under Guzzle 7 as well.
  • catch (ConnectException) becomes catch (Psr\Http\Client\NetworkExceptionInterface), which both majors implement.

Hand-written code (lib/)

  • RetryMiddlewareFactory::getRetryFunctionByConnectionErrors() matches NetworkExceptionInterface and reads the cURL errno from the exception message.
  • Retry deciders accept any PSR-7 RequestInterface/ResponseInterface instead of only the concrete GuzzleHttp\Psr7 classes.
  • apiRequest() uppercases the method option.

Tests and CI

  • New tests/Unit/GeneratedApiClientTest.php drives a generated client over a MockHandler: 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 unpatched BasicApi.php — 2 errors + 1 failure, including Call to undefined method GuzzleHttp\Exception\ConnectException::getResponse().
  • RetryMiddlewareFactoryTest no longer relies on the removed handler-context constructor argument.
  • phpunit and phpspec now run a ['^7.3', '^8.0'] matrix.

Verification

23 tests and 139 specs pass against both guzzle 7.15.2 (with psr7 2.13.0) and guzzle 8.0.1 (with psr7 3.0.0 / promises 3.0.0). php-cs-fixer reports no changes.

Things checked and found not to be affected

  • Header values: 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 returning Configuration, unrelated to the removed ClientInterface::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 upstream openapi-generator's php/api.mustache still emits \GuzzleHttp\Utils::jsonEncode() and the unguarded getResponse(). The generator templates need the same three changes, or a regeneration will revert all 147 files.

🤖 Generated with Claude Code

@brandenrodgers

Copy link
Copy Markdown
Contributor

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 codegen/ folder from this PR and we should be able to get this in (assuming all tests are looking good!).

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>
@vencakrecl

Copy link
Copy Markdown
Author

Confirmed that this branch includes the current upstream master (1e0bcd88), including #625, and the fork's master matches upstream.

After dropping the original codegen/ changes, I found 51 generated API clients that still contain GuzzleHttp\Utils::jsonEncode() and the old exception handling. For example, Webhooks\Api\SubscriptionsApi::createRequest() fails under Guzzle 8 with Call to undefined method GuzzleHttp\Utils::jsonEncode(), and a request failure without a response triggers the removed RequestException::getResponse() method. The Contacts client covered by the existing tests was already fixed, so those tests passed.

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 RequestException, not ConnectException. We now recognize these errors through cURL handler context, retain context-based errno lookup where available, and fall back to the exception message for Guzzle 8. Regression tests cover the actual exception types, retry limits, configured error codes, and Webhooks JSON encoding and synchronous/asynchronous failures without a response.

Validation on PHP 8.5.10:

Dependencies PHPUnit phpspec
Guzzle 7.15.5 / PSR-7 2.13.1 26 tests, 72 assertions passed 139 examples passed
Guzzle 8.2.0 / PSR-7 3.1.0 26 tests, 78 assertions passed 139 examples passed

All 54 changed PHP files pass syntax checks, and git diff --check passes. PHPUnit reports 28 existing metadata deprecations on each version. These are local mocked-transport tests; the existing CI matrix will validate on PHP 8.2.

@vencakrecl

Copy link
Copy Markdown
Author

@brandenrodgers Can you take a look at my comment ⬆️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add guzzle 8.0 support

2 participants