From aa8d7276cea5d9b8df81b5399fc7412c79aa022a Mon Sep 17 00:00:00 2001 From: Wan Zulkarnain Date: Fri, 23 Oct 2026 13:38:29 +0200 Subject: [PATCH 1/6] fix: return public key PEM instead of throwing on string body GET /public_key/ answers with a JSON-encoded PEM string. GuzzleClient declared stdClass|array as its return type, so decoding that body threw a TypeError and the public key could never be retrieved for callback signature verification. - widen ClientInterface/GuzzleClient/RetryClient request() to stdClass|array|string (BC-safe: implementers may keep the narrower type via covariance) - return decoded string bodies as-is; return plain-text bodies instead of discarding them; keep stdClass for empty bodies - PublicKeyResource::get() returns the bare PEM (and still accepts the legacy {public_key: ...} object shape) - BillingResource::create() return type follows the client - tests: cover JSON-encoded PEM, plain-text PEM, legacy object body, and a full fetch-then-verify round trip Fixes #18 --- CHANGELOG.md | 6 ++++ README.md | 3 ++ lib/Http/ClientInterface.php | 7 ++-- lib/Http/GuzzleClient.php | 16 +++++++-- lib/Http/RetryClient.php | 4 +-- lib/Resource/BillingResource.php | 4 +-- lib/Resource/PublicKeyResource.php | 14 +++++++- tests/ApiTest.php | 54 ++++++++++++++++++++++++++---- 8 files changed, 91 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c95e963..f4e0a77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- `$chip->publicKey->get()` no longer throws `TypeError: Chip\Http\GuzzleClient::request(): Return value must be of type stdClass|array, string returned`. `GET /public_key/` answers with a JSON-encoded PEM string, so the HTTP client now returns decoded string bodies as-is and `PublicKeyResource::get()` returns the bare PEM, making it directly usable with `ChipApi::verify()`. +- The HTTP client no longer discards non-JSON response bodies: plain-text bodies (e.g. a bare PEM) are returned as-is instead of being silently turned into an empty `stdClass`. Empty bodies still return `stdClass`. +- `ClientInterface::request()` return type widened to `\stdClass|array|string`; `BillingResource::create()` follows suit. Implementations that declare the narrower `\stdClass|array` remain valid (return type covariance). + ## [2.1.0] - 2026-09-01 ### Fixed diff --git a/README.md b/README.md index e1e79f8..42ec331 100644 --- a/README.md +++ b/README.md @@ -243,7 +243,10 @@ $statement = $chip->statements->cancel($statementId); ### Public Key ```php +// PEM public key for verifying callback signatures $publicKey = $chip->publicKey->get(); + +$isValid = ChipApi::verify($callbackBody, $signatureHeader, $publicKey); ``` ### Billing diff --git a/lib/Http/ClientInterface.php b/lib/Http/ClientInterface.php index 180be9f..b5a048a 100644 --- a/lib/Http/ClientInterface.php +++ b/lib/Http/ClientInterface.php @@ -7,9 +7,12 @@ interface ClientInterface { /** + * `GET /public_key/` answers with a JSON-encoded PEM string rather than an + * object, so a decoded string body is returned as-is. + * * @param array $options - * @return \stdClass|array + * @return \stdClass|array|string * @throws \Chip\Exception\ChipApiException */ - public function request(string $method, string $endpoint, array $options = []): \stdClass|array; + public function request(string $method, string $endpoint, array $options = []): \stdClass|array|string; } diff --git a/lib/Http/GuzzleClient.php b/lib/Http/GuzzleClient.php index c2db5d6..24c96fc 100644 --- a/lib/Http/GuzzleClient.php +++ b/lib/Http/GuzzleClient.php @@ -40,9 +40,9 @@ public function __construct( /** * @param array $options - * @return \stdClass|array + * @return \stdClass|array|string */ - public function request(string $method, string $endpoint, array $options = []): \stdClass|array + public function request(string $method, string $endpoint, array $options = []): \stdClass|array|string { $headers = []; if ($this->apiKey) { @@ -92,7 +92,17 @@ public function request(string $method, string $endpoint, array $options = []): } $body = (string) $response->getBody()->getContents(); + $decoded = json_decode($body, true); - return json_decode($body, true) ?? new \stdClass(); + if (is_array($decoded) || is_string($decoded)) { + return $decoded; + } + + // Empty or non-array JSON body (e.g. `204 No Content`), or plain text + // such as a bare PEM: return the raw body instead of discarding it so + // plain-text endpoints stay usable, and keep `{}` for empty bodies. + $raw = trim($body); + + return '' === $raw ? new \stdClass() : $raw; } } diff --git a/lib/Http/RetryClient.php b/lib/Http/RetryClient.php index 5b17440..617c753 100644 --- a/lib/Http/RetryClient.php +++ b/lib/Http/RetryClient.php @@ -23,9 +23,9 @@ public function __construct( /** * @param array $options - * @return \stdClass|array + * @return \stdClass|array|string */ - public function request(string $method, string $endpoint, array $options = []): \stdClass|array + public function request(string $method, string $endpoint, array $options = []): \stdClass|array|string { $attempt = 0; diff --git a/lib/Resource/BillingResource.php b/lib/Resource/BillingResource.php index 14e85b3..f3705be 100644 --- a/lib/Resource/BillingResource.php +++ b/lib/Resource/BillingResource.php @@ -19,9 +19,9 @@ public function __construct(private readonly ClientInterface $client) } /** - * @return array|\stdClass + * @return array|\stdClass|string */ - public function create(BillingTemplate $billing): array|\stdClass + public function create(BillingTemplate $billing): array|\stdClass|string { return $this->client->request('POST', 'billing/', [ 'json' => $billing, diff --git a/lib/Resource/PublicKeyResource.php b/lib/Resource/PublicKeyResource.php index 306eced..f579ab8 100644 --- a/lib/Resource/PublicKeyResource.php +++ b/lib/Resource/PublicKeyResource.php @@ -12,11 +12,23 @@ public function __construct(private readonly ClientInterface $client) { } + /** + * Returns the PEM public key used to verify callback payload signatures. + * + * `GET /public_key/` answers with a JSON-encoded PEM string, so the decoded + * body is the key itself. An object body (`{"public_key": "..."}`) is still + * accepted for compatibility with older gateway responses. + */ public function get(): string { $response = $this->client->request('GET', 'public_key/'); + + if (is_string($response)) { + return trim($response); + } + $arr = (array) $response; - return $arr['public_key'] ?? ''; + return (string) ($arr['public_key'] ?? ''); } } diff --git a/tests/ApiTest.php b/tests/ApiTest.php index b0d1882..d6f8520 100644 --- a/tests/ApiTest.php +++ b/tests/ApiTest.php @@ -10,6 +10,10 @@ final class ApiTest extends TestCase { + private const SIGNED_CONTENT = '{"id": "", "due": 1642060235, "type": "purchase", "client": {"cc": [], "bcc": [], "city": "", "email": "", "phone": "", "country": "", "zip_code": "", "bank_code": "", "full_name": "", "brand_name": "", "legal_name": "", "tax_number": "", "client_type": null, "bank_account": "", "personal_code": "", "shipping_city": "", "street_address": "", "shipping_country": "", "shipping_zip_code": "", "registration_number": "", "shipping_street_address": ""}, "issued": "", "status": "created", "is_test": true, "payment": null, "product": "purchases", "user_id": null, "brand_id": "", "order_id": null, "platform": "api", "purchase": {"debt": 0, "notes": "", "total": 100, "currency": "EUR", "language": "en", "products": [{"name": "test", "price": 100, "category": "", "discount": 0, "quantity": "1.0000", "tax_percent": "0.00"}], "timezone": "UTC", "due_strict": false, "email_message": "", "total_override": null, "shipping_options": [], "subtotal_override": null, "total_tax_override": null, "payment_method_details": {}, "request_client_details": [], "total_discount_override": null}, "client_id": null, "reference": "", "viewed_on": null, "company_id": "", "created_on": 1642056635, "event_type": "purchase.created", "updated_on": 1642056635, "invoice_url": null, "checkout_url": "", "send_receipt": false, "skip_capture": false, "creator_agent": "", "issuer_details": {"website": "", "brand_name": "", "legal_city": "", "legal_name": "", "tax_number": "", "bank_accounts": [{"bank_code": "", "bank_account": ""}], "legal_country": "", "legal_zip_code": "", "registration_number": "", "legal_street_address": ""}, "marked_as_paid": false, "status_history": [{"status": "created", "timestamp": 1642056635}], "cancel_redirect": "", "created_from_ip": "", "direct_post_url": null, "force_recurring": false, "recurring_token": null, "failure_redirect": "", "success_callback": "", "success_redirect": "", "transaction_data": {"flow": "payform", "extra": {}, "country": "", "attempts": [], "payment_method": ""}, "refundable_amount": 0, "is_recurring_token": false, "billing_template_id": null, "currency_conversion": null, "reference_generated": "", "refund_availability": "none", "payment_method_whitelist": null}'; + private const SIGNATURE = 'dHgVBR7qLldrgjMAM0exDnDIBsUU0ZpQC4lkPhAjmjZjkFlRoIYcaC4fR03avykxujZwakM1mGjvInFvCHE8zrrUemeJhHSHN+8n54zecQQ0U84JhdDufr0bSXvSduaqLW1cbBEOHKXm4UCVkMp3bRKzPGEYLM0L6PYd00x3yY53gDeOm05HWlXb5UG8hpKHJPhhr5S58r+hStlM0yAI7tkeTTy6neIin7WKS8imeiGGRh6n46mXEtIcwMzmOaRmQ7me3GAxvD8gDEPY6JV6r3eQZpTF7iX/rU0pod0P35XTvQ3pO2HMBCeRm5zfFCva9JGEVvtiJ1ZDZO/4/UfPEQ=='; + private const PUBLIC_KEY = "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEArzedRaG/aa191+f3/Syf\nye4lbwaVDngwBpsV/JidZ3T/27oEAPtwZ3oqhmhsBQcVB/f94ecFdj49NTG1DZZN\nfkWjSZEViL22oEGBryK2MjkUrW30kY1Yh0vCa/e0nIG/+9b1TLfzHIwjm54hw1R/\nRi/m/tf1nLMEm06ogDNV/AUyg6uyNLqp21NxKP7+xV6yfPkfX1s+qSjciyCPzO6r\n+TsG3GTqopG1FSaWx+R0+bmsOEmV5YQKMUlLKlf0wJUD7mjsNioFomEp5QBpASbE\nLfNDO13L5FiUgLtWcz+ZazCZmNUdhstLvrEVt8NhvPWBy96YWm4GfXx7xr8F11yH\npQIDAQAB\n-----END PUBLIC KEY-----"; + public function testRefundWithoutAmount(): void { $container = []; @@ -176,11 +180,11 @@ public function testDeleteRecurringToken(): void public function testVerify(): void { - $content = '{"id": "", "due": 1642060235, "type": "purchase", "client": {"cc": [], "bcc": [], "city": "", "email": "", "phone": "", "country": "", "zip_code": "", "bank_code": "", "full_name": "", "brand_name": "", "legal_name": "", "tax_number": "", "client_type": null, "bank_account": "", "personal_code": "", "shipping_city": "", "street_address": "", "shipping_country": "", "shipping_zip_code": "", "registration_number": "", "shipping_street_address": ""}, "issued": "", "status": "created", "is_test": true, "payment": null, "product": "purchases", "user_id": null, "brand_id": "", "order_id": null, "platform": "api", "purchase": {"debt": 0, "notes": "", "total": 100, "currency": "EUR", "language": "en", "products": [{"name": "test", "price": 100, "category": "", "discount": 0, "quantity": "1.0000", "tax_percent": "0.00"}], "timezone": "UTC", "due_strict": false, "email_message": "", "total_override": null, "shipping_options": [], "subtotal_override": null, "total_tax_override": null, "payment_method_details": {}, "request_client_details": [], "total_discount_override": null}, "client_id": null, "reference": "", "viewed_on": null, "company_id": "", "created_on": 1642056635, "event_type": "purchase.created", "updated_on": 1642056635, "invoice_url": null, "checkout_url": "", "send_receipt": false, "skip_capture": false, "creator_agent": "", "issuer_details": {"website": "", "brand_name": "", "legal_city": "", "legal_name": "", "tax_number": "", "bank_accounts": [{"bank_code": "", "bank_account": ""}], "legal_country": "", "legal_zip_code": "", "registration_number": "", "legal_street_address": ""}, "marked_as_paid": false, "status_history": [{"status": "created", "timestamp": 1642056635}], "cancel_redirect": "", "created_from_ip": "", "direct_post_url": null, "force_recurring": false, "recurring_token": null, "failure_redirect": "", "success_callback": "", "success_redirect": "", "transaction_data": {"flow": "payform", "extra": {}, "country": "", "attempts": [], "payment_method": ""}, "refundable_amount": 0, "is_recurring_token": false, "billing_template_id": null, "currency_conversion": null, "reference_generated": "", "refund_availability": "none", "payment_method_whitelist": null}'; - $signature = 'dHgVBR7qLldrgjMAM0exDnDIBsUU0ZpQC4lkPhAjmjZjkFlRoIYcaC4fR03avykxujZwakM1mGjvInFvCHE8zrrUemeJhHSHN+8n54zecQQ0U84JhdDufr0bSXvSduaqLW1cbBEOHKXm4UCVkMp3bRKzPGEYLM0L6PYd00x3yY53gDeOm05HWlXb5UG8hpKHJPhhr5S58r+hStlM0yAI7tkeTTy6neIin7WKS8imeiGGRh6n46mXEtIcwMzmOaRmQ7me3GAxvD8gDEPY6JV6r3eQZpTF7iX/rU0pod0P35XTvQ3pO2HMBCeRm5zfFCva9JGEVvtiJ1ZDZO/4/UfPEQ=='; - $publicKey = "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEArzedRaG/aa191+f3/Syf\nye4lbwaVDngwBpsV/JidZ3T/27oEAPtwZ3oqhmhsBQcVB/f94ecFdj49NTG1DZZN\nfkWjSZEViL22oEGBryK2MjkUrW30kY1Yh0vCa/e0nIG/+9b1TLfzHIwjm54hw1R/\nRi/m/tf1nLMEm06ogDNV/AUyg6uyNLqp21NxKP7+xV6yfPkfX1s+qSjciyCPzO6r\n+TsG3GTqopG1FSaWx+R0+bmsOEmV5YQKMUlLKlf0wJUD7mjsNioFomEp5QBpASbE\nLfNDO13L5FiUgLtWcz+ZazCZmNUdhstLvrEVt8NhvPWBy96YWm4GfXx7xr8F11yH\npQIDAQAB\n-----END PUBLIC KEY-----"; - - $this->assertTrue(\Chip\ChipApi::verify($content, $signature, $publicKey)); + $this->assertTrue(\Chip\ChipApi::verify( + self::SIGNED_CONTENT, + self::SIGNATURE, + self::PUBLIC_KEY + )); } public function testGetPurchaseMapsResponseToModel(): void @@ -867,10 +871,11 @@ public function testPartialUpdateWebhook(): void public function testGetPublicKey(): void { + // `GET /public_key/` answers with a JSON-encoded PEM string, not an object. $container = []; $history = Middleware::history($container); $api = $this->getMockApi(new MockHandler([ - new Response(200, [], $this->jsonResponse(['public_key' => 'pk_test'])), + new Response(200, [], (string) json_encode(self::PUBLIC_KEY)), ]), $history); $key = $api->publicKey->get(); @@ -878,7 +883,42 @@ public function testGetPublicKey(): void $this->assertEquals('GET', $transaction['request']->getMethod()); $this->assertStringContainsString('public_key/', $transaction['request']->getUri()->getPath()); - $this->assertEquals('pk_test', $key); + $this->assertEquals(self::PUBLIC_KEY, $key); + } + + public function testPublicKeyFromApiVerifiesCallbackSignature(): void + { + $container = []; + $history = Middleware::history($container); + $api = $this->getMockApi(new MockHandler([ + new Response(200, [], (string) json_encode(self::PUBLIC_KEY)), + ]), $history); + + $key = $api->publicKey->get(); + + $this->assertTrue(\Chip\ChipApi::verify(self::SIGNED_CONTENT, self::SIGNATURE, $key)); + } + + public function testGetPublicKeyAcceptsPlainTextPemBody(): void + { + $container = []; + $history = Middleware::history($container); + $api = $this->getMockApi(new MockHandler([ + new Response(200, [], self::PUBLIC_KEY), + ]), $history); + + $this->assertEquals(self::PUBLIC_KEY, $api->publicKey->get()); + } + + public function testGetPublicKeyAcceptsObjectBody(): void + { + $container = []; + $history = Middleware::history($container); + $api = $this->getMockApi(new MockHandler([ + new Response(200, [], $this->jsonResponse(['public_key' => 'pk_test'])), + ]), $history); + + $this->assertEquals('pk_test', $api->publicKey->get()); } public function testGetBalance(): void From 2171c46f86be6b3cf908bf673695e17859679102 Mon Sep 17 00:00:00 2001 From: Wan Zulkarnain Date: Fri, 23 Oct 2026 13:42:34 +0200 Subject: [PATCH 2/6] test: assert on the values the two dead locals were hiding PHPStan's bleeding edge flags $body (payment methods) and $result (turnover) as never read. Replace them with real assertions so static analysis passes: GET payment_methods sends no body, and turnover maps the response array. --- tests/ApiTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/ApiTest.php b/tests/ApiTest.php index d6f8520..a4cd983 100644 --- a/tests/ApiTest.php +++ b/tests/ApiTest.php @@ -57,8 +57,8 @@ public function testPaymentMethods(): void $this->assertEquals('GET', $transaction['request']->getMethod()); $this->assertStringContainsString('payment_methods/', $transaction['request']->getUri()->getPath()); - $body = json_decode($transaction['request']->getBody()->getContents(), true); $this->assertStringContainsString('currency=MYR', $transaction['request']->getUri()->getQuery()); + $this->assertEmpty($transaction['request']->getBody()->getContents()); } public function testCreatePurchase(): void @@ -949,6 +949,7 @@ public function testGetTurnover(): void $result = $api->account->turnover(['currency' => 'MYR', 'from' => 1609459200, 'to' => 1609545600]); $transaction = $container[0]; + $this->assertEquals(['incoming' => ['turnover' => 50], 'outgoing' => ['turnover' => 20]], $result); $this->assertEquals('GET', $transaction['request']->getMethod()); $this->assertStringContainsString('account/json/turnover/', $transaction['request']->getUri()->getPath()); $query = $transaction['request']->getUri()->getQuery(); From cf5829c630ceab6e2f8b145db00465181498e781 Mon Sep 17 00:00:00 2001 From: Wan Zulkarnain Date: Fri, 23 Oct 2026 14:44:14 +0200 Subject: [PATCH 3/6] docs: mark 2.1.1 release section in CHANGELOG Folds the release heading into this PR so the release needs no follow-up commit on main (main requires a reviewed PR): after merge it is a tag push only -- `git tag v2.1.1 && git push origin v2.1.1`. No version field exists in composer.json and ci.yml has no changelog check, so the tag plus this heading are the whole version bump. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4e0a77..a90c247 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [2.1.1] - 2026-10-23 + ### Fixed - `$chip->publicKey->get()` no longer throws `TypeError: Chip\Http\GuzzleClient::request(): Return value must be of type stdClass|array, string returned`. `GET /public_key/` answers with a JSON-encoded PEM string, so the HTTP client now returns decoded string bodies as-is and `PublicKeyResource::get()` returns the bare PEM, making it directly usable with `ChipApi::verify()`. From 1b1edd8f4adf9e29c89c436c257f963b8768e71a Mon Sep 17 00:00:00 2001 From: Wan Zulkarnain Date: Fri, 23 Oct 2026 17:53:55 +0200 Subject: [PATCH 4/6] fix: keep failures catchable and pagination finite on non-object responses Audited every path where a decoded response reaches a typed parameter or return; three more TypeError shapes turned up beside the public-key one: - error bodies: Detail/message as a list or object was passed straight to ChipApiException::__construct(string $message), and a JSON scalar body to its ?array $responseBody -- so a 4xx/5xx raised TypeError instead of a ChipApiException the caller can catch. Normalise both in describeError(). - pagination: iterate() passed the raw next value to request(string $endpoint); next:false (seen from some deployments) raised TypeError and next:"" looped forever. Only a non-empty string continues now, across webhooks, company_statements, billing_templates, billing clients, clients. Regression tests in tests/ResponseShapeTest.php cover each shape; all five fail with the reported TypeError (or an exhausted mock loop) on the previous code. Also folds the 2.1.1 changelog entries. --- CHANGELOG.md | 2 ++ lib/Http/GuzzleClient.php | 49 ++++++++++++++++++++++++----- lib/Resource/BillingResource.php | 10 ++++-- lib/Resource/ClientsResource.php | 5 ++- lib/Resource/StatementsResource.php | 5 ++- lib/Resource/WebhooksResource.php | 5 ++- 6 files changed, 63 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a90c247..2496660 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `$chip->publicKey->get()` no longer throws `TypeError: Chip\Http\GuzzleClient::request(): Return value must be of type stdClass|array, string returned`. `GET /public_key/` answers with a JSON-encoded PEM string, so the HTTP client now returns decoded string bodies as-is and `PublicKeyResource::get()` returns the bare PEM, making it directly usable with `ChipApi::verify()`. - The HTTP client no longer discards non-JSON response bodies: plain-text bodies (e.g. a bare PEM) are returned as-is instead of being silently turned into an empty `stdClass`. Empty bodies still return `stdClass`. - `ClientInterface::request()` return type widened to `\stdClass|array|string`; `BillingResource::create()` follows suit. Implementations that declare the narrower `\stdClass|array` remain valid (return type covariance). +- Error responses no longer throw a `TypeError` when the failure body is not a JSON object: a `detail`/`message` that is a list or object is JSON-encoded into the exception message, and a JSON scalar or plain-text body is reported as an empty `getResponseBody()`. The failure stays a `ChipApiException`, so `catch (ChipApiException $e)` keeps working instead of dying on a type error. +- Paginated `iterate()` loops only follow a real URL: `next: false` used to be a `TypeError` (`Argument #2 ($endpoint) must be of type string, false given`) and `next: ""` looped forever. Affects `webhooks`, `company_statements`, `billing_templates`, `billing_templates/{id}/clients` and `clients`. ## [2.1.0] - 2026-09-01 diff --git a/lib/Http/GuzzleClient.php b/lib/Http/GuzzleClient.php index 24c96fc..2a67da8 100644 --- a/lib/Http/GuzzleClient.php +++ b/lib/Http/GuzzleClient.php @@ -10,6 +10,7 @@ use Chip\Exception\ServerException; use Chip\Exception\ValidationException; use GuzzleHttp\Exception\ClientException as GuzzleClientException; +use GuzzleHttp\Exception\RequestException; use GuzzleHttp\Exception\ServerException as GuzzleServerException; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; @@ -61,10 +62,7 @@ public function request(string $method, string $endpoint, array $options = []): try { $response = $this->client->request($method, $endpoint, $mergedOptions); } catch (GuzzleClientException $e) { - $response = $e->getResponse(); - $statusCode = $response->getStatusCode(); - $body = json_decode((string) $response->getBody(), true) ?? []; - $message = $body['detail'] ?? $body['message'] ?? $e->getMessage(); + [$statusCode, $body, $message] = $this->describeError($e); $this->logger->error('CHIP API client error', [ 'status' => $statusCode, @@ -78,10 +76,7 @@ public function request(string $method, string $endpoint, array $options = []): default => new ClientException($message, $statusCode, $body, $e), }; } catch (GuzzleServerException $e) { - $response = $e->getResponse(); - $statusCode = $response->getStatusCode(); - $body = json_decode((string) $response->getBody(), true) ?? []; - $message = $body['detail'] ?? $body['message'] ?? $e->getMessage(); + [$statusCode, $body, $message] = $this->describeError($e); $this->logger->error('CHIP API server error', [ 'status' => $statusCode, @@ -105,4 +100,42 @@ public function request(string $method, string $endpoint, array $options = []): return '' === $raw ? new \stdClass() : $raw; } + + /** + * Normalise a failed response into `[status, body, message]`. + * + * A failure is not guaranteed to carry a JSON object: a proxy in front of the + * gateway can answer with a JSON scalar or plain text, and `detail`/`message` + * can themselves be a list or an object. Coercing here keeps the failure a + * `ChipApiException` instead of letting a typed constructor argument turn it + * into a `TypeError` the caller cannot catch as an API error. + * + * @return array{0: int, 1: array, 2: string} + */ + private function describeError(RequestException $e): array + { + $response = $e->getResponse(); + + if (null === $response) { + return [0, [], $e->getMessage()]; + } + + $statusCode = $response->getStatusCode(); + $decoded = json_decode((string) $response->getBody(), true); + $body = is_array($decoded) ? $decoded : []; + + $detail = $body['detail'] ?? $body['message'] ?? null; + $message = $e->getMessage(); + + if (is_string($detail) && '' !== $detail) { + $message = $detail; + } elseif (is_array($detail)) { + $encoded = json_encode($detail, \JSON_UNESCAPED_SLASHES | \JSON_UNESCAPED_UNICODE); + if (is_string($encoded)) { + $message = $encoded; + } + } + + return [$statusCode, $body, $message]; + } } diff --git a/lib/Resource/BillingResource.php b/lib/Resource/BillingResource.php index f3705be..fd67203 100644 --- a/lib/Resource/BillingResource.php +++ b/lib/Resource/BillingResource.php @@ -59,7 +59,10 @@ public function iterateTemplates(): \Generator yield $template; } - $url = $list->next; + // `next` is string|null per the API, but `false`/"" show up too: + // a non-string is a TypeError and an empty string loops forever. + $next = $list->next; + $url = is_string($next) && '' !== $next ? $next : null; } } @@ -124,7 +127,10 @@ public function iterateClients(string $billingId): \Generator yield $client; } - $url = $list->next; + // `next` is string|null per the API, but `false`/"" show up too: + // a non-string is a TypeError and an empty string loops forever. + $next = $list->next; + $url = is_string($next) && '' !== $next ? $next : null; } } diff --git a/lib/Resource/ClientsResource.php b/lib/Resource/ClientsResource.php index 0afac8f..86ad8b2 100644 --- a/lib/Resource/ClientsResource.php +++ b/lib/Resource/ClientsResource.php @@ -47,7 +47,10 @@ public function iterate(): \Generator yield $client; } - $url = $list->next; + // `next` is string|null per the API, but `false`/"" show up too: + // a non-string is a TypeError and an empty string loops forever. + $next = $list->next; + $url = is_string($next) && '' !== $next ? $next : null; } } diff --git a/lib/Resource/StatementsResource.php b/lib/Resource/StatementsResource.php index 65ad66b..fcdc2ed 100644 --- a/lib/Resource/StatementsResource.php +++ b/lib/Resource/StatementsResource.php @@ -49,7 +49,10 @@ public function iterate(): \Generator yield $statement; } - $url = $list->next; + // `next` is string|null per the API, but `false`/"" show up too: + // a non-string is a TypeError and an empty string loops forever. + $next = $list->next; + $url = is_string($next) && '' !== $next ? $next : null; } } diff --git a/lib/Resource/WebhooksResource.php b/lib/Resource/WebhooksResource.php index e66d50b..ecbf25a 100644 --- a/lib/Resource/WebhooksResource.php +++ b/lib/Resource/WebhooksResource.php @@ -45,7 +45,10 @@ public function iterate(): \Generator yield $webhook; } - $url = $list->next; + // `next` is string|null per the API, but `false`/"" show up too: + // a non-string is a TypeError and an empty string loops forever. + $next = $list->next; + $url = is_string($next) && '' !== $next ? $next : null; } } From 4c49948dbcbcb08c239da3942603ccc7c2fc5cfd Mon Sep 17 00:00:00 2001 From: Wan Zulkarnain Date: Fri, 23 Oct 2026 17:54:10 +0200 Subject: [PATCH 5/6] test: cover non-object response shapes (exception bodies + pagination) The previous commit referenced these tests but did not stage the new file. Eight cases: array/object detail, JSON scalar body, plain-text body, missing detail, next:false, next:"", and a normal string next page. --- tests/ResponseShapeTest.php | 121 ++++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 tests/ResponseShapeTest.php diff --git a/tests/ResponseShapeTest.php b/tests/ResponseShapeTest.php new file mode 100644 index 0000000..bfd15e1 --- /dev/null +++ b/tests/ResponseShapeTest.php @@ -0,0 +1,121 @@ + $responses + */ + private function api(array $responses): \Chip\ChipApi + { + return new \Chip\ChipApi('brand', 'key', 'https://gate.chip-in.asia/api/v1/', [ + 'handler' => HandlerStack::create(new MockHandler($responses)), + ]); + } + + public function testErrorWithArrayDetailBecomesChipApiException(): void + { + $api = $this->api([new Response(400, [], '{"detail": ["first", "second"]}')]); + + try { + $api->purchases->get('p1'); + $this->fail('Expected ClientException'); + } catch (\Chip\Exception\ClientException $e) { + $this->assertSame('["first","second"]', $e->getMessage()); + $this->assertSame(['detail' => ['first', 'second']], $e->getResponseBody()); + } + } + + public function testErrorWithObjectDetailBecomesChipApiException(): void + { + $api = $this->api([new Response(404, [], '{"detail": {"field": "missing"}}')]); + + try { + $api->purchases->get('p1'); + $this->fail('Expected NotFoundException'); + } catch (\Chip\Exception\NotFoundException $e) { + $this->assertSame('{"field":"missing"}', $e->getMessage()); + } + } + + public function testErrorWithJsonScalarBodyKeepsExceptionTypeAndEmptyBody(): void + { + $api = $this->api(array_fill(0, 4, new Response(500, [], '"internal error"'))); + + try { + $api->purchases->get('p1'); + $this->fail('Expected ServerException'); + } catch (\Chip\Exception\ServerException $e) { + $this->assertSame([], $e->getResponseBody()); + $this->assertStringContainsString('500', $e->getMessage()); + } + } + + public function testErrorWithPlainTextBodyKeepsExceptionType(): void + { + $api = $this->api(array_fill(0, 4, new Response(503, [], 'bad gateway'))); + + $this->expectException(\Chip\Exception\ServerException::class); + + $api->purchases->get('p1'); + } + + public function testErrorWithoutDetailFallsBackToTransportMessage(): void + { + $api = $this->api([new Response(400, [], '{"code": "invalid"}')]); + + try { + $api->purchases->get('p1'); + $this->fail('Expected ClientException'); + } catch (\Chip\Exception\ClientException $e) { + $this->assertStringContainsString('400', $e->getMessage()); + $this->assertSame(['code' => 'invalid'], $e->getResponseBody()); + } + } + + public function testIterateStopsWhenNextIsFalse(): void + { + $api = $this->api([new Response(200, [], '{"results": [], "next": false, "previous": null}')]); + + $this->assertSame([], iterator_to_array($api->webhooks->iterate())); + } + + public function testIterateStopsWhenNextIsEmptyString(): void + { + $api = $this->api([new Response(200, [], '{"results": [], "next": "", "previous": null}')]); + + $this->assertSame([], iterator_to_array($api->billing->iterateTemplates())); + } + + public function testIterateFollowsStringNext(): void + { + $api = $this->api([ + new Response(200, [], '{"results": [{"id": "wh_1"}], "next": "webhooks/?page=2", "previous": null}'), + new Response(200, [], '{"results": [{"id": "wh_2"}], "next": null, "previous": null}'), + ]); + + $ids = []; + foreach ($api->webhooks->iterate() as $webhook) { + $ids[] = $webhook->id; + } + + $this->assertSame(['wh_1', 'wh_2'], $ids); + } +} From 4fdb64d0ba86564b9913ebd26e913d6e54c46675 Mon Sep 17 00:00:00 2001 From: Wan Zulkarnain Date: Fri, 23 Oct 2026 18:20:32 +0200 Subject: [PATCH 6/6] test: cover the remaining iterate() sites and error-detail edge cases QA gap: the pagination fix touched five iterate() sites but only two had a regression test. Adds the missing three (company_statements, billing template clients, clients) and pins the describeError() edge cases: detail as [], detail as "", and a message list when detail is absent. An empty detail list no longer becomes the useless message "[]" -- it falls back to the transport message. --- lib/Http/GuzzleClient.php | 2 +- tests/ResponseShapeTest.php | 58 +++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/lib/Http/GuzzleClient.php b/lib/Http/GuzzleClient.php index 2a67da8..7c19bad 100644 --- a/lib/Http/GuzzleClient.php +++ b/lib/Http/GuzzleClient.php @@ -129,7 +129,7 @@ private function describeError(RequestException $e): array if (is_string($detail) && '' !== $detail) { $message = $detail; - } elseif (is_array($detail)) { + } elseif (is_array($detail) && [] !== $detail) { $encoded = json_encode($detail, \JSON_UNESCAPED_SLASHES | \JSON_UNESCAPED_UNICODE); if (is_string($encoded)) { $message = $encoded; diff --git a/tests/ResponseShapeTest.php b/tests/ResponseShapeTest.php index bfd15e1..656359b 100644 --- a/tests/ResponseShapeTest.php +++ b/tests/ResponseShapeTest.php @@ -118,4 +118,62 @@ public function testIterateFollowsStringNext(): void $this->assertSame(['wh_1', 'wh_2'], $ids); } + + public function testErrorWithEmptyArrayDetailFallsBackToTransportMessage(): void + { + $api = $this->api([new Response(400, [], '{"detail": []}')]); + + try { + $api->purchases->get('p1'); + $this->fail('Expected ClientException'); + } catch (\Chip\Exception\ClientException $e) { + $this->assertStringContainsString('400', $e->getMessage()); + $this->assertSame(['detail' => []], $e->getResponseBody()); + } + } + + public function testErrorWithEmptyStringDetailFallsBackToTransportMessage(): void + { + $api = $this->api([new Response(400, [], '{"detail": ""}')]); + + try { + $api->purchases->get('p1'); + $this->fail('Expected ClientException'); + } catch (\Chip\Exception\ClientException $e) { + $this->assertStringContainsString('400', $e->getMessage()); + } + } + + public function testErrorUsesMessageFieldWhenDetailMissing(): void + { + $api = $this->api([new Response(403, [], '{"message": ["blocked", "by", "rule"]}')]); + + try { + $api->purchases->get('p1'); + $this->fail('Expected ClientException'); + } catch (\Chip\Exception\ClientException $e) { + $this->assertSame('["blocked","by","rule"]', $e->getMessage()); + } + } + + public function testStatementsIterateStopsOnNonStringNext(): void + { + $api = $this->api([new Response(200, [], '{"results": [], "next": false, "previous": null}')]); + + $this->assertSame([], iterator_to_array($api->statements->iterate())); + } + + public function testBillingClientsIterateStopsOnEmptyNext(): void + { + $api = $this->api([new Response(200, [], '{"results": [], "next": "", "previous": null}')]); + + $this->assertSame([], iterator_to_array($api->billing->iterateClients('bt_1'))); + } + + public function testClientsIterateStopsOnNonStringNext(): void + { + $api = $this->api([new Response(200, [], '{"results": [], "next": false, "previous": null}')]); + + $this->assertSame([], iterator_to_array($api->clients->iterate())); + } }