Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

---

Expand Down
38 changes: 15 additions & 23 deletions src/shared/components/OAuthModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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"]);

Expand Down Expand Up @@ -125,7 +126,7 @@ export default function OAuthModal({
}),
});

const data = await res.json();
const data = (await parseResponseBody(res)) as Record<string, unknown>;
if (!res.ok) {
const errorObject =
typeof data.error === "object" && data.error !== null
Expand Down Expand Up @@ -188,13 +189,9 @@ export default function OAuthModal({
connectionId: reauthConnection?.id,
}),
});
const data = await res.json();
const data = (await parseResponseBody(res)) as Record<string, unknown>;
if (!res.ok) {
const errMsg =
typeof data.error === "object" && data.error !== null
? ((data.error as Record<string, unknown>).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");
Expand Down Expand Up @@ -228,7 +225,7 @@ export default function OAuthModal({
}),
});

const data = await res.json();
const data = (await parseResponseBody(res)) as Record<string, unknown>;

if (data.success) {
setStep("success");
Expand Down Expand Up @@ -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<string, unknown>;
if (!res.ok) {
const errMsg =
typeof data.error === "object" && data.error !== null
? ((data.error as Record<string, unknown>).message as string) ||
JSON.stringify(data.error)
: data.error || "Request failed";
const errMsg = getErrorMessage(data, res.status, "Request failed");
throw new Error(errMsg);
}

Expand Down Expand Up @@ -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<string, unknown>;
if (!serverRes.ok)
throw new Error(
getErrorMessage(serverData, serverRes.status, "Failed to start callback server")
);

setAuthData({ ...serverData, redirectUri: serverData.redirectUri });
setStep("waiting");
Expand All @@ -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<string, unknown>;

if (pollData.success) {
setStep("success");
Expand Down Expand Up @@ -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<string, unknown>;
if (!res.ok) {
const errMsg =
typeof data.error === "object" && data.error !== null
? ((data.error as Record<string, unknown>).message as string) ||
JSON.stringify(data.error)
: data.error || "Authorization failed";
const errMsg = getErrorMessage(data, res.status, "Authorization failed");
throw new Error(errMsg);
}

Expand Down
47 changes: 47 additions & 0 deletions src/shared/utils/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<unknown> {
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<string, unknown>;
const err = rec.error;
if (typeof err === "string" && err.trim()) return err;
if (err && typeof err === "object") {
const message = (err as Record<string, unknown>).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();

Expand Down
34 changes: 34 additions & 0 deletions tests/unit/api-parse-response.test.ts
Original file line number Diff line number Diff line change
@@ -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)");
});
Loading