fix: handle non-json 200 responses

[naufalfx805-source]

Refs #94

Discord: npall_805

Summary:

• map HTTP 200 non-JSON success bodies to a typed UNAVAILABLE ApiError instead of leaking SyntaxError
• preserve abort/timeout classification during body reads
• add regression coverage for text/html 200 responses without retrying or leaking the HTML body

Validation:

• npm test -- src/lib/http.test.ts
• npm run typecheck
• npm run lint
• npm run build
• npx prettier --check src/lib/http.ts src/lib/http.test.ts
• git diff --check

Summary by CodeRabbit

• Bug Fixes
  • Improved error handling when a request succeeds but returns non-JSON content.
  • client.get() now reports a clearer API error instead of exposing a raw parsing failure.
  • Added richer error details, including HTTP status and response content type, plus guidance for checking endpoint configuration.
  • Debug logging now follows the expected request → response → error sequence for this case.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e1e89d6c-6d6e-4269-8ed1-f86eb552f0c3

📥 Commits

Reviewing files that changed from the base of the PR and between e53257d and 21160b4.

📒 Files selected for processing (2)

• src/lib/http.test.ts
• src/lib/http.ts

---

Walkthrough

HttpClient now converts JSON-parsing failures on successful (200 OK) responses into a standardized ApiError via a new nonJsonSuccessResponseError helper, providing UNAVAILABLE error code, diagnostic details, and next-action guidance. A corresponding test validates this error mapping and debug event sequencing.

Changes

Non-JSON Success Response Handling

Layer / File(s)	Summary
Non-JSON success error helper and integration src/lib/http.ts	Adds nonJsonSuccessResponseError(response, requestId) helper constructing an ApiError with UNAVAILABLE code, HTTP status, content-type details, and nextAction; wires it into requestWithMeta's response.ok branch so JSON parse failures throw this synthesized error instead of the raw parse error, emitting a matching debug error event.
Test coverage for non-JSON success responses src/lib/http.test.ts	Adds a test asserting a 200 response with non-JSON content-type yields an ApiError with UNAVAILABLE code, correct exitCode, httpStatus, details.contentType, nextAction, no raw HTML leak in the message, and a request → response → error debug event sequence.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant HttpClient
  participant Server

  Caller->>HttpClient: get()/requestWithMeta()
  HttpClient->>Server: fetch request
  Server-->>HttpClient: 200 response, content-type text/html
  HttpClient->>HttpClient: response.json() fails
  HttpClient->>HttpClient: nonJsonSuccessResponseError(response, requestId)
  HttpClient-->>Caller: throw ApiError (UNAVAILABLE)

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly reflects the main change: fixing handling of HTTP 200 responses with non-JSON bodies.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[Davidson3556]

please close this @zeshi-du , I have the issue assigned to me and I opened the issue first

[zeshi-du]

Closing in favor of #166, which landed today covering the same gap (200-with-non-JSON-body → typed error instead of a raw SyntaxError). The taxonomy call went to INTERNAL/exit 1 — a captive portal or misconfigured endpoint won't fix itself, so an exit code scripts might blind-retry on (UNAVAILABLE/10) felt wrong. Two things from your version were genuinely better and are worth follow-ups if you want them: the debug error event emission on this path (#166 doesn't emit one), and the details.contentType normalization. Thanks — you were first to file this one.