Skip to content

fix: skip malformed list entries and coerce account bodies instead of throwing TypeError - #21

Merged
wzul merged 1 commit into
mainfrom
fix/response-shape-coercion
Oct 23, 2026
Merged

wzul merged 1 commit into
mainfrom
fix/response-shape-coercion

Conversation

@wzul

@wzul wzul commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

Problem

Two TypeError shapes were left over from the v2.1.1 sweep — the same class as #18 (a decoded response value reaching a typed parameter or return), just at different call sites. Both are reachable only if the gateway (or a proxy in front of it) answers with a shape the schema does not describe, which is exactly what happened with GET /public_key/.

1. A list entry that is not an object. Every paginated list model maps results through a closure typed fn (array $x):

$list->results = array_map(
    fn (array $w) => Webhook::fromArray($w),   // one non-object entry = TypeError
    $data['results'] ?? []
);

{"results": ["x"]} → TypeError: WebhookList::{closure}(): Argument #1 ($w) must be of type array, string given. One bad entry fails the whole page, including the entries that were fine.

2. A non-object body from the account endpoints.

$json = json_encode($response);
return json_decode($json !== false ? $json : '[]', true);   // declared : array

A string body round-trips to a string → TypeError: AccountResource::balance(): Return value must be of type array, string returned.

Changes

  • lib/Model/{WebhookList,CompanyStatementList,ClientList,ClientRecurringTokenList}.php — filter results to arrays before mapping (array_filter($results, 'is_array') + array_values()); a missing/invalid results key still yields [] as before.
  • lib/Model/Billing/{BillingTemplateList,BillingTemplateClientList}.php — same item filtering, keeping their existing null for an absent/invalid results key.
  • lib/Resource/AccountResource.php — balance() and turnover() coerce a non-array decode to [].
  • tests/ResponseShapeTest.php — four cases: a mixed list (one string + one valid object, asserts the valid entry still comes through), an all-invalid list, and both account methods against a string body.
  • CHANGELOG.md — two ### Fixed entries under [Unreleased].

Verification

  • composer test — 131 tests, 381 assertions, OK.
  • composer phpstan — no errors (level 8 + bleedingEdge).
  • composer cs-check — clean.
  • The new tests bite: with lib/ reverted, all four fail with exactly
    WebhookList::{closure}(): Argument #1 ($w) must be of type array, string given,
    BillingTemplateList::{closure}(): Argument #1 ($r) must be of type array, string given,
    and AccountResource::balance()/turnover(): Return value must be of type array, string returned.

Potential Risks & Review Items

  • Silent skipping vs. failing. A malformed entry is now dropped instead of exploding the page. That is the right trade for a list endpoint (and matches what a caller can act on), but it does mean a gateway regression that returns garbage results looks like an empty/short page rather than an error. results still carries every valid entry, and next/previous are untouched.
  • Billing vs. other lists keep different "empty" semantics (null for an absent/invalid results key in the two billing models, [] in the rest). I preserved each model's existing behaviour rather than normalising them in this PR — worth a follow-up decision, but changing it here would be a silent API change.
  • array_values() re-indexes the filtered results; previously the map preserved keys from results (which is always a JSON array, so indexes were already sequential).
  • Changelog overlap with fix: wrap transport failures (DNS/connect/TLS/timeout) in a catchable NetworkException #20: that PR also edits the [Unreleased] section (adds ### Added for NetworkException). Whichever merges second will need a trivial rebase — flagging so it does not read as a surprise conflict.
  • No version bump: both entries are under [Unreleased].

Two response-shape TypeErrors left over from the 2.1.1 sweep:

- every paginated list model mapped $data['results'] through a closure
  typed fn (array $x); one non-object entry made the whole call a
  TypeError. Six models now filter to arrays before mapping (and keep
  their existing null-vs-[] semantics for a missing/invalid results key).
- AccountResource::balance()/turnover() round-tripped the response through
  json_encode/json_decode and returned it under an array type; a
  non-object body produced 'Return value must be of type array, string
  returned'. Both now coerce to [] instead.

Tests cover a mixed results list, an all-invalid list, and both account
methods against a string body; all four fail with the original TypeError
on the previous code.
@wzul
wzul requested a review from LiyanaAmil October 23, 2026 16:59
@wzul
wzul merged commit a839145 into main Oct 23, 2026
7 checks passed
@wzul
wzul deleted the fix/response-shape-coercion branch October 23, 2026 17:03
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