diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e2402be27..1b84e60eef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ _In development — bullets added per PR; finalized at release._ - **fix(translator): same-format response path no longer leaks a `data: null` SSE event** — the streaming response translator's same-format fast path returned `[chunk]` unconditionally, so the end-of-stream null/flush signal (`chunk === null`) propagated as a literal `[null]`. Downstream this surfaced as an empty `data: null` SSE event between chunks and crashed strict clients (e.g. Factory Droid BYOK on `/v1/responses`). The fast path now drops the null flush (returns `[]`) while still passing real chunks through unchanged. (thanks @thaitryhand) - **fix(translator): strip client-only assistant echo fields on the OpenAI target path (Mistral 422)** — strict OpenAI-compatible upstreams (e.g. `mistral/codestral-latest`) reject client-only assistant "echo" fields sent back as input history with `422 extra_forbidden` (the report hit `messages[].assistant.reasoning_content` via Codex `/responses`). Only `reasoning_content` was being stripped on the OpenAI target path; the sibling echo fields `reasoning`, `refusal`, `annotations` and `cache_control` leaked through and tripped the 422. They are now all dropped on the non-reasoner OpenAI target path. `audio` is deliberately preserved (OpenAI audio models reference a prior assistant audio response by id on multi-turn; Mistral never emits audio, so nothing is lost there). (thanks @xxy9468615) - **fix(cli): Tailscale login honors `TAILSCALE_AUTHKEY` for non-interactive sign-in** — `startTailscaleLogin` built `tailscale up` without ever reading `process.env.TAILSCALE_AUTHKEY`, so on a pre-authenticated / headless daemon the login waited for an interactive auth URL and timed out (~15s). When `TAILSCALE_AUTHKEY` is set it is now passed via `--auth-key=` (as a spawn argv element — no shell interpolation) so the daemon authenticates non-interactively; when unset, behavior is unchanged. (thanks @ipeterpetrus) +- **fix(dashboard): OAuth modal shows the real error on a non-JSON server response** — the OAuth connect/reauth modal called `await res.json()` unconditionally, so when a build/OAuth endpoint returned a plain-text error (e.g. a `500 Internal Server Error` page) the modal threw `Unexpected token 'I'…` and hid the real failure. Two shared helpers (`parseResponseBody` / `getErrorMessage` in `src/shared/utils/api.ts`) now read the body safely (JSON when it is JSON, raw text otherwise) and surface a clean message either way; all modal fetch sites use them. (thanks @DNNYF) --- diff --git a/src/shared/components/OAuthModal.tsx b/src/shared/components/OAuthModal.tsx index 65b01a4c48..5a498f049d 100644 --- a/src/shared/components/OAuthModal.tsx +++ b/src/shared/components/OAuthModal.tsx @@ -6,6 +6,7 @@ import Modal from "./Modal"; import Button from "./Button"; import Input from "./Input"; import { useCopyToClipboard } from "@/shared/hooks/useCopyToClipboard"; +import { parseResponseBody, getErrorMessage } from "@/shared/utils/api"; const GOOGLE_OAUTH_PROVIDERS = new Set(["antigravity", "agy", "gemini-cli"]); @@ -125,7 +126,7 @@ export default function OAuthModal({ }), }); - const data = await res.json(); + const data = (await parseResponseBody(res)) as Record; if (!res.ok) { const errorObject = typeof data.error === "object" && data.error !== null @@ -188,13 +189,9 @@ export default function OAuthModal({ connectionId: reauthConnection?.id, }), }); - const data = await res.json(); + const data = (await parseResponseBody(res)) as Record; if (!res.ok) { - const errMsg = - typeof data.error === "object" && data.error !== null - ? ((data.error as Record).message as string) || - JSON.stringify(data.error) - : data.error || "Save failed"; + const errMsg = getErrorMessage(data, res.status, "Save failed"); throw new Error(errMsg); } setStep("success"); @@ -228,7 +225,7 @@ export default function OAuthModal({ }), }); - const data = await res.json(); + const data = (await parseResponseBody(res)) as Record; if (data.success) { setStep("success"); @@ -293,13 +290,9 @@ export default function OAuthModal({ } const res = await fetch(deviceCodeUrl.toString()); - const data = await res.json(); + const data = (await parseResponseBody(res)) as Record; if (!res.ok) { - const errMsg = - typeof data.error === "object" && data.error !== null - ? ((data.error as Record).message as string) || - JSON.stringify(data.error) - : data.error || "Request failed"; + const errMsg = getErrorMessage(data, res.status, "Request failed"); throw new Error(errMsg); } @@ -339,8 +332,11 @@ export default function OAuthModal({ if (isTrueLocalhost) { try { const serverRes = await fetch(`/api/oauth/${provider}/start-callback-server`); - const serverData = await serverRes.json(); - if (!serverRes.ok) throw new Error(serverData.error); + const serverData = (await parseResponseBody(serverRes)) as Record; + if (!serverRes.ok) + throw new Error( + getErrorMessage(serverData, serverRes.status, "Failed to start callback server") + ); setAuthData({ ...serverData, redirectUri: serverData.redirectUri }); setStep("waiting"); @@ -361,7 +357,7 @@ export default function OAuthModal({ headers: { "Content-Type": "application/json" }, body: JSON.stringify({ connectionId: reauthConnection?.id }), }); - const pollData = await pollRes.json(); + const pollData = (await parseResponseBody(pollRes)) as Record; if (pollData.success) { setStep("success"); @@ -432,13 +428,9 @@ export default function OAuthModal({ const res = await fetch( `/api/oauth/${provider}/authorize?redirect_uri=${encodeURIComponent(redirectUri)}` ); - const data = await res.json(); + const data = (await parseResponseBody(res)) as Record; if (!res.ok) { - const errMsg = - typeof data.error === "object" && data.error !== null - ? ((data.error as Record).message as string) || - JSON.stringify(data.error) - : data.error || "Authorization failed"; + const errMsg = getErrorMessage(data, res.status, "Authorization failed"); throw new Error(errMsg); } diff --git a/src/shared/utils/api.ts b/src/shared/utils/api.ts index 711b3ac5e2..f15eff16b3 100644 --- a/src/shared/utils/api.ts +++ b/src/shared/utils/api.ts @@ -48,6 +48,53 @@ export async function del(url: string, options: ApiOptions = {}) { return handleResponse(response); } +/** + * Safely read a fetch `Response` body. Returns the parsed JSON when the body is + * JSON, the raw text when it is not, or `null` when empty. Never throws on a + * non-JSON body — a plain-text `500 Internal Server Error` no longer surfaces to + * the caller as `Unexpected token 'I'…`. (#1318) The body is read exactly once, + * so pass the returned value to {@link getErrorMessage} rather than re-reading + * the response. + */ +export async function parseResponseBody(response: Response): Promise { + const text = await response.text(); + if (!text) return null; + try { + return JSON.parse(text); + } catch { + return text; + } +} + +/** + * Extract a human-readable error message from an already-parsed response body + * (see {@link parseResponseBody}), regardless of whether it is a JSON object + * (`{ error }` / `{ error: { message } }` / `{ message }`) or plain text. Falls + * back to a status-qualified default. (#1318) + */ +export function getErrorMessage( + body: unknown, + status?: number, + fallback = "Request failed" +): string { + if (body && typeof body === "object") { + const rec = body as Record; + const err = rec.error; + if (typeof err === "string" && err.trim()) return err; + if (err && typeof err === "object") { + const message = (err as Record).message; + if (typeof message === "string" && message.trim()) return message; + return JSON.stringify(err); + } + const msg = rec.message ?? rec.detail; + if (typeof msg === "string" && msg.trim()) return msg; + } + if (typeof body === "string" && body.trim()) { + return body.length > 300 ? `${body.slice(0, 300)}…` : body; + } + return status != null ? `${fallback} (HTTP ${status})` : fallback; +} + async function handleResponse(response: Response) { const data = await response.json(); diff --git a/tests/unit/api-parse-response.test.ts b/tests/unit/api-parse-response.test.ts new file mode 100644 index 0000000000..924d315dae --- /dev/null +++ b/tests/unit/api-parse-response.test.ts @@ -0,0 +1,34 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +// Regression for port-from-9router#1318: the OAuth modal called `await res.json()` +// unconditionally, so a non-JSON error response (e.g. a plain-text `Internal Server +// Error` 500 from a Build-OAuth endpoint) threw `Unexpected token 'I'…` instead of +// surfacing the real failure. The shared `parseResponseBody`/`getErrorMessage` +// helpers read the body safely and produce a clean message either way. +const { parseResponseBody, getErrorMessage } = await import("../../src/shared/utils/api.ts"); + +test("#1318: parseResponseBody returns parsed JSON for a JSON body", async () => { + const res = new Response(JSON.stringify({ error: "nope" }), { + status: 400, + headers: { "Content-Type": "application/json" }, + }); + assert.deepEqual(await parseResponseBody(res), { error: "nope" }); +}); + +test("#1318: parseResponseBody returns raw text for a non-JSON body (no throw)", async () => { + const res = new Response("Internal Server Error", { status: 500 }); + assert.equal(await parseResponseBody(res), "Internal Server Error"); +}); + +test("#1318: parseResponseBody returns null for an empty body", async () => { + const res = new Response("", { status: 200 }); + assert.equal(await parseResponseBody(res), null); +}); + +test("#1318: getErrorMessage handles string-error, nested-error, plain-text and fallback", () => { + assert.equal(getErrorMessage({ error: "bad key" }), "bad key"); + assert.equal(getErrorMessage({ error: { message: "expired" } }), "expired"); + assert.equal(getErrorMessage("Internal Server Error"), "Internal Server Error"); + assert.equal(getErrorMessage(null, 500, "Save failed"), "Save failed (HTTP 500)"); +});