fix(walkthrough): name an outdated server instead of failing to parse its HTML
A server without these routes does not answer 404 with JSON. The unmatched /api path reaches the OpenCode proxy, and OpenCode serves its embedded web UI for anything it does not recognise — HTML, status 200 — so a client newer than its server parsed a web page as JSON and put "Unexpected token '<', "<!doctype" in the panel, naming neither the cause nor the remedy. The client now checks the content type before parsing. A non-JSON answer on 2xx or 404 blocks with "this server is older than the app, update it and refresh". A non-JSON 5xx keeps its own failure: a server that answered badly is not a server missing the feature, and sending that user to upgrade chases the wrong thing.
This commit is contained in:
Bohdan Triapitsyn
@@ -0,0 +1,88 @@
|
||||
import { beforeEach, describe, expect, mock, test } from 'bun:test';
|
||||
|
||||
// A server older than this client does not answer 404-with-JSON: unmatched
|
||||
// `/api/*` reaches the OpenCode proxy, and OpenCode serves its embedded web UI
|
||||
// for any unknown path — HTML, status 200. These tests pin that the panel gets
|
||||
// an actionable code instead of a JSON parser error.
|
||||
|
||||
let nextResponse: Response = new Response('{}', { headers: { 'Content-Type': 'application/json' } });
|
||||
|
||||
mock.module('@/lib/runtime-fetch', () => ({
|
||||
runtimeFetch: mock(async () => nextResponse),
|
||||
}));
|
||||
|
||||
const { fetchWalkthrough, generateWalkthrough } = await import('./api');
|
||||
const { WalkthroughError } = await import('./types');
|
||||
import type { WalkthroughSource } from './types';
|
||||
|
||||
const SOURCE: WalkthroughSource = { kind: 'working-tree', scope: 'all' };
|
||||
|
||||
const html = (status: number) =>
|
||||
new Response('<!doctype html><html><body>OpenCode</body></html>', {
|
||||
status,
|
||||
headers: { 'Content-Type': 'text/html; charset=utf-8' },
|
||||
});
|
||||
|
||||
describe('walkthrough api', () => {
|
||||
beforeEach(() => {
|
||||
nextResponse = new Response('{}', { headers: { 'Content-Type': 'application/json' } });
|
||||
});
|
||||
|
||||
test('reads a JSON answer', async () => {
|
||||
nextResponse = new Response(JSON.stringify({ hunkCount: 3 }), {
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
|
||||
const result = await fetchWalkthrough('/repo', SOURCE);
|
||||
|
||||
expect(result.hunkCount).toBe(3);
|
||||
});
|
||||
|
||||
test('reports HTML served with 200 as a server without the routes', async () => {
|
||||
nextResponse = html(200);
|
||||
|
||||
const error = await fetchWalkthrough('/repo', SOURCE).catch((caught: unknown) => caught);
|
||||
|
||||
expect(error).toBeInstanceOf(WalkthroughError);
|
||||
expect((error as InstanceType<typeof WalkthroughError>).code).toBe('server-unsupported');
|
||||
expect((error as Error).message).not.toContain('JSON');
|
||||
});
|
||||
|
||||
test('reports a non-JSON 404 the same way', async () => {
|
||||
nextResponse = html(404);
|
||||
|
||||
const error = await generateWalkthrough('/repo', SOURCE).catch((caught: unknown) => caught);
|
||||
|
||||
expect((error as InstanceType<typeof WalkthroughError>).code).toBe('server-unsupported');
|
||||
});
|
||||
|
||||
test('keeps a server-side failure rather than blaming the server version', async () => {
|
||||
nextResponse = new Response(JSON.stringify({ error: 'model exploded', code: 'output-exhausted' }), {
|
||||
status: 500,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
|
||||
const error = await generateWalkthrough('/repo', SOURCE).catch((caught: unknown) => caught);
|
||||
|
||||
expect((error as InstanceType<typeof WalkthroughError>).code).toBe('output-exhausted');
|
||||
expect((error as Error).message).toBe('model exploded');
|
||||
});
|
||||
|
||||
test('a 5xx that is not JSON is a broken server, not a missing route', async () => {
|
||||
nextResponse = html(502);
|
||||
|
||||
const error = await fetchWalkthrough('/repo', SOURCE).catch((caught: unknown) => caught);
|
||||
|
||||
expect((error as InstanceType<typeof WalkthroughError>).code).toBe(undefined);
|
||||
expect((error as Error).message).toBe('Failed to load walkthrough');
|
||||
});
|
||||
|
||||
test('JSON that does not parse is reported without the parser wording', async () => {
|
||||
nextResponse = new Response('{"walkthrough":', { headers: { 'Content-Type': 'application/json' } });
|
||||
|
||||
const error = await fetchWalkthrough('/repo', SOURCE).catch((caught: unknown) => caught);
|
||||
|
||||
expect(error).toBeInstanceOf(WalkthroughError);
|
||||
expect((error as Error).message).toBe('The server returned a malformed walkthrough response');
|
||||
});
|
||||
});
|
||||
@@ -16,9 +16,30 @@ interface ErrorPayload {
|
||||
availableChars?: unknown;
|
||||
}
|
||||
|
||||
const isJsonResponse = (response: Response): boolean =>
|
||||
/^application\/(?:[\w.+-]+\+)?json\b/i.test(response.headers.get('content-type') ?? '');
|
||||
|
||||
/**
|
||||
* A server without these routes does not answer 404 with JSON. Unmatched
|
||||
* `/api/*` falls through to the OpenCode proxy, and OpenCode serves its embedded
|
||||
* web UI for any path it does not know — HTML, status 200. Parsing that as JSON
|
||||
* surfaced `Unexpected token '<', "<!doctype "...` in the panel, which names
|
||||
* neither the cause nor the remedy.
|
||||
*
|
||||
* Only a missing route is reported this way: 2xx and 404 are the shapes it
|
||||
* produces. A 5xx that is not JSON came from a server that did answer, so it
|
||||
* keeps its own failure rather than becoming advice to upgrade.
|
||||
*/
|
||||
const serverUnsupported = () =>
|
||||
new WalkthroughError('This OpenChamber server has no walkthrough API', { code: 'server-unsupported' });
|
||||
|
||||
const looksUnsupported = (response: Response): boolean =>
|
||||
!isJsonResponse(response) && (response.ok || response.status === 404);
|
||||
|
||||
// An authoritative read that fails must never look like "there is nothing
|
||||
// here" — the caller would clear a perfectly good walkthrough off the screen.
|
||||
const throwFromResponse = async (response: Response, fallback: string): Promise<never> => {
|
||||
if (looksUnsupported(response)) throw serverUnsupported();
|
||||
const payload = (await response.json().catch(() => null)) as ErrorPayload | null;
|
||||
throw new WalkthroughError(typeof payload?.error === 'string' ? payload.error : fallback, {
|
||||
code: typeof payload?.code === 'string' ? (payload.code as WalkthroughError['code']) : undefined,
|
||||
@@ -28,6 +49,17 @@ const throwFromResponse = async (response: Response, fallback: string): Promise<
|
||||
});
|
||||
};
|
||||
|
||||
const readJson = async <T>(response: Response): Promise<T> => {
|
||||
if (!isJsonResponse(response)) throw serverUnsupported();
|
||||
try {
|
||||
return (await response.json()) as T;
|
||||
} catch {
|
||||
// Declared JSON, arrived truncated or empty: still not an answer, and the
|
||||
// parser's own message says nothing a reader can act on.
|
||||
throw new WalkthroughError('The server returned a malformed walkthrough response');
|
||||
}
|
||||
};
|
||||
|
||||
export async function fetchWalkthrough(
|
||||
directory: string,
|
||||
source: WalkthroughSource,
|
||||
@@ -45,7 +77,7 @@ export async function fetchWalkthrough(
|
||||
if (!response.ok) {
|
||||
return throwFromResponse(response, 'Failed to load walkthrough');
|
||||
}
|
||||
return response.json();
|
||||
return readJson<WalkthroughResult>(response);
|
||||
}
|
||||
|
||||
export async function generateWalkthrough(
|
||||
@@ -68,7 +100,7 @@ export async function generateWalkthrough(
|
||||
if (!response.ok) {
|
||||
return throwFromResponse(response, 'Failed to generate walkthrough');
|
||||
}
|
||||
return response.json();
|
||||
return readJson<WalkthroughResult>(response);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -89,6 +89,7 @@ export interface WalkthroughResult {
|
||||
*/
|
||||
export type WalkthroughStage = 'collecting' | 'asking' | 'retrying' | 'assembling';
|
||||
|
||||
/** Reasons the server reports for refusing to generate. */
|
||||
export type WalkthroughBlockedReason =
|
||||
| 'no-model'
|
||||
| 'no-provider-login'
|
||||
@@ -98,6 +99,13 @@ export type WalkthroughBlockedReason =
|
||||
| 'structured-output-unsupported'
|
||||
| 'output-exhausted';
|
||||
|
||||
/**
|
||||
* Everything the panel can render as a blocking screen. `server-unsupported` is
|
||||
* never sent by a server — it is what the client concludes when the answer is
|
||||
* not JSON at all, which is how a server too old to have these routes replies.
|
||||
*/
|
||||
export type WalkthroughBlockedState = WalkthroughBlockedReason | 'server-unsupported';
|
||||
|
||||
export interface WalkthroughReadiness {
|
||||
ready: boolean;
|
||||
reason?: WalkthroughBlockedReason;
|
||||
@@ -116,7 +124,7 @@ export interface WalkthroughReadiness {
|
||||
}
|
||||
|
||||
export class WalkthroughError extends Error {
|
||||
readonly code?: WalkthroughBlockedReason | 'invalid-walkthrough' | 'github-not-connected' | 'no-github-remote';
|
||||
readonly code?: WalkthroughBlockedState | 'invalid-walkthrough' | 'github-not-connected' | 'no-github-remote';
|
||||
readonly model?: WalkthroughModel;
|
||||
readonly requiredChars?: number;
|
||||
readonly availableChars?: number;
|
||||
|
||||
Reference in New Issue
Block a user