Migrate Adapter::call() to utopia-php/fetch#31
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR replaces the hand-rolled curl implementation in The new Confidence Score: 4/5Safe 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
Reviews (3): Last reviewed commit: "Scope JSON decode to application/json an..." | Re-trigger Greptile |
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.
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.
Summary
Pay\Adapter::call()withutopia-php/fetch(^1.1).Response::getHeaders()instead of aCURLOPT_HEADERFUNCTIONcallback.Why
curl_close()deprecation as a side effect.Greptile follow-up commit
throw $e;afterhandleError().handleError()always throws, so the trailingthrowwas dead code. Behavior unchanged because it was already unreachable.$optionsparameter fromAdapter::call(). The migration accepted it but never applied it to the fetchClient. No internal caller uses$options; any external subclass overridingcall()will need to drop the arg from its override.Header parity (verified)
The only consumer of
$responseHeadersisAdapter.phpreading$responseHeaders['content-type']. fetch'sClientlowercases header keys identically to the legacyCURLOPT_HEADERFUNCTIONimplementation, so the lookup continues to work.Test plan
composer lint(Pint)composer test— same pre-existing failure pattern asmain(Stripe tests needSTRIPE_SECRETenv var; auth 401s are environment, not regressions).Greptile re-review fixes
application/json. The earlier follow-up preserved a quirk from the legacy code that effectively JSON-decoded any response with a non-emptycontent-typeheader. That would silently turn non-JSON bodies (e.g.text/htmlerror pages) intonull. Now gated explicitly withstr_contains($responseType, 'application/json'). Audited downstream consumers (Stripe::execute and every->call(site undersrc/Pay/Adapter/) — none rely on the broken behavior.$response = nullbefore the try block. Guards against subclasses that might overridehandleError()to not throw — without the init, PHP would emit an undefined-variable warning on the next line.