Report non-JSON API responses as they came

[tiagobarros01]

Problem

The GraphQL client called response.json() on every response, whatever came back:

return response.json().then(result => { ... });

When the API is behind a gateway that answers with a plain-text error — an Envoy 503 whose body is upstream connect error or disconnect/reset before headers. reset reason: connection termination — the parse blows up and the CLI prints:

╭────────────────────── Unexpected error ──────────────────────╮
│  Unexpected token 'u', "upstream c"... is not valid JSON     │
│  › JSON.parse (<anonymous>)                                  │
│  › parseJSONFromBytes (node:internal/deps/undici/undici)     │
╰──────────────────────────────────────────────────────────────╯

That message names neither the status nor the failure, and the body is discarded before anything reaches the user. V8 reveals only the first 10 characters of the input, so upstream c is all that survives — not enough to tell a connection reset from a circuit breaker or a rate limit.

This surfaced in a downstream project whose CI hit a transient gateway error during npm ci. The parse error was the only clue, and it pointed nowhere near the cause.

Change

The body is read as text and parsed explicitly. A body that is not a JSON object is reported as it came, so the gateway's own message reaches the user.

Why not check response.ok

Checking the status looks simpler but breaks the structured error path. Probing the API directly:

Request	Status	Body
Valid query	200	{"data":{...}}
Unknown field	400	{"errors":[{"extensions":{"type":"...#invalid-input","status":400,...}}]}
Inaccessible resource	200	{"errors":[{"extensions":{"type":"...#access-denied","status":403,...}}]}

A !response.ok guard would throw on the 400 before the payload is read, discarding the Problem — its type, detail and violations — and replacing Invalid input with a raw JSON dump. It would also miss the access-denied case entirely, since that arrives as 200. The status simply does not indicate whether the body carries structured errors, so the check is on the payload instead. A test pins the 400-with-payload behaviour so the regression cannot be reintroduced quietly.

Tests

test/infrastructure/graphql/fetchGraphqlClient.test.ts is new — the client had no test file. It covers the gateway error, an HTML error page, an empty body, a non-object payload, the 400-with-payload path, and the existing valid-payload and token-provider behaviour. fetchGraphqlClient.ts is at 100% coverage.

Validation

npm run validate, npm run build, npm test (621 passed, 38 suites) and npm run lint all pass.

Out of scope

Two related issues remain, both outside this change:

• The CLI exits 0 after printing this error, so a postinstall chained with && carries on and npm ci reports success with a broken tree.
• node_modules/@croct/content is cleared before fetching, so a failed run leaves the tree emptier than it found it.

Together those turn a transient gateway blip into a green install that fails much later with an unrelated-looking error. Happy to follow up on either.

🤖 Generated with Claude Code

[github-actions]

👋 @tiagobarros01
Thanks for your contribution!
The approval and merge process is almost fully automated 🧙
Here's how it works:

1. You open a new pull request
2. Automated tests check the code
3. Maintainers review the code
4. Once approved, the PR is ready to merge.

👉 Omit the extended description
Please remove the commit body before merging the pull request.
Instead, include the pull request number in the title to provide the full context
about the change.

☝️ Lastly, the title for the commit will come from the pull request title. So please provide a descriptive title that summarizes the changes in 50 characters or less using the imperative mood.
Happy coding! 🎉

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/croct@149

commit: 0c107f8