Migrate Adapter::call() to utopia-php/fetch#63
Conversation
Greptile SummaryThis PR replaces raw PHP curl in Confidence Score: 5/5Safe to merge — all previously identified P1/P2 issues are addressed and no new defects were found. Both previously flagged bugs (lowercase content-type key lookup and silent exception-code loss) are resolved. The adapter API is unchanged, the redundant json_decode calls in all four concrete adapters have been removed to prevent double-decoding, and the minimum PHP version bump to 8.1 is correctly reflected in both composer.json and composer.lock. No P0 or P1 issues remain. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "Apply Pint to Mixpanel.php and Analytics..." | Re-trigger Greptile |
Addresses Greptile review feedback on the utopia-php/fetch migration. The original code looked up the response Content-Type header with a capitalized key while utopia-php/fetch lowercases all keys, so the JSON decode branch never fired. Concrete adapters compensated with their own json_decode() calls; with the lookup fixed, those become redundant and are removed.
Pre-existing style violations (fully_qualified_strict_types, unary_operator_spaces) were blocking CI on the migration PR. Auto-fixed via composer format.
Summary
Analytics\Adapter::call()withutopia-php/fetch(^1.1).Response::getHeaders()instead of aCURLOPT_HEADERFUNCTIONcallback.Why
curl_close()deprecation as a side effect.Notable behavior fix (in follow-up commit)
The original code in
Adapter::call()looked up the response content-type with$responseHeaders['Content-Type'], but the legacyCURLOPT_HEADERFUNCTIONcallback already lowercased every header key — so the capitalized lookup never matched and the JSON-decode branch was dead code.utopia-php/fetchalso returns lowercase keys, so the migration didn't introduce the bug, but it's the right place to fix it. The follow-up commit:'content-type'.substr(..., 0, strpos(..., ';'))returned''for content-types without a;(so even with the right key case, plainapplication/jsonwould have missed).json_decode($x, true)calls in 4 concrete adapters (GoogleAnalytics, HubSpot, Orbit, Plausible) that compensated for the dead branch — without that change they'd start double-decoding.FetchException::getCode()when re-throwing as a plainException(Greptile P2).Test plan
composer lint(Pint)php -lon every modified filecomposer test— same pre-existing failure pattern asmain(expired test API keys for Plausible/Orbit/HubSpot/Mixpanel/ReoDev). No regression introduced.Lint cleanup commit
Pre-existing Pint violations in
Adapter/Mixpanel.phpandtests/Analytics/AnalyticsTest.phpwere blocking CI on this PR. Auto-fixed viacomposer format. These files are not part of the migration scope but the linter needs to pass for the PR to merge cleanly.