Skip to content

Commit c3382e5

Browse files
committed
fix(shell-dashboard): harden ops-api types and error reporting
- ProbeLastRun.summary is now nullable to match ProbeRun.summary; the server can complete a run with no summary (showcase-ops CR-A1.5). - ensureOk surfaces body-read failures with a 'body read failed: ...' marker instead of swallowing them silently (CR-B1.6 / CR-C15). - New parseJson helper attributes JSON parse failures to the requesting URL so call sites can tell which endpoint produced bad bytes. - triggerProbe gains a TriggerProbeOptions interface with an optional AbortSignal; passed straight through to fetch() so callers can cancel in-flight POSTs (CR-B1.5).
1 parent ca37705 commit c3382e5

2 files changed

Lines changed: 74 additions & 7 deletions

File tree

showcase/shell-dashboard/src/lib/ops-api.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,4 +258,43 @@ describe("triggerProbe", () => {
258258
}),
259259
).rejects.toThrow(/500/);
260260
});
261+
262+
it("propagates AbortSignal to fetch (CR-B1.5)", async () => {
263+
fetchSpy.mockResolvedValue(jsonResponse(triggerOk()));
264+
const ctrl = new AbortController();
265+
await triggerProbe("smoke", {
266+
token: "t",
267+
baseUrl: "http://ops.test",
268+
signal: ctrl.signal,
269+
});
270+
const init = fetchSpy.mock.calls[0]![1] as FetchInit;
271+
expect(init?.signal).toBe(ctrl.signal);
272+
});
273+
});
274+
275+
describe("ensureOk error handling (CR-B1.6)", () => {
276+
it("includes a body-read failure marker when text() throws", async () => {
277+
// Build a Response-like object whose `text()` throws and whose `ok` is
278+
// false. Response.text() does not normally throw on Response objects
279+
// with string bodies, so we hand-roll a stub.
280+
const fakeResponse = {
281+
ok: false,
282+
status: 502,
283+
statusText: "Bad Gateway",
284+
text: vi.fn(async () => {
285+
throw new Error("stream consumed");
286+
}),
287+
} as unknown as Response;
288+
fetchSpy.mockResolvedValue(fakeResponse);
289+
let caught: unknown = null;
290+
try {
291+
await fetchProbes({ baseUrl: "http://ops.test" });
292+
} catch (err) {
293+
caught = err;
294+
}
295+
expect(caught).toBeInstanceOf(Error);
296+
expect((caught as Error).message).toMatch(/body read failed/);
297+
// status should still appear so callers can match on it.
298+
expect((caught as Error).message).toMatch(/502/);
299+
});
261300
});

showcase/shell-dashboard/src/lib/ops-api.ts

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,10 @@ export interface ProbeLastRun {
5050
finishedAt: string;
5151
durationMs: number;
5252
state: "completed" | "failed";
53-
summary: ProbeRunSummary;
53+
// Nullable: a "completed" run produced by a failure path can land here
54+
// without a summary (see showcase-ops CR-A1.5). Consumers MUST null-guard
55+
// before reading summary.passed / summary.total / summary.failed.
56+
summary: ProbeRunSummary | null;
5457
}
5558

5659
export interface ProbeServiceProgress {
@@ -139,14 +142,31 @@ async function ensureOk(response: Response, url: string): Promise<void> {
139142
try {
140143
const text = await response.text();
141144
if (text && text.length <= 200) detail = ` — ${text}`;
142-
} catch {
143-
// ignore body-read failures; the status line is enough.
145+
} catch (bodyErr) {
146+
// Surface the body-read failure rather than swallowing it silently —
147+
// callers and tests need a marker to distinguish "server returned an
148+
// empty/unreachable body" from "we just chose not to include it".
149+
const msg = (bodyErr as Error)?.message ?? "unknown";
150+
detail = ` (body read failed: ${msg})`;
144151
}
145152
throw new Error(
146153
`ops-api request failed: ${response.status} ${response.statusText} at ${url}${detail}`,
147154
);
148155
}
149156

157+
/**
158+
* Parse a JSON response, wrapping any parse failure with the URL so the
159+
* caller can attribute the failure to a specific endpoint.
160+
*/
161+
async function parseJson<T>(response: Response, url: string): Promise<T> {
162+
try {
163+
return (await response.json()) as T;
164+
} catch (parseErr) {
165+
const msg = (parseErr as Error)?.message ?? "unknown";
166+
throw new Error(`ops-api JSON parse failed at ${url}: ${msg}`);
167+
}
168+
}
169+
150170
// ─────────────────────────────────────────────────────────────────────────
151171
// Public API
152172
// ─────────────────────────────────────────────────────────────────────────
@@ -162,7 +182,7 @@ export async function fetchProbes(opts: {
162182
headers: { accept: "application/json" },
163183
});
164184
await ensureOk(response, url);
165-
return (await response.json()) as ProbesResponse;
185+
return parseJson<ProbesResponse>(response, url);
166186
}
167187

168188
export async function fetchProbeDetail(
@@ -176,12 +196,19 @@ export async function fetchProbeDetail(
176196
headers: { accept: "application/json" },
177197
});
178198
await ensureOk(response, url);
179-
return (await response.json()) as ProbeDetailResponse;
199+
return parseJson<ProbeDetailResponse>(response, url);
200+
}
201+
202+
export interface TriggerProbeOptions {
203+
slugs?: string[];
204+
token: string;
205+
baseUrl?: string;
206+
signal?: AbortSignal;
180207
}
181208

182209
export async function triggerProbe(
183210
id: string,
184-
opts: { slugs?: string[]; token: string; baseUrl?: string },
211+
opts: TriggerProbeOptions,
185212
): Promise<TriggerResponse> {
186213
const url = `${resolveBaseUrl(opts.baseUrl)}/probes/${encodeURIComponent(
187214
id,
@@ -199,7 +226,8 @@ export async function triggerProbe(
199226
authorization: `Bearer ${opts.token}`,
200227
},
201228
body: JSON.stringify(body),
229+
signal: opts.signal,
202230
});
203231
await ensureOk(response, url);
204-
return (await response.json()) as TriggerResponse;
232+
return parseJson<TriggerResponse>(response, url);
205233
}

0 commit comments

Comments
 (0)