mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
fix(dashboard): OAuth modal surfaces the real error on a non-JSON response (#4351)
* fix(dashboard): OAuth modal surfaces real error on non-JSON responses (port from 9router#1318) The OAuth connect/reauth modal called `await res.json()` unconditionally, so a non-JSON error response (e.g. a plain-text 500 page from a build/OAuth endpoint) threw `Unexpected token 'I'...` and hid the real failure. New shared helpers parseResponseBody / getErrorMessage (src/shared/utils/api.ts) read the body safely (JSON when JSON, raw text otherwise) and produce a clean message either way; every modal fetch site now uses them. Reported-by: DNNYF (https://github.com/decolua/9router/issues/1318) Co-authored-by: DNNYF <74033321+DNNYF@users.noreply.github.com> * fix(dashboard): type OAuth modal response body as Record<string, unknown> (t11 any-budget) Switch the parseResponseBody casts from Record<string, any> to Record<string, unknown> so OAuthModal.tsx stays within its t11 explicit-any budget. getErrorMessage already takes unknown; the success paths typecheck clean under strict:false. No runtime change. --------- Co-authored-by: DNNYF <74033321+DNNYF@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
0773aa81c3
commit
838fddacee
@@ -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)
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
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<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");
|
||||
@@ -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");
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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");
|
||||
@@ -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");
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
|
||||
34
tests/unit/api-parse-response.test.ts
Normal file
34
tests/unit/api-parse-response.test.ts
Normal file
@@ -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)");
|
||||
});
|
||||
Reference in New Issue
Block a user