fix: keep page errors local and preserve Workspace error details

This commit is contained in:
2026-10-14 21:19:42 +09:00
parent 63d581914c
commit 65ead5fffd
13 changed files with 459 additions and 62 deletions
@@ -72,6 +72,49 @@ async function queryRequests(base: string) {
return requests.filter((row: { path: string }) => row.path === "/tickets/query"); return requests.filter((row: { path: string }) => row.path === "/tickets/query");
} }
Deno.test("Ticket parser failures stay local, retain Workspace navigation, and recover on retry", async () => {
await withFixture(async (browser, base) => {
const page = await browser.newPage();
const wire = await detail(base, "T-1");
delete wire.initial_post;
let invalid = true;
await page.route("**/api/w/tickets/tickets/T-1", async (route) => {
if (invalid) await route.fulfill({ status: 200, json: wire });
else await route.continue();
});
await page.goto(`${base}/w/tickets/tickets/T-1`);
await page.getByRole("alert").filter({ hasText: "Ticket detail.initial_post must be an object" }).waitFor();
assertEquals(await page.getByText("The selected Workspace cannot be opened", { exact: true }).count(), 0);
assertEquals(await page.getByText("Workspace unavailable", { exact: true }).count(), 0);
assert(await page.getByRole("navigation", { name: "Current workspace location" }).isVisible());
invalid = false;
await page.getByRole("button", { name: "Retry", exact: true }).click();
await page.locator(".ticket-detail-header").waitFor();
assertEquals(await page.getByRole("alert").filter({ hasText: "initial_post must be an object" }).count(), 0);
await page.close();
});
});
Deno.test("Workspace HTTP failure retains its real status and recovers without recreating the Workspace", async () => {
await withFixture(async (browser, base) => {
const page = await browser.newPage();
let unavailable = true;
await page.route("**/api/w/tickets/workspace", async (route) => {
if (unavailable) await route.fulfill({ status: 503, body: "private server failure" });
else await route.continue();
});
await page.goto(`${base}/w/tickets/tickets/T-1`);
await page.getByRole("heading", { name: "Workspace details could not be loaded" }).waitFor();
assert(await page.locator(".workspace-page [role=alert]").filter({ hasText: "HTTP 503" }).isVisible());
assertEquals(await page.getByText("The selected Workspace cannot be opened", { exact: true }).count(), 0);
assertEquals(await page.getByText("private server failure", { exact: true }).count(), 0);
unavailable = false;
await page.getByRole("button", { name: "Retry", exact: true }).click();
await page.locator(".ticket-detail-header").waitFor();
await page.close();
});
});
Deno.test("single Ticket query searches beyond loaded rows and preserves qualifiers on pagination", async () => { Deno.test("single Ticket query searches beyond loaded rows and preserves qualifiers on pagination", async () => {
await withFixture(async (browser, base) => { await withFixture(async (browser, base) => {
const page = await browser.newPage(); const page = await browser.newPage();
@@ -12,6 +12,8 @@ import {
export type ApiResult<T> = { export type ApiResult<T> = {
data: T | null; data: T | null;
error: string | null; error: string | null;
/** HTTP failure status, when a response was received; absent for local/transport failures. */
status?: number;
}; };
export type { SkillCatalogResponse, SkillDetailResponse }; export type { SkillCatalogResponse, SkillDetailResponse };
@@ -19,6 +21,8 @@ export type { SkillCatalogResponse, SkillDetailResponse };
export type JsonLoadPolicy = { export type JsonLoadPolicy = {
diagnosticLabel: string; diagnosticLabel: string;
maxResponseBytes: number; maxResponseBytes: number;
/** Only explicitly trusted validator errors may expose bounded field diagnostics. */
describeValidationError?: (error: unknown) => string | undefined;
}; };
const SKILL_API_LOAD_POLICY: JsonLoadPolicy = { const SKILL_API_LOAD_POLICY: JsonLoadPolicy = {
@@ -96,6 +100,7 @@ export async function loadJson<T>(
parse: (value: unknown) => T = (value) => value as T, parse: (value: unknown) => T = (value) => value as T,
policy?: JsonLoadPolicy, policy?: JsonLoadPolicy,
): Promise<ApiResult<T>> { ): Promise<ApiResult<T>> {
let stage: "request" | "decode" | "validate" = "request";
try { try {
const response = await fetchFn(path, init); const response = await fetchFn(path, init);
if (!response.ok) { if (!response.ok) {
@@ -103,6 +108,7 @@ export async function loadJson<T>(
await response.body?.cancel(); await response.body?.cancel();
return { return {
data: null, data: null,
status: response.status,
error: error:
`${policy.diagnosticLabel} request failed with HTTP ${response.status}`, `${policy.diagnosticLabel} request failed with HTTP ${response.status}`,
}; };
@@ -110,19 +116,31 @@ export async function loadJson<T>(
const text = await response.text(); const text = await response.text();
return { return {
data: null, data: null,
status: response.status,
error: text || `${path} request failed (${response.status})`, error: text || `${path} request failed (${response.status})`,
}; };
} }
stage = "decode";
const payload: unknown = policy const payload: unknown = policy
? await readBoundedJson(response, policy.maxResponseBytes) ? await readBoundedJson(response, policy.maxResponseBytes)
: await response.json(); : await response.json();
stage = "validate";
return { data: parse(payload), error: null }; return { data: parse(payload), error: null };
} catch (error) { } catch (error) {
if (policy) { if (policy) {
const validationDiagnostic = stage === "validate"
? policy.describeValidationError?.(error)
: undefined;
const diagnostic = error instanceof SkillApiContractError const diagnostic = error instanceof SkillApiContractError
? error.message ? error.message
: validationDiagnostic !== undefined
? `${policy.diagnosticLabel}: ${validationDiagnostic}`
: error instanceof ResponseByteLimitError : error instanceof ResponseByteLimitError
? `${policy.diagnosticLabel} response exceeds its byte limit` ? `${policy.diagnosticLabel} response exceeds its byte limit`
: stage === "request"
? `${policy.diagnosticLabel} request could not be completed`
: stage === "decode"
? `${policy.diagnosticLabel} response could not be decoded as JSON`
: `${policy.diagnosticLabel} response is invalid`; : `${policy.diagnosticLabel} response is invalid`;
return { data: null, error: diagnostic.slice(0, 256) }; return { data: null, error: diagnostic.slice(0, 256) };
} }
@@ -2,8 +2,10 @@ declare const Deno: {
test(name: string, fn: () => Promise<void> | void): void; test(name: string, fn: () => Promise<void> | void): void;
}; };
import { loadJson } from "./http.ts";
import { fixturePost } from "../home/dashboard.test-fixtures.ts"; import { fixturePost } from "../home/dashboard.test-fixtures.ts";
import { import {
TICKET_BROWSER_API_LOAD_POLICY,
parseTicketEvent, parseTicketEvent,
parseMergeRequestDetailResponse, parseMergeRequestDetailResponse,
parseMergeRequestListResponse, parseMergeRequestListResponse,
@@ -64,12 +66,37 @@ Deno.test("Ticket post parser rejects absent safety fields and unknown wire fiel
} }
assertThrows(() => parseTicketEvent({ ...fixturePost("Post"), editable: "yes" }), "editable"); assertThrows(() => parseTicketEvent({ ...fixturePost("Post"), editable: "yes" }), "editable");
assertThrows(() => parseTicketEvent({ ...fixturePost("Post"), edited_by: 7 }), "edited_by"); assertThrows(() => parseTicketEvent({ ...fixturePost("Post"), edited_by: 7 }), "edited_by");
assertThrows(() => parseTicketEvent({ ...fixturePost("Post"), surprise: true }), "unknown field surprise"); assertThrows(() => parseTicketEvent({ ...fixturePost("Post"), surprise: true }), "unknown field");
const detail: Record<string, unknown> = { ...ticketDetailFixture() }; const detail: Record<string, unknown> = { ...ticketDetailFixture() };
delete detail.initial_post; delete detail.initial_post;
assertThrows(() => parseTicketDetail(detail), "initial_post"); assertThrows(() => parseTicketDetail(detail), "initial_post");
}); });
Deno.test("Ticket load diagnostics retain the failed field without leaking response contents", async () => {
const secret = "PRIVATE_RESPONSE_TEXT";
const wire: Record<string, unknown> = { ...ticketDetailFixture(), body: secret };
delete wire.initial_post;
const load = (payload: unknown) => loadJson(
(async () => Response.json(payload)) as typeof fetch,
"/api/w/workspace-a/tickets/T-1",
undefined,
parseTicketDetail,
TICKET_BROWSER_API_LOAD_POLICY,
);
const missing = await load(wire);
assertEquals(missing.data, null);
assertEquals(missing.error, "Ticket Browser API: Ticket detail.initial_post must be an object");
assertEquals(missing.status, undefined);
const unknown = await load({ ...ticketDetailFixture(), [secret]: true });
assertEquals(unknown.error, "Ticket Browser API: Ticket detail contains unknown field");
const nested = ticketDetailFixture();
const malformed = await load({ ...nested, initial_post: { ...nested.initial_post, attributes: { [secret]: 5 } } });
assertEquals(malformed.error, "Ticket Browser API: Ticket detail.initial_post.attributes value must be a string");
for (const result of [missing, unknown, malformed]) {
assertEquals(result.error?.includes(secret), false);
}
});
Deno.test("Ticket Browser parser accepts the generated Ticket list shape", () => { Deno.test("Ticket Browser parser accepts the generated Ticket list shape", () => {
const parsed = parseTicketListResponse({ const parsed = parseTicketListResponse({
workspace_id: "workspace-a", workspace_id: "workspace-a",
@@ -105,7 +132,7 @@ Deno.test("Ticket Browser parser rejects unknown fields and out-of-range limits"
}; };
assertThrows( assertThrows(
() => parseTicketListResponse({ ...fixture, stale: true }), () => parseTicketListResponse({ ...fixture, stale: true }),
"unknown field stale", "unknown field",
); );
assertThrows( assertThrows(
() => parseTicketListResponse({ ...fixture, limit: 1001 }), () => parseTicketListResponse({ ...fixture, limit: 1001 }),
@@ -296,7 +323,7 @@ Deno.test("Ticket Browser parser accepts target collections and rejects singular
repository_key: "main", repository_key: "main",
ref_selector: "develop", ref_selector: "develop",
}), }),
"unknown field repository_key", "unknown field",
); );
}); });
@@ -393,11 +420,11 @@ Deno.test("Merge Request summaries reject missing and malformed tagged source ob
{ status: "unavailable", code: "x".repeat(262_145) }, { status: "unavailable", code: "x".repeat(262_145) },
"code exceeds its length limit", "code exceeds its length limit",
], ],
[{ status: "observed", code: "stale" }, "unknown field code"], [{ status: "observed", code: "stale" }, "unknown field"],
[{ status: "not_required", code: "stale" }, "unknown field code"], [{ status: "not_required", code: "stale" }, "unknown field"],
[ [
{ status: "unavailable", code: "missing", extra: true }, { status: "unavailable", code: "missing", extra: true },
"unknown field extra", "unknown field",
], ],
] as const ] as const
) { ) {
@@ -529,7 +556,7 @@ Deno.test("Ticket Browser parser validates the canonical Merge Request thread",
...parsed, ...parsed,
thread: [{ ...parsed.thread[0], unexpected: true }], thread: [{ ...parsed.thread[0], unexpected: true }],
}), }),
"unknown field unexpected", "unknown field",
); );
}); });
@@ -692,7 +719,7 @@ Deno.test("Ticket tag projections reject missing collections and malformed ident
"last_operation_key must be a string", "last_operation_key must be a string",
], ],
[{ ...tag, deleted: "false" }, "deleted must be a boolean"], [{ ...tag, deleted: "false" }, "deleted must be a boolean"],
[{ ...tag, state: "closed" }, "unknown field state"], [{ ...tag, state: "closed" }, "unknown field"],
] as const ] as const
) { ) {
assertThrows(() => parse([value]), message); assertThrows(() => parse([value]), message);
@@ -54,10 +54,14 @@ import type {
WorkspaceTag, WorkspaceTag,
} from "#lib/generated/ticket-api.ts"; } from "#lib/generated/ticket-api.ts";
export class TicketBrowserContractError extends Error {}
export const TICKET_BROWSER_API_MAX_RESPONSE_BYTES = 4 * 1024 * 1024; export const TICKET_BROWSER_API_MAX_RESPONSE_BYTES = 4 * 1024 * 1024;
export const TICKET_BROWSER_API_LOAD_POLICY = { export const TICKET_BROWSER_API_LOAD_POLICY = {
diagnosticLabel: "Ticket Browser API", diagnosticLabel: "Ticket Browser API",
maxResponseBytes: TICKET_BROWSER_API_MAX_RESPONSE_BYTES, maxResponseBytes: TICKET_BROWSER_API_MAX_RESPONSE_BYTES,
describeValidationError: (error: unknown) =>
error instanceof TicketBrowserContractError ? error.message : undefined,
} as const; } as const;
const MAX_STRING_LENGTH = 262_144; const MAX_STRING_LENGTH = 262_144;
@@ -66,7 +70,7 @@ const MAX_MAP_ENTRIES = 4_096;
function object(value: unknown, label: string): Record<string, unknown> { function object(value: unknown, label: string): Record<string, unknown> {
if (typeof value !== "object" || value === null || Array.isArray(value)) { if (typeof value !== "object" || value === null || Array.isArray(value)) {
throw new Error(`${label} must be an object`); throw new TicketBrowserContractError(`${label} must be an object`);
} }
return value as Record<string, unknown>; return value as Record<string, unknown>;
} }
@@ -78,20 +82,20 @@ function exact(
): void { ): void {
const unexpected = Object.keys(value).find((key) => !allowed.includes(key)); const unexpected = Object.keys(value).find((key) => !allowed.includes(key));
if (unexpected !== undefined) { if (unexpected !== undefined) {
throw new Error(`${label} contains unknown field ${unexpected}`); throw new TicketBrowserContractError(`${label} contains unknown field`);
} }
} }
function string(value: unknown, label: string): string { function string(value: unknown, label: string): string {
if (typeof value !== "string") throw new Error(`${label} must be a string`); if (typeof value !== "string") throw new TicketBrowserContractError(`${label} must be a string`);
if (value.length > MAX_STRING_LENGTH) { if (value.length > MAX_STRING_LENGTH) {
throw new Error(`${label} exceeds its length limit`); throw new TicketBrowserContractError(`${label} exceeds its length limit`);
} }
return value; return value;
} }
function boolean(value: unknown, label: string): boolean { function boolean(value: unknown, label: string): boolean {
if (typeof value !== "boolean") throw new Error(`${label} must be a boolean`); if (typeof value !== "boolean") throw new TicketBrowserContractError(`${label} must be a boolean`);
return value; return value;
} }
@@ -105,7 +109,7 @@ function integer(
typeof value !== "number" || !Number.isSafeInteger(value) || typeof value !== "number" || !Number.isSafeInteger(value) ||
value < minimum || value > maximum value < minimum || value > maximum
) { ) {
throw new Error( throw new TicketBrowserContractError(
`${label} must be an integer from ${minimum} through ${maximum}`, `${label} must be an integer from ${minimum} through ${maximum}`,
); );
} }
@@ -128,9 +132,9 @@ function array<T>(
label: string, label: string,
parse: (item: unknown, label: string) => T, parse: (item: unknown, label: string) => T,
): T[] { ): T[] {
if (!Array.isArray(value)) throw new Error(`${label} must be an array`); if (!Array.isArray(value)) throw new TicketBrowserContractError(`${label} must be an array`);
if (value.length > MAX_COLLECTION_LENGTH) { if (value.length > MAX_COLLECTION_LENGTH) {
throw new Error(`${label} exceeds its item limit`); throw new TicketBrowserContractError(`${label} exceeds its item limit`);
} }
return value.map((item, index) => parse(item, `${label}[${index}]`)); return value.map((item, index) => parse(item, `${label}[${index}]`));
} }
@@ -143,12 +147,12 @@ function stringMap(value: unknown, label: string): Record<string, string> {
const source = object(value, label); const source = object(value, label);
const entries = Object.entries(source); const entries = Object.entries(source);
if (entries.length > MAX_MAP_ENTRIES) { if (entries.length > MAX_MAP_ENTRIES) {
throw new Error(`${label} exceeds its entry limit`); throw new TicketBrowserContractError(`${label} exceeds its entry limit`);
} }
return Object.fromEntries( return Object.fromEntries(
entries.map(([key, item]) => [ entries.map(([key, item]) => [
string(key, `${label} key`), string(key, `${label} key`),
string(item, `${label}.${key}`), string(item, `${label} value`),
]), ]),
); );
} }
@@ -199,7 +203,7 @@ function parseInvalidProjectRecord(
function parseTicketState(value: unknown, label: string): BrowserTicketState { function parseTicketState(value: unknown, label: string): BrowserTicketState {
if (value !== "open" && value !== "closed") { if (value !== "open" && value !== "closed") {
throw new Error(`${label} is invalid`); throw new TicketBrowserContractError(`${label} is invalid`);
} }
return value; return value;
} }
@@ -226,7 +230,7 @@ function parseCaseAction(value: unknown, label: string): TicketCaseAction {
return { action: "reopen" }; return { action: "reopen" };
} }
if (item.action !== "close" && item.action !== "correct_closure") { if (item.action !== "close" && item.action !== "correct_closure") {
throw new Error(`${label}.action is invalid`); throw new TicketBrowserContractError(`${label}.action is invalid`);
} }
exact(item, ["action", "reason"], label); exact(item, ["action", "reason"], label);
return { return {
@@ -351,7 +355,7 @@ export function parseWorkspaceTag(
], label); ], label);
const id = string(item.id, `${label}.id`); const id = string(item.id, `${label}.id`);
if (id.trim().length === 0 || new TextEncoder().encode(id).length > 128) { if (id.trim().length === 0 || new TextEncoder().encode(id).length > 128) {
throw new Error(`${label}.id must be nonempty and at most 128 UTF-8 bytes`); throw new TicketBrowserContractError(`${label}.id must be nonempty and at most 128 UTF-8 bytes`);
} }
return { return {
workspace_id: string(item.workspace_id, `${label}.workspace_id`), workspace_id: string(item.workspace_id, `${label}.workspace_id`),
@@ -588,7 +592,7 @@ function parseTicketAssignmentPrincipal(
exact(item, ["agent_key", "kind"], label); exact(item, ["agent_key", "kind"], label);
return { kind, agent_key: string(item.agent_key, `${label}.agent_key`) }; return { kind, agent_key: string(item.agent_key, `${label}.agent_key`) };
} }
throw new Error(`${label}.kind is invalid`); throw new TicketBrowserContractError(`${label}.kind is invalid`);
} }
function parseTicketAssignmentSummary( function parseTicketAssignmentSummary(
@@ -653,7 +657,7 @@ function parseTicketRoleAssignmentRecord(
], label); ], label);
const role = string(item.role, `${label}.role`); const role = string(item.role, `${label}.role`);
if (!["orchestrator", "worker", "owner", "contributor"].includes(role)) { if (!["orchestrator", "worker", "owner", "contributor"].includes(role)) {
throw new Error(`${label}.role is invalid`); throw new TicketBrowserContractError(`${label}.role is invalid`);
} }
return { return {
assigned_at: string(item.assigned_at, `${label}.assigned_at`), assigned_at: string(item.assigned_at, `${label}.assigned_at`),
@@ -681,7 +685,7 @@ export function parseAssignmentTracking(
); );
const status = string(item.status, `${label}.status`); const status = string(item.status, `${label}.status`);
if (status !== "waiting" && status !== "inprogress" && status !== "done") { if (status !== "waiting" && status !== "inprogress" && status !== "done") {
throw new Error(`${label}.status is invalid`); throw new TicketBrowserContractError(`${label}.status is invalid`);
} }
return { return {
status, status,
@@ -715,7 +719,7 @@ function parseAssignmentRelease(
], label); ], label);
const action = string(item.action, `${label}.action`); const action = string(item.action, `${label}.action`);
if (action !== "unassigned" && action !== "replaced") { if (action !== "unassigned" && action !== "replaced") {
throw new Error(`${label}.action is invalid`); throw new TicketBrowserContractError(`${label}.action is invalid`);
} }
return { return {
action, action,
@@ -839,7 +843,7 @@ function parseSourceRefObservation(
exact(item, ["status", "code"], label); exact(item, ["status", "code"], label);
return { status, code: string(item.code, `${label}.code`) }; return { status, code: string(item.code, `${label}.code`) };
} }
throw new Error(`${label}.status is invalid`); throw new TicketBrowserContractError(`${label}.status is invalid`);
} }
function parseTicketMergeRequestSummary( function parseTicketMergeRequestSummary(
@@ -1085,7 +1089,7 @@ function parseTicketTarget(value: unknown, label: string): TicketTarget {
exact(item, ["access", "ref_selector", "repository_key"], label); exact(item, ["access", "ref_selector", "repository_key"], label);
const access = string(item.access, `${label}.access`); const access = string(item.access, `${label}.access`);
if (access !== "read_only" && access !== "read_write") { if (access !== "read_only" && access !== "read_write") {
throw new Error(`${label}.access is invalid`); throw new TicketBrowserContractError(`${label}.access is invalid`);
} }
return { return {
access, access,
@@ -1234,7 +1238,7 @@ export function parseTicketRecordRef(value: unknown): TicketRef {
exact(item, ["id", "resource_key", "slug", "status"], label); exact(item, ["id", "resource_key", "slug", "status"], label);
const status = string(item.status, `${label}.status`); const status = string(item.status, `${label}.status`);
if (status !== "Open" && status !== "Closed") { if (status !== "Open" && status !== "Closed") {
throw new Error(`${label}.status is invalid`); throw new TicketBrowserContractError(`${label}.status is invalid`);
} }
return { return {
id: string(item.id, `${label}.id`), id: string(item.id, `${label}.id`),
@@ -1502,7 +1506,7 @@ function parseReviewFinding(value: unknown, label: string): ReviewFinding {
exact(item, ["body", "code", "line", "path", "severity"], label); exact(item, ["body", "code", "line", "path", "severity"], label);
const severity = string(item.severity, `${label}.severity`); const severity = string(item.severity, `${label}.severity`);
if (!["blocker", "major", "minor", "note"].includes(severity)) { if (!["blocker", "major", "minor", "note"].includes(severity)) {
throw new Error(`${label}.severity is invalid`); throw new TicketBrowserContractError(`${label}.severity is invalid`);
} }
return { return {
body: string(item.body, `${label}.body`), body: string(item.body, `${label}.body`),
@@ -1576,7 +1580,7 @@ function parseMergeRequestThreadEvent(
], label); ], label);
const decision = string(item.decision, `${label}.decision`); const decision = string(item.decision, `${label}.decision`);
if (!["approve", "request_changes"].includes(decision)) { if (!["approve", "request_changes"].includes(decision)) {
throw new Error(`${label}.decision is invalid`); throw new TicketBrowserContractError(`${label}.decision is invalid`);
} }
return { return {
...eventBase, ...eventBase,
@@ -1658,10 +1662,10 @@ function parseMergeRequestThreadEvent(
const strategy = string(item.strategy, `${label}.strategy`); const strategy = string(item.strategy, `${label}.strategy`);
const resolution = string(item.resolution, `${label}.resolution`); const resolution = string(item.resolution, `${label}.resolution`);
if (!["fast_forward", "merge"].includes(strategy)) { if (!["fast_forward", "merge"].includes(strategy)) {
throw new Error(`${label}.strategy is invalid`); throw new TicketBrowserContractError(`${label}.strategy is invalid`);
} }
if (!["none", "clean", "conflicts_resolved"].includes(resolution)) { if (!["none", "clean", "conflicts_resolved"].includes(resolution)) {
throw new Error(`${label}.resolution is invalid`); throw new TicketBrowserContractError(`${label}.resolution is invalid`);
} }
return { return {
...eventBase, ...eventBase,
@@ -1688,7 +1692,7 @@ function parseMergeRequestThreadEvent(
), ),
}; };
} }
throw new Error(`${label}.kind is invalid`); throw new TicketBrowserContractError(`${label}.kind is invalid`);
} }
function parseMergeRequestRef( function parseMergeRequestRef(
@@ -1743,7 +1747,7 @@ export function parseMergeRequestDetailResponse(
], label); ], label);
const state = string(item.state, `${label}.state`); const state = string(item.state, `${label}.state`);
if (!["open", "merged", "closed"].includes(state)) { if (!["open", "merged", "closed"].includes(state)) {
throw new Error(`${label}.state is invalid`); throw new TicketBrowserContractError(`${label}.state is invalid`);
} }
return { return {
created_at: string(item.created_at, `${label}.created_at`), created_at: string(item.created_at, `${label}.created_at`),
@@ -13,6 +13,31 @@
let resolved = $state.raw<{ path: string; value: ResolvedPageData<T> } | null>(null); let resolved = $state.raw<{ path: string; value: ResolvedPageData<T> } | null>(null);
let failure = $state.raw<{ path: string; value: DeferredFailure } | null>(null); let failure = $state.raw<{ path: string; value: DeferredFailure } | null>(null);
let loading = $state(true); let loading = $state(true);
let renderFailure = $state.raw<{ path: string; error: unknown } | null>(null);
let recoveryVersion = $state(0);
let retrying = $state(false);
let retryFailure = $state<string | null>(null);
const errorMessage = (error: unknown) => {
// SvelteKit may transform render exceptions into a serialized App.Error.
if (error !== null && typeof error === 'object' && 'message' in error && typeof error.message === 'string') {
return error.message;
}
return String(error);
};
async function retry() {
if (retrying || loading) return;
const requestedPath = path;
retrying = true;
retryFailure = null;
try {
await invalidateAll();
} catch (error) {
if (path === requestedPath) retryFailure = errorMessage(error);
} finally {
retrying = false;
}
}
const path = $derived(identity ?? page.url.pathname); const path = $derived(identity ?? page.url.pathname);
const current = $derived(resolved?.path === path ? resolved.value : null); const current = $derived(resolved?.path === path ? resolved.value : null);
const currentFailure = $derived(failure?.path === path ? failure.value : null); const currentFailure = $derived(failure?.path === path ? failure.value : null);
@@ -23,9 +48,14 @@
let active = true; let active = true;
loading = true; loading = true;
failure = null; failure = null;
retryFailure = null;
void resolvePageData(requestedData).then(async (result) => { void resolvePageData(requestedData).then(async (result) => {
if (!active) return; if (!active) return;
if (result.ok) { if (result.ok) {
// Never reset a failed boundary against stale data, including while Retry
// invalidation is pending. Remount only after this load has fresh data.
if (renderFailure?.path === requestedPath) recoveryVersion += 1;
renderFailure = null;
resolved = { path: requestedPath, value: result.data }; resolved = { path: requestedPath, value: result.data };
loading = false; loading = false;
} else if (result.failure.redirect) { } else if (result.failure.redirect) {
@@ -51,7 +81,7 @@
</script> </script>
<svelte:head> <svelte:head>
{#if currentFailure}<title>{title} · Unavailable</title> {#if currentFailure || renderFailure?.path === path}<title>{title} · Error</title>
{:else if !current}<title>{title} · Loading</title>{/if} {:else if !current}<title>{title} · Loading</title>{/if}
</svelte:head> </svelte:head>
@@ -59,11 +89,26 @@
<section class="workspace-page deferred-page" aria-label={title}> <section class="workspace-page deferred-page" aria-label={title}>
<h1>{title}</h1> <h1>{title}</h1>
<p role="alert">{currentFailure.message}</p> <p role="alert">{currentFailure.message}</p>
<button type="button" class="workspace-secondary-button" onclick={() => invalidateAll()}>Retry</button> {#if retryFailure}<p role="alert">{retryFailure}</p>{/if}
<button type="button" class="workspace-secondary-button" disabled={retrying || loading} onclick={retry}>Retry</button>
</section> </section>
{:else if current} {:else if current}
{#if loading}<p class="deferred-refresh" role="status">Updating…</p>{/if} {#if loading}<p class="deferred-refresh" role="status">Updating…</p>{/if}
{#key path}{@render children(current)}{/key} {#key path}
{#key recoveryVersion}
<svelte:boundary onerror={(error) => { renderFailure = { path, error }; }}>
{@render children(current)}
{#snippet failed(error)}
<section class="workspace-page deferred-page" aria-label={title}>
<h1>{title}</h1>
<p role="alert">{errorMessage(error)}</p>
{#if retryFailure}<p role="alert">{retryFailure}</p>{/if}
<button type="button" class="workspace-secondary-button" disabled={retrying || loading} onclick={retry}>Retry</button>
</section>
{/snippet}
</svelte:boundary>
{/key}
{/key}
{:else} {:else}
<section class="workspace-page deferred-page" aria-label={title} aria-busy="true"> <section class="workspace-page deferred-page" aria-label={title} aria-busy="true">
<h1>{title}</h1> <h1>{title}</h1>
@@ -0,0 +1,31 @@
<script lang="ts">
import DeferredPage from './DeferredPage.svelte';
import DeferredPageTestContent from './DeferredPageTestContent.svelte';
import type { DeferredResult } from './deferred.ts';
let { data, identity, onConstruct }: {
data: {
deferredWorkspace?: Promise<DeferredResult<{ workspace: string }>>;
deferredPage: Promise<DeferredResult<{ label: string; message?: string; serializedError?: boolean }>>;
};
identity: string;
onConstruct?: (label: string) => void;
} = $props();
</script>
<svelte:boundary>
<header>Valid Workspace</header>
<aside aria-label="Workspace sidebar"><a href="/w/team/tickets">Tickets</a></aside>
<main>
<DeferredPage {data} {identity} title="Ticket">
{#snippet children(content)}
<DeferredPageTestContent {content} {onConstruct} />
{/snippet}
</DeferredPage>
</main>
{#snippet failed()}
<h1>Workspace unavailable</h1>
<p>Workspace removed</p>
<button type="button">Create Workspace</button>
{/snippet}
</svelte:boundary>
@@ -0,0 +1,19 @@
<script lang="ts">
let { content, onConstruct }: {
content: { label: string; message?: string; serializedError?: boolean };
onConstruct?: (label: string) => void;
} = $props();
// Model PageContent's constructor check when an ApiResult has null data.
function construct() {
onConstruct?.(content.label);
if (content.message) {
if (content.serializedError) throw { message: content.message };
throw new Error(content.message);
}
}
construct();
</script>
<p>{content.label}</p>
<label>Draft<input aria-label="Draft" /></label>
@@ -1,8 +1,9 @@
// @vitest-environment happy-dom // @vitest-environment happy-dom
import { cleanup, fireEvent, render, waitFor } from "@testing-library/svelte"; import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/svelte";
import { afterEach, expect, test, vi } from "vitest"; import { afterEach, expect, test, vi } from "vitest";
import { createRawSnippet, tick } from "svelte"; import { createRawSnippet, tick } from "svelte";
import DeferredPage from "./DeferredPage.svelte"; import DeferredPage from "./DeferredPage.svelte";
import DeferredPageHarness from "./DeferredPageHarness.svelte";
import { type DeferredResult, settle } from "./deferred.ts"; import { type DeferredResult, settle } from "./deferred.ts";
const navigation = vi.hoisted(() => ({ const navigation = vi.hoisted(() => ({
@@ -19,6 +20,7 @@ afterEach(() => {
cleanup(); cleanup();
vi.clearAllMocks(); vi.clearAllMocks();
navigation.to = null; navigation.to = null;
navigation.invalidateAll.mockImplementation(async () => {});
}); });
function gate<T>() { function gate<T>() {
let resolve!: (value: T) => void; let resolve!: (value: T) => void;
@@ -141,3 +143,148 @@ test("old canonical redirect cannot overtake a navigation still loading its rout
await tick(); await tick();
expect(navigation.goto).not.toHaveBeenCalled(); expect(navigation.goto).not.toHaveBeenCalled();
}); });
function expectWorkspacePreserved() {
expect(screen.getByText("Valid Workspace")).toBeTruthy();
expect(screen.getByRole("complementary", { name: "Workspace sidebar" })).toBeTruthy();
expect(screen.queryByText("Workspace unavailable")).toBeNull();
expect(screen.queryByText("Workspace removed")).toBeNull();
expect(screen.queryByRole("button", { name: "Create Workspace" })).toBeNull();
}
const renderError = 'Ticket lookup failed: <img src=x onerror="throw 1">';
const brokenData = () => ({ deferredPage: settle({ label: "Broken ticket", message: renderError }) });
test("child constructor error stays local, with the actual message escaped below the page title", async () => {
const view = render(DeferredPageHarness, { data: brokenData(), identity: "team:ticket-a" });
const alert = await view.findByRole("alert");
expect(view.getByRole("heading", { name: "Ticket" })).toBeTruthy();
expect(alert.textContent).toBe(renderError);
expect(alert.querySelector("img")).toBeNull();
expect(view.container.querySelector("main h1 + p")).toBe(alert);
expect(view.queryByLabelText("Draft")).toBeNull();
expectWorkspacePreserved();
});
test("serialized SvelteKit App.Error displays its message, not object coercion", async () => {
const view = render(DeferredPageHarness, {
data: { deferredPage: settle({ label: "Broken ticket", message: renderError, serializedError: true }) },
identity: "team:ticket-a",
});
const alert = await view.findByRole("alert");
expect(alert.textContent).toBe(renderError);
expect(alert.querySelector("img")).toBeNull();
expectWorkspacePreserved();
});
test("Retry waits for new successful deferred data before remounting a failed child", async () => {
const onConstruct = vi.fn();
const view = render(DeferredPageHarness, { data: brokenData(), identity: "team:ticket-a", onConstruct });
await view.findByRole("alert");
const invalidation = gate<void>();
navigation.invalidateAll.mockImplementationOnce(() => invalidation.promise);
await fireEvent.click(view.getByRole("button", { name: "Retry" }));
expect(navigation.invalidateAll).toHaveBeenCalledOnce();
expect(onConstruct).toHaveBeenCalledTimes(1);
expect((view.getByRole("button", { name: "Retry" }) as HTMLButtonElement).disabled).toBe(true);
const refresh = gate<DeferredResult<Content>>();
await view.rerender({ data: { deferredPage: refresh.promise }, identity: "team:ticket-a", onConstruct });
invalidation.resolve();
await tick();
expect(view.getByRole("alert").textContent).toBe(renderError);
expect(onConstruct).toHaveBeenCalledTimes(1);
expect((view.getByRole("button", { name: "Retry" }) as HTMLButtonElement).disabled).toBe(true);
expectWorkspacePreserved();
refresh.resolve({ ok: true, data: { label: "Recovered ticket" } });
await view.findByText("Recovered ticket");
expect(onConstruct).toHaveBeenCalledTimes(2);
expect(view.queryByRole("alert")).toBeNull();
expect(view.queryByRole("status")).toBeNull();
expect(view.getByLabelText("Draft")).toBeTruthy();
expectWorkspacePreserved();
});
test("invalidation without new data never resets the boundary against stale failed data", async () => {
const onConstruct = vi.fn();
const view = render(DeferredPageHarness, { data: brokenData(), identity: "team:ticket-a", onConstruct });
await view.findByRole("alert");
await fireEvent.click(view.getByRole("button", { name: "Retry" }));
await tick();
expect(onConstruct).toHaveBeenCalledTimes(1);
expect(view.getByRole("alert").textContent).toBe(renderError);
expectWorkspacePreserved();
});
test("failed replacement data remains local and another fresh result can recover", async () => {
const onConstruct = vi.fn();
const view = render(DeferredPageHarness, { data: brokenData(), identity: "team:ticket-a", onConstruct });
await view.findByRole("alert");
await view.rerender({
data: { deferredPage: settle({ label: "Still broken", message: "Ticket still missing" }) },
identity: "team:ticket-a", onConstruct,
});
await waitFor(() => expect(view.getByRole("alert").textContent).toBe("Ticket still missing"));
expect(onConstruct).toHaveBeenCalledTimes(2);
expectWorkspacePreserved();
await view.rerender({ data: { deferredPage: settle({ label: "Recovered" }) }, identity: "team:ticket-a", onConstruct });
await view.findByText("Recovered");
expect(view.queryByRole("alert")).toBeNull();
});
test("changing route identity clears a failed boundary and rejects late cross-workspace results", async () => {
const onConstruct = vi.fn();
const view = render(DeferredPageHarness, { data: brokenData(), identity: "team:ticket-a", onConstruct });
await view.findByRole("alert");
const stale = gate<DeferredResult<Content>>();
await view.rerender({ data: { deferredPage: stale.promise }, identity: "team:ticket-a", onConstruct });
const workspace = gate<DeferredResult<{ workspace: string }>>();
const destination = gate<DeferredResult<Content>>();
await view.rerender({
data: { deferredWorkspace: workspace.promise, deferredPage: destination.promise },
identity: "other-team:ticket-b", onConstruct,
});
expect(view.queryByRole("alert")).toBeNull();
expect(view.getByRole("status").textContent).toBe("Loading…");
expect(view.queryByLabelText("Draft")).toBeNull();
destination.resolve({ ok: true, data: { label: "Other Workspace ticket" } });
stale.resolve({ ok: true, data: { label: "Stale team ticket" } });
await tick();
expect(onConstruct).toHaveBeenCalledTimes(1);
expect(view.queryByText("Stale team ticket")).toBeNull();
workspace.resolve({ ok: true, data: { workspace: "other-team" } });
await view.findByText("Other Workspace ticket");
expect(onConstruct.mock.calls.map(([label]) => label)).toEqual(["Broken ticket", "Other Workspace ticket"]);
expect(view.queryByRole("alert")).toBeNull();
expectWorkspacePreserved();
});
test("a failed deferred Retry never reconstructs the old failed child", async () => {
const onConstruct = vi.fn();
const view = render(DeferredPageHarness, { data: brokenData(), identity: "team:ticket-a", onConstruct });
await view.findByRole("alert");
await view.rerender({
data: { deferredPage: Promise.resolve({ ok: false, failure: { message: "Ticket access denied", status: 403, redirect: null } }) },
identity: "team:ticket-a", onConstruct,
});
await waitFor(() => expect(view.getByRole("alert").textContent).toBe("Ticket access denied"));
expect(onConstruct).toHaveBeenCalledTimes(1);
expect(view.queryByLabelText("Draft")).toBeNull();
expectWorkspacePreserved();
await view.rerender({ data: { deferredPage: settle({ label: "Authorized ticket" }) }, identity: "team:ticket-a", onConstruct });
await view.findByText("Authorized ticket");
expect(onConstruct).toHaveBeenCalledTimes(2);
expect(view.queryByRole("alert")).toBeNull();
});
test("a failed Retry request is handled locally without losing the original render error", async () => {
const view = render(DeferredPageHarness, { data: brokenData(), identity: "team:ticket-a" });
await view.findByRole("alert");
navigation.invalidateAll.mockRejectedValueOnce(new Error("Reload failed"));
await fireEvent.click(view.getByRole("button", { name: "Retry" }));
await view.findByText("Reload failed");
expect(view.getByText(renderError)).toBeTruthy();
expect((view.getByRole("button", { name: "Retry" }) as HTMLButtonElement).disabled).toBe(false);
expectWorkspacePreserved();
});
@@ -1,23 +1,16 @@
<script lang="ts"> <script lang="ts">
import { page } from "$app/state"; import { page } from "$app/state";
import "#lib/workspace/styles/workspace-catalog.css"; import "#lib/workspace/styles/workspace-pages.css";
const workspaceId = $derived(page.params.workspaceId ?? "unknown");
</script> </script>
<svelte:head><title>Workspace unavailable · Yoi</title></svelte:head> <svelte:head><title>Page error · Yoi</title></svelte:head>
<div class="workspace-catalog-shell"> <section class="workspace-page">
<section class="workspace-empty-state"> <h1>Unable to display this page</h1>
<p class="workspace-catalog-eyebrow">Workspace unavailable</p> <p role="alert">{page.error?.message ?? "An unexpected error occurred."}</p>
<h1>The selected Workspace cannot be opened</h1> <p>Error {page.status}</p>
<p> <div class="workspace-toolbar">
<code>{workspaceId}</code> may have been removed, become inaccessible, or no longer exist on <button type="button" class="workspace-secondary-button" onclick={() => window.location.reload()}>Reload page</button>
this Backend. No state from a previously selected Workspace was retained. <a href="/">Home</a>
</p> </div>
<div class="workspace-switcher-actions"> </section>
<a href="/">Choose another Workspace</a>
<a href="/#workspace-create-title">Create Workspace</a>
</div>
</section>
</div>
@@ -91,13 +91,13 @@
/> />
<svelte:head> <svelte:head>
{#if workspaceError}<title>Workspace · Unavailable</title> {#if workspaceError}<title>Workspace · Load error</title>
{:else if !workspace}<title>Workspace · Loading</title>{/if} {:else if !workspace}<title>Workspace · Loading</title>{/if}
</svelte:head> </svelte:head>
{#if workspaceError} {#if workspaceError}
<section class="workspace-page"> <section class="workspace-page">
<h1>Workspace unavailable</h1> <h1>Workspace details could not be loaded</h1>
<p role="alert">{workspaceError}</p> <p role="alert">{workspaceError}</p>
<button type="button" class="workspace-secondary-button" onclick={() => invalidateAll()}>Retry</button> <button type="button" class="workspace-secondary-button" onclick={() => invalidateAll()}>Retry</button>
</section> </section>
@@ -25,9 +25,13 @@ async function loadWorkspace(fetch: Parameters<LayoutLoad>[0]["fetch"], workspac
const workspace = workspaceResult.data; const workspace = workspaceResult.data;
const workspaceError = workspaceResult.error; const workspaceError = workspaceResult.error;
if (!workspace) { if (!workspace) {
error(404, { const message = workspaceError ?? "Workspace details could not be loaded";
message: workspaceError ?? `Workspace ${workspaceId} is unavailable`, const status = workspaceResult.status;
}); if (status !== undefined && status >= 400 && status <= 599) {
error(status, message);
}
// Transport/decoding failures do not prove that the Workspace was removed.
throw new Error(message);
} }
const repositories = repositoryResult.data; const repositories = repositoryResult.data;
@@ -1,4 +1,4 @@
import { redirect } from "@sveltejs/kit"; import { error, redirect } from "@sveltejs/kit";
import { loadJson, workspaceApiPath } from "#lib/workspace/api/http.ts"; import { loadJson, workspaceApiPath } from "#lib/workspace/api/http.ts";
import { loadWorkspaceRepositoryList } from "#lib/workspace/api/repositories.ts"; import { loadWorkspaceRepositoryList } from "#lib/workspace/api/repositories.ts";
import { parseBrowserWorkspaceOrchestratorResponse } from "#lib/workspace/api/workers.ts"; import { parseBrowserWorkspaceOrchestratorResponse } from "#lib/workspace/api/workers.ts";
@@ -36,6 +36,14 @@ export const load = deferPage(async ({ fetch, params }: Parameters<PageLoad>[0])
parseBrowserWorkspaceOrchestratorResponse, parseBrowserWorkspaceOrchestratorResponse,
), ),
]); ]);
if (!ticket.data) {
const message = ticket.error ?? "Ticket details could not be loaded";
if (ticket.status !== undefined && ticket.status >= 400 && ticket.status <= 599) {
error(ticket.status, message);
}
// Expected read failures belong to the deferred loader, not component construction.
throw new Error(message);
}
if (ticket.data) { if (ticket.data) {
const canonical = canonicalResourceReference( const canonical = canonicalResourceReference(
ticket.data.resource_key, ticket.data.resource_key,
@@ -0,0 +1,58 @@
// @vitest-environment happy-dom
import { cleanup, fireEvent, render } from "@testing-library/svelte";
import { afterEach, expect, test, vi } from "vitest";
import WorkspaceError from "./+error.svelte";
import { load } from "./+layout.ts";
const state = vi.hoisted(() => ({
page: { params: { workspaceId: "workspace-a" }, status: 500, error: { message: "Ticket Browser API response is invalid" } },
}));
vi.mock("$app/state", () => state);
afterEach(() => { cleanup(); vi.restoreAllMocks(); });
test("a child page error preserves its message and never claims Workspace deletion", async () => {
const reload = vi.spyOn(window.location, "reload").mockImplementation(() => {});
const view = render(WorkspaceError);
expect(view.getByRole("alert").textContent).toBe("Ticket Browser API response is invalid");
expect(view.getByText("Error 500")).toBeTruthy();
expect(view.queryByText("The selected Workspace cannot be opened")).toBeNull();
expect(view.queryByText("Workspace unavailable")).toBeNull();
expect(view.queryByText("Create Workspace")).toBeNull();
expect(view.queryByText("Choose another Workspace")).toBeNull();
await fireEvent.click(view.getByRole("button", { name: "Reload page" }));
expect(reload).toHaveBeenCalledOnce();
});
async function loadFailure(workspaceFetch: () => Promise<Response>) {
const fetcher = vi.fn(async (input: RequestInfo | URL) => {
if (String(input).endsWith("/workspace")) return await workspaceFetch();
return new Response("repository lookup unavailable", { status: 503 });
});
return await load({ fetch: fetcher, params: { workspaceId: "workspace-a" } } as unknown as Parameters<typeof load>[0]).deferredWorkspace;
}
test.each([401, 403, 404, 500, 503])("Workspace HTTP %i is not rewritten as 404", async (status) => {
const result = await loadFailure(async () => new Response("private backend body", { status }));
expect(result.ok).toBe(false);
if (result.ok) throw new Error("expected a failed Workspace load");
expect(result.failure.status).toBe(status);
expect(result.failure.message).toBe(`Workspace API request failed with HTTP ${status}`);
expect(result.failure.message).not.toContain("private backend body");
});
test.each(["network", "json", "contract"] as const)("Workspace %s failures do not imply a missing Workspace", async (kind) => {
const result = await loadFailure(async () => {
if (kind === "network") throw new TypeError("Failed to fetch");
return kind === "json" ? new Response("not json") : Response.json({});
});
expect(result.ok).toBe(false);
if (result.ok) throw new Error("expected a failed Workspace load");
expect(result.failure.status).toBeNull();
expect(result.failure.message).toBe({
network: "Workspace API request could not be completed",
json: "Workspace API response could not be decoded as JSON",
contract: "Workspace API response is invalid",
}[kind]);
expect(result.failure.message).not.toContain("removed");
expect(result.failure.message).not.toContain("404");
});