Skip to content

Migrate Adapter::call() to utopia-php/fetch#31

Open
lohanidamodar wants to merge 3 commits intomainfrom
feat/clo-4294-migrate-to-fetch
Open

Migrate Adapter::call() to utopia-php/fetch#31
lohanidamodar wants to merge 3 commits intomainfrom
feat/clo-4294-migrate-to-fetch

Conversation

@lohanidamodar
Copy link
Copy Markdown
Contributor

@lohanidamodar lohanidamodar commented May 3, 2026

Summary

  • Replace raw PHP curl in Pay\Adapter::call() with utopia-php/fetch (^1.1).
  • Response headers now read from Response::getHeaders() instead of a CURLOPT_HEADERFUNCTION callback.
  • Provider adapters (Stripe etc.) untouched.

Why

  • Unifies HTTP handling across the Utopia ecosystem.
  • Removes the PHP 8.5 curl_close() deprecation as a side effect.

Greptile follow-up commit

  • Removed unreachable throw $e; after handleError(). handleError() always throws, so the trailing throw was dead code. Behavior unchanged because it was already unreachable.
  • Dropped the $options parameter from Adapter::call(). The migration accepted it but never applied it to the fetch Client. No internal caller uses $options; any external subclass overriding call() will need to drop the arg from its override.

Header parity (verified)

The only consumer of $responseHeaders is Adapter.php reading $responseHeaders['content-type']. fetch's Client lowercases header keys identically to the legacy CURLOPT_HEADERFUNCTION implementation, so the lookup continues to work.

Test plan

  • composer lint (Pint)
  • composer test — same pre-existing failure pattern as main (Stripe tests need STRIPE_SECRET env var; auth 401s are environment, not regressions).

Greptile re-review fixes

  • Scoped JSON decode to application/json. The earlier follow-up preserved a quirk from the legacy code that effectively JSON-decoded any response with a non-empty content-type header. That would silently turn non-JSON bodies (e.g. text/html error pages) into null. Now gated explicitly with str_contains($responseType, 'application/json'). Audited downstream consumers (Stripe::execute and every ->call( site under src/Pay/Adapter/) — none rely on the broken behavior.
  • Initialised $response = null before the try block. Guards against subclasses that might override handleError() to not throw — without the init, PHP would emit an undefined-variable warning on the next line.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 3, 2026

Greptile Summary

This PR replaces the hand-rolled curl implementation in Adapter::call() with utopia-php/fetch (^1.1) and bumps the PHP floor to 8.1. A follow-up commit within the PR also removed the previously unreachable throw $e and dropped the now-unused $options parameter from the method signature.

The new str_contains($responseType, 'application/json') JSON-decode guard is actually an improvement over the legacy switch ($responseType && is_string($responseBody)) branch: the old switch operated on a boolean expression, so PHP loose-comparison (true == 'application/json') meant JSON was decoded for any truthy content-type, not just application/json. The new check is more precise.

Confidence Score: 4/5

Safe to merge; all previously flagged P1s are addressed in this branch, and no new blocking defects were found.

The follow-up commits within this PR resolve the previously raised issues (unreachable throw, dropped $options, undefined-variable concern via $response = null init). The one structural pattern that persists — $response->getHeaders() being called outside the try block, which becomes a fatal null dereference if a subclass overrides handleError without throwing — was already flagged in prior review threads and is scored here as P2 given that the base-class implementation unconditionally throws. No new P1 or P0 issues were identified.

src/Pay/Adapter.php — the try/catch structure around the fetch call warrants a second look if any subclass overrides handleError.

Important Files Changed

Filename Overview
src/Pay/Adapter.php Replaces raw curl with utopia-php/fetch Client; follow-up commits removed the unreachable throw $e and dropped the unused $options parameter. The str_contains('application/json') check is correct and improves on the old boolean-switch behavior. One structural concern remains: $response->getHeaders() is called outside the try block and becomes a fatal null dereference if a subclass overrides handleError() without throwing, though the base-class always throws.
composer.json PHP minimum version correctly bumped to 8.1 to match utopia-php/fetch requirement; new utopia-php/fetch ^1.1 runtime dependency added.
composer.lock Lockfile updated: utopia-php/fetch 1.1.2 added as a runtime package; several dev-dependency packages bumped to latest patch/minor versions.

Reviews (3): Last reviewed commit: "Scope JSON decode to application/json an..." | Re-trigger Greptile

Comment thread src/Pay/Adapter.php
Comment thread src/Pay/Adapter.php Outdated
eldadfux
eldadfux previously approved these changes May 3, 2026
Addresses Greptile review feedback on the utopia-php/fetch migration. handleError() always throws, so the trailing throw $e; was dead code. The $options array was accepted but silently never applied to the fetch Client; no internal caller uses it.
Comment thread src/Pay/Adapter.php Outdated
Comment thread src/Pay/Adapter.php
Addresses Greptile P1 review feedback. The previous content-type guard was too broad and would coerce non-JSON bodies to null via json_decode. Also explicitly initializes $response before the try block to guard against subclass overrides of handleError() that fail to throw.
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.

2 participants