refactor: drop guzzle dependency for PSR-18 alignment#66
Open
kyeh-amp wants to merge 12 commits into
Open
Conversation
Backoff::doWithBackoff was never invoked in src/ or tests/. The class was a vestigial promise-chained retry helper with no call sites. Removing it also drops guzzlehttp/promises from the transitive dependency tree. Part of #30.
Switch from a hard guzzlehttp/guzzle requirement to PSR-18 / PSR-17 interfaces plus php-http/discovery for runtime client lookup. Guzzle moves to require-dev (still used by the test suite) and is listed under suggest alongside symfony/http-client. The new virtual psr/http-client-implementation constraint causes a composer install without any PSR-18 implementation to fail at install time rather than at runtime. SDK code still references the Guzzle HTTP client classes and will be migrated to PSR-18 + discovery in subsequent commits. Part of #30.
Introduce a synchronous PSR-18 retry decorator with method-gated retry behavior. RetryConfig defaults to 9 total attempts with exponential backoff (500ms -> 10s, scalar 1.5) applied to GET only, preserving the v1 retry budget while avoiding amplification on non-idempotent POSTs. Callers needing POST retries opt in via RetryConfig::retryMethods. Retries trigger only on PSR-18 ClientExceptionInterface; HTTP error responses (4xx/5xx) and non-PSR throwables propagate without retry, matching v1 GuzzleHttpClient middleware semantics. Not yet wired up; callers continue to use GuzzleHttpClient until the default-client swap commit. Part of #30.
Replace the SDK-owned HttpClientInterface abstraction with direct PSR-18 ClientInterface + PSR-17 RequestFactoryInterface usage at the three call sites: Amplitude::post, FlagConfigFetcher::fetch, and RemoteEvaluationClient::fetchWithOptions. Configs now expose three optional fields: httpClient: ?ClientInterface requestFactory: ?RequestFactoryInterface retryConfig: ?RetryConfig A new HttpClientFactory helper centralizes the resolution rules: when httpClient is null, a PSR-18 implementation is auto-discovered via php-http/discovery and wrapped in RetryingClient using the supplied RetryConfig (or sensible defaults). When httpClient is supplied, the SDK uses it verbatim — no auto-wrap — so users whose own client already implements retry are not double-wrapped. guzzleClientConfig is removed entirely; retry behavior is now configured via RetryConfig only. GuzzleHttpClient and HttpClientInterface remain in the codebase but are no longer referenced; they are removed in a follow-up commit. Tests migrate from MockGuzzleHttpClient (Guzzle MockHandler-backed) to a simple MockPsr18Client harness in tests/Util/. The Amplitude backoff tests are rewritten to opt POST into RetryConfig::retryMethods since the new default scopes retry to GET only; an additional test asserts that POST is NOT retried under the default RetryConfig. Part of #30.
The Guzzle-era SDK HTTP scaffolding has no remaining call sites after the PSR-18 migration. Delete the three classes and the autoload entry for GuzzleConstants. Also drop a stale GuzzleHttp\Psr7\Request import from AmplitudeTest that no longer references the type. guzzlehttp/guzzle stays in require-dev: tests use Guzzle's PSR-7 implementation (Request/Response) and ConnectException as a ClientExceptionInterface fixture, and one integration test uses GuzzleHttp\Client directly to fetch flag configs from the live API. Part of #30.
Replace direct GuzzleHttp\Psr7\Request, Response, and ConnectException usage with Psr7TestUtil helpers backed by php-http/discovery, plus a portable test-internal ClientExceptionInterface implementation. This lets the existing unit tests run against any PSR-7 implementation — required for the upcoming CI matrix axis that exercises symfony/http- client + nyholm/psr7 alongside the default Guzzle stack. Also consolidate RetryingClientTest's inline mock onto the canonical tests/Util/MockPsr18Client. Part of #30.
Lets the test run on PSR-18 stacks other than Guzzle. Part of #30.
Smoke-tests the SDK against a non-Guzzle PSR-18 stack to catch regressions in the discovery path. Verified locally. Part of #30.
Required by Composer 2.2+ allow-plugins enforcement. Without this, lowest-deps installs blocked at php-http/discovery 1.15.0.
Pin guzzle ^7.4 + psr7 ^2.0 in require-dev so discovery resolves a working PSR-17 factory under --prefer-lowest. Disable discovery's allow-plugins entry to prevent auto-injection of abandoned adapter packages during composer install.
Lets consumers catch missing-impl distinctly from transport faults.
1a9e655 to
e6130c5
Compare
The third arg of HttpClientFactory::resolveAll was always null at every call site and no config builder exposed a setter to override it. Inline Psr17FactoryDiscovery::findStreamFactory() and drop the dead parameter.
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.
Summary
guzzlehttp/guzzlerequirement with PSR-18 / PSR-17 +php-http/discovery. Consumers bring their own PSR-18 client (Guzzle, Symfony HttpClient, or any compliant implementation).HttpClientInterface,GuzzleHttpClient,guzzleClientConfig); builders now acceptPsr\Http\Client\ClientInterfaceandPsr\Http\Message\RequestFactoryInterfacedirectly.RetryingClientdecorator withRetryConfig. Auto-wraps the discovered client only — user-supplied clients are used verbatim to avoid layered-retry amplification. Default retry scope is GET only; opt POST in viaRetryConfig::retryMethodsto preserve the v1 retry knob without amplifying duplicate-event risk by default.Behavior changes (v2)
composer installfails on the unsatisfied virtualpsr/http-client-implementationconstraint at install time, not at runtime.guzzleClientConfigis removed. Retry is configured viaRetryConfigonly.HttpClientInterfaceandGuzzleHttpClientare removed. UsePsr\Http\Client\ClientInterfacedirectly.Test plan
tests-symfonyCI job exercises this)composer validate --strictcleanFollow-up
UPGRADING.mdwith worked migration examples (Guzzle / Symfony / raw PSR-18) — held until review feedback identifies which migrations need the most explanation.Closes #30.
Note
Medium Risk
Moderate risk because it rewires all outbound HTTP (events, remote fetch, flag config fetch) to PSR-18 discovery and a new retry layer, and changes public configuration surface away from
guzzleClientConfig/custom interface.Overview
Drops the SDK-owned Guzzle transport (
HttpClientInterface/GuzzleHttpClient/guzzleClientConfig) and switches core clients (Amplitude,RemoteEvaluationClient,LocalEvaluationClient/FlagConfigFetcher) to PSR-18ClientInterfaceplus PSR-17 factories resolved via newHttpClientFactory(php-http/discovery), throwingMissingHttpImplementationExceptionwhen discovery fails.Introduces
RetryingClient+RetryConfigto preserve transport-error retries for auto-discovered clients while avoiding double-retry for user-supplied clients; default retry is GET-only with explicit opt-in forPOST.Updates
composer.jsonto require PSR virtual packages andphp-http/discovery(Guzzle moved torequire-dev), adds a GitHub Actions smoke job that runs tests with Symfony HttpClient, and rewrites tests/utilities to use PSR-18 mocks and the new retry behavior.Reviewed by Cursor Bugbot for commit 9ad2cec. Bugbot is set up for automated code reviews on this repo. Configure here.