diff --git a/apps/web/src/components/thread/github/sandbox-git-api.test.ts b/apps/web/src/components/thread/github/sandbox-git-api.test.ts index 721e67c730..803e21bb99 100644 --- a/apps/web/src/components/thread/github/sandbox-git-api.test.ts +++ b/apps/web/src/components/thread/github/sandbox-git-api.test.ts @@ -12,8 +12,10 @@ import { isDecoOnlyPaths, needsSmartReviewJudgment, normalizePublishPolicy, + parseJson, resolvePathGate, reviewDiffSignature, + SandboxGitError, sandboxGitStatusQueryKey, sandboxGitStatusQueryOptions, shouldUseBaseDiff, @@ -655,3 +657,31 @@ describe("hasNothingToReview", () => { expect(hasNothingToReview({ ...cleanStatus, unpushed: 1 })).toBe(false); }); }); + +describe("parseJson", () => { + test("returns the parsed body on a 2xx JSON response", async () => { + const res = new Response(JSON.stringify({ ok: true }), { status: 200 }); + expect(await parseJson<{ ok: boolean }>(res)).toEqual({ ok: true }); + }); + + test("throws a SandboxGitError with the response's error field on non-2xx JSON", async () => { + const res = new Response(JSON.stringify({ error: "no runner" }), { + status: 503, + }); + await expect(parseJson(res)).rejects.toMatchObject( + new SandboxGitError("no runner", 503), + ); + }); + + test("degrades a non-JSON error page instead of throwing a raw SyntaxError", async () => { + const page = () => + new Response("502 Bad Gateway", { status: 502 }); + await expect(parseJson(page())).rejects.toBeInstanceOf(SandboxGitError); + await expect(parseJson(page())).rejects.toMatchObject({ status: 502 }); + }); + + test("degrades a non-JSON 2xx body instead of throwing a raw SyntaxError", async () => { + const res = new Response("not json", { status: 200 }); + await expect(parseJson(res)).rejects.toBeInstanceOf(SandboxGitError); + }); +}); diff --git a/apps/web/src/components/thread/github/sandbox-git-api.ts b/apps/web/src/components/thread/github/sandbox-git-api.ts index 1de4f5471c..17db969db3 100644 --- a/apps/web/src/components/thread/github/sandbox-git-api.ts +++ b/apps/web/src/components/thread/github/sandbox-git-api.ts @@ -71,7 +71,7 @@ function buildSandboxGitUrl( /** Error carrying the HTTP status so callers can back off on unreachable * sandboxes (503 no runner, 410 handle gone) instead of polling forever. */ -class SandboxGitError extends Error { +export class SandboxGitError extends Error { constructor( message: string, readonly status: number, @@ -103,11 +103,24 @@ function retryGitRequest(failureCount: number, error: unknown): boolean { return failureCount < 1; } -async function parseJson(res: Response): Promise { - const body = (await res.json()) as T & { error?: string }; - if (!res.ok) { +/** + * A gateway/proxy hiccup between the browser and the sandbox daemon (a 502/504 + * error page, an empty body on a dropped connection) is not guaranteed to be + * JSON even on a non-2xx response. `res.json()` throwing a raw `SyntaxError` + * on that would surface as an opaque "Unexpected token" instead of a + * `SandboxGitError` callers can branch on (e.g. `isSandboxUnreachable`). + */ +export async function parseJson(res: Response): Promise { + const text = await res.text(); + let body: (T & { error?: string }) | undefined; + try { + body = text ? (JSON.parse(text) as T & { error?: string }) : undefined; + } catch { + body = undefined; + } + if (!res.ok || body === undefined) { throw new SandboxGitError( - typeof body.error === "string" + typeof body?.error === "string" ? body.error : `Request failed (${res.status})`, res.status,