fix(api): check HTTP status before parsing the response body by Tomauskasz · Pull Request #5 · monid-ai/cli

Tomauskasz · 2026-09-07T12:38:00Z

Problem

MonidAPI.request() parses every response as JSON before it checks res.status:

const data = await res.json() as T & ApiErrorResponse;   // throws here

if (!res.ok) {
  const message = data?.error?.message ?? data?.message ?? `HTTP ${res.status}`;
  const code = data?.error?.code ?? statusToCode(res.status);
  throw new MonidError(code, message, res.status);        // never reached
}

Not every response arriving on that socket comes from the API. A gateway, proxy or load balancer in front of it answers 5xx with an HTML error page. When that happens the parse throws and the status is gone:

$ monid inspect -p exa -e /search
monid: error: Unexpected token '<', "<html>\r\n<h"... is not valid JSON

Everything needed to report this well was already present and unreachable — the HTTP ${res.status} fallback on the line below, statusToCode(), and friendlyMessage(), which renders any 5xx as "Something went wrong. Please try again later." The CLI had the status code in hand and discarded it in favour of a parse error.

The practical cost is misdiagnosis. A transient 502 is indistinguishable from a broken CLI, a bad endpoint or an expired key, so the reasonable next move is to go hunting for another provider or start debugging the tool — when the correct move was to retry. I hit this against api.monid.ai; the same call succeeded on every attempt minutes later.

Worth noting fetchLatestVersion() in src/utils/update-check.ts already gets the ordering right — if (!res.ok) return null; before res.json(). This brings the API client in line with the convention already in the codebase.

Fix

Read the body with res.text(), attempt JSON.parse defensively, and check !res.ok first so the status drives the error. A structured API error body is still preferred over the bare status when one parses, so no existing error message changes.
A 2xx that is not JSON is still a broken response, so it raises INVALID_RESPONSE naming the declared content type and a whitespace-collapsed 80-char snippet — enough to tell a captive portal from a bad deploy without dumping a document into the terminal.

Behaviour on every well-formed response is unchanged.

Tests

New test/api/client.test.ts, driving request() through whoami() with a stubbed fetch:

Case	Expected
502 + `text/html` gateway page	`MonidError`, `HTTP_502`, status `502`
401 + `text/html`	code stays `AUTH_FAILED`
402 + structured JSON error	still prefers `Balance too low` over the status
200 + `text/html`	`INVALID_RESPONSE`, names `text/html`
200 + empty body	`INVALID_RESPONSE`, describes it as empty
200 + 5000-char body	snippet truncated, message under 200 chars
200 + valid JSON	payload returned unchanged

Five of the seven fail on main with the original parse error; the two that pass there are the no-regression cases.

# main + new tests
   2 pass, 5 fail

# this branch
   69 pass, 0 fail   (bun test, full suite)
   tsc --noEmit clean

Relation to #4

Independent — branched from main, touches different files. #4 fixes a crash in the inspect renderer; this fixes how any non-JSON response is reported. They can merge in either order.

Summary by CodeRabbit

Bug Fixes
- Improved handling of non-JSON server responses while preserving HTTP status information.
- Provides clearer error details for failed requests, including invalid or empty successful responses.
- Prevents excessively long response content from overwhelming error messages.
- Continues to correctly parse valid JSON responses and prioritize structured API errors.

`request()` parsed every response as JSON before it looked at `res.status`. Not every response on that socket comes from the API — a gateway, proxy or load balancer in front of it answers 5xx with an HTML error page — so the parse threw first and the status was lost: monid inspect -p exa -e /search monid: error: Unexpected token '<', "<html>\r\n<h"... is not valid JSON The `!res.ok` branch below it already built a good message, and `friendlyMessage()` already renders 5xx as "Something went wrong. Please try again later." Neither could run. The status code was in hand and discarded, so a transient upstream blip was indistinguishable from a broken CLI or a bad endpoint — the reasonable next move is to go hunting for another provider rather than simply retrying. Read the body as text, attempt the parse defensively, and check the status first so the error is driven by it. A 2xx that is not JSON is still broken, so it now raises INVALID_RESPONSE naming the content type and a truncated snippet, which separates a captive portal from a bad deploy. `fetchLatestVersion()` in utils/update-check.ts already checks `res.ok` before parsing; this brings the API client in line with it. Signed-off-by: Tomas <180413002+Tomauskasz@users.noreply.github.com>

coderabbitai · 2026-09-07T12:39:06Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: e2c1def8-5dc2-48c6-822f-e24d6d4e804a

📥 Commits

Reviewing files that changed from the base of the PR and between 3b64605 and fae5073.

📒 Files selected for processing (2)

src/api/client.ts
test/api/client.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

Walkthrough

MonidAPI.request now parses response text safely, preserves HTTP error handling for non-JSON responses, and reports invalid successful responses with content-type and body details. Tests cover error mapping, diagnostics, truncation, empty bodies, and valid JSON responses.

Changes

API response handling

Layer / File(s)	Summary
Response parsing and error mapping `src/api/client.ts`	Responses are read as text and parsed defensively. Non-2xx responses retain status-based and structured error handling. Invalid 2xx responses use `INVALID_RESPONSE` with content-type and truncated body details.
Response handling tests `test/api/client.test.ts`	Tests cover gateway and authentication errors, structured API errors, invalid and empty successful responses, body truncation, and valid JSON parsing.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to fae50

API responses now retain useful HTTP error handling when intermediaries return non-JSON bodies, while malformed successful responses produce clear invalid-response errors. The covered behavior is ready to merge.

Suggested reviewers: feiyoug

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: checking HTTP status before parsing response bodies in MonidAPI.request().
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.

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

Tomauskasz requested review from a team and ooctoo777 September 7, 2026 12:38

Navigation Menu

fix(api): check HTTP status before parsing the response body - #5

fix(api): check HTTP status before parsing the response body#5
Tomauskasz wants to merge 1 commit into
monid-ai:mainfrom
Tomauskasz:fix/non-json-error-response

Tomauskasz commented Sep 7, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 7, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Tomauskasz commented Sep 7, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Tests

Relation to #4

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Tomauskasz commented Sep 7, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 7, 2026 •

edited

Loading