From f9e448be6b1d939d65f3c1e12541be811ad60ad9 Mon Sep 17 00:00:00 2001 From: eldar702 Date: Tue, 9 Jun 2026 10:18:33 +0300 Subject: [PATCH] fix: check res.ok before parsing knowledge-graph.json in dashboard The dashboard's knowledge-graph.json fetch parsed the response body unconditionally (`.then((res) => res.json())`) with no `res.ok` guard, while its four sibling fetches in the same component (meta.json, config.json, diff-overlay.json, domain-graph.json) all check `res.ok` first. When the graph file was missing or returned a non-2xx status, the 404/error body was handed to `validateGraph`, which then surfaced the misleading "Invalid knowledge graph: Missing or invalid project metadata" instead of a clear HTTP error. Diagnosis credited to collaborator ZebangCheng, who identified the exact missing guard. Fix: extract the fetch + HTTP-status guard + parse + validate chain into a small pure helper (`fetchAndValidateGraph`) that mirrors the sibling fetches by short-circuiting on `!res.ok` with an HTTP-status error, and wire App.tsx to use it. A RED-first unit test (mocked fetch) asserts that a 404 surfaces an HTTP error containing the status code rather than the schema-validation message; the 200-with-invalid-body and 401 paths are also covered. Closes #288 AI-assisted contribution. --- .../packages/dashboard/src/App.tsx | 30 +++-- .../__tests__/fetchAndValidateGraph.test.ts | 107 ++++++++++++++++++ .../src/utils/fetchAndValidateGraph.ts | 77 +++++++++++++ 3 files changed, 198 insertions(+), 16 deletions(-) create mode 100644 understand-anything-plugin/packages/dashboard/src/utils/__tests__/fetchAndValidateGraph.test.ts create mode 100644 understand-anything-plugin/packages/dashboard/src/utils/fetchAndValidateGraph.ts diff --git a/understand-anything-plugin/packages/dashboard/src/App.tsx b/understand-anything-plugin/packages/dashboard/src/App.tsx index 4d0b3969..d78fbe36 100644 --- a/understand-anything-plugin/packages/dashboard/src/App.tsx +++ b/understand-anything-plugin/packages/dashboard/src/App.tsx @@ -1,6 +1,7 @@ import { useEffect, useState, useMemo, useCallback, lazy, Suspense } from "react"; import { validateGraph } from "@understand-anything/core/schema"; import type { GraphIssue } from "@understand-anything/core/schema"; +import { fetchAndValidateGraph } from "./utils/fetchAndValidateGraph"; import { useDashboardStore } from "./store"; import GraphView from "./components/GraphView"; import DomainGraphView from "./components/DomainGraphView"; @@ -130,14 +131,12 @@ function Dashboard({ accessToken }: { accessToken: string }) { }, []); useEffect(() => { - fetch(dataUrl("knowledge-graph.json", accessToken)) - .then((res) => res.json()) - .then((data: unknown) => { - const result = validateGraph(data); - if (result.success && result.data) { - setGraph(result.data); + fetchAndValidateGraph(dataUrl("knowledge-graph.json", accessToken)) + .then((result) => { + if (result.status === "loaded") { + setGraph(result.graph); setGraphIssues(result.issues); - if ((data as Record).kind === "knowledge") { + if (result.isKnowledge) { useDashboardStore.getState().setViewMode("knowledge"); useDashboardStore.getState().setIsKnowledgeGraph(true); } @@ -148,17 +147,16 @@ function Dashboard({ accessToken }: { accessToken: string }) { console.error(`[graph] dropped: ${issue.message}`); } } - } else if (result.fatal) { - console.error("Knowledge graph validation failed:", result.fatal); - setLoadError(`Invalid knowledge graph: ${result.fatal}`); + } else if (result.status === "http-error" || result.status === "network-error") { + // Guard on res.ok (issue #288): surface a clear HTTP/network error + // instead of letting a 404 body fall through to graph validation, + // which produced a misleading "Missing or invalid project metadata". + console.error("Failed to load knowledge graph:", result.error); + setLoadError(`Failed to load knowledge graph: ${result.error}`); } else { - console.error("Knowledge graph validation failed: unknown error"); - setLoadError("Invalid knowledge graph: unknown validation error"); + console.error("Knowledge graph validation failed:", result.error); + setLoadError(`Invalid knowledge graph: ${result.error}`); } - }) - .catch((err) => { - console.error("Failed to load knowledge graph:", err); - setLoadError(`Failed to load knowledge graph: ${err instanceof Error ? err.message : String(err)}`); }); }, [setGraph]); diff --git a/understand-anything-plugin/packages/dashboard/src/utils/__tests__/fetchAndValidateGraph.test.ts b/understand-anything-plugin/packages/dashboard/src/utils/__tests__/fetchAndValidateGraph.test.ts new file mode 100644 index 00000000..4b4d6217 --- /dev/null +++ b/understand-anything-plugin/packages/dashboard/src/utils/__tests__/fetchAndValidateGraph.test.ts @@ -0,0 +1,107 @@ +import { describe, it, expect } from "vitest"; +import { fetchAndValidateGraph } from "../fetchAndValidateGraph"; + +/** + * Regression coverage for issue #288: the knowledge-graph.json fetch did not + * guard on `res.ok` before parsing, so a 404 error body was handed to + * `validateGraph`, surfacing the misleading "Missing or invalid project + * metadata" instead of a clear HTTP error. + * + * Diagnosis credited to collaborator ZebangCheng. + */ + +/** Build a minimal mock `fetch` returning the given Response-like object. */ +function mockFetch(response: Partial & { json: () => Promise }) { + return (async () => response as unknown as Response) as typeof fetch; +} + +/** A valid knowledge-graph document (passes validateGraph). */ +const VALID_GRAPH = { + kind: "knowledge", + project: { + name: "demo", + languages: ["typescript"], + frameworks: ["vitest"], + description: "A demo project", + analyzedAt: "2026-06-09T00:00:00.000Z", + gitCommitHash: "abc123", + }, + nodes: [ + { + id: "n1", + type: "article", + name: "Intro", + summary: "An article node.", + tags: [], + complexity: "simple", + }, + ], + edges: [], + layers: [], + tour: [], +}; + +describe("fetchAndValidateGraph", () => { + it("surfaces a 404 as an HTTP error, NOT a graph-validation error (issue #288)", async () => { + // The server returns a 404 whose body is an error object — exactly the + // shape that previously slipped past the missing res.ok guard. + const fetchImpl = mockFetch({ + ok: false, + status: 404, + statusText: "Not Found", + json: () => Promise.resolve({ error: "knowledge-graph.json not found" }), + }); + + const result = await fetchAndValidateGraph("/knowledge-graph.json", fetchImpl); + + expect(result.status).toBe("http-error"); + expect(result.status === "http-error" && result.error).toContain("404"); + // The pre-fix bug surfaced this misleading message; it must NOT appear. + if (result.status !== "loaded") { + expect(result.error).not.toContain("Missing or invalid project metadata"); + } + }); + + it("surfaces a 401 as an HTTP error", async () => { + const fetchImpl = mockFetch({ + ok: false, + status: 401, + statusText: "Unauthorized", + json: () => Promise.resolve({ error: "invalid token" }), + }); + + const result = await fetchAndValidateGraph("/knowledge-graph.json", fetchImpl); + + expect(result.status).toBe("http-error"); + expect(result.status === "http-error" && result.error).toContain("401"); + }); + + it("still surfaces a schema error for a 200 with an invalid graph body", async () => { + // A 200 response whose body is well-formed JSON but not a valid graph must + // still produce a validation error — the fix must not break this path. + const fetchImpl = mockFetch({ + ok: true, + status: 200, + statusText: "OK", + json: () => Promise.resolve({ not: "a graph" }), + }); + + const result = await fetchAndValidateGraph("/knowledge-graph.json", fetchImpl); + + expect(result.status).toBe("validation-error"); + }); + + it("loads a valid knowledge graph on a 200 response", async () => { + const fetchImpl = mockFetch({ + ok: true, + status: 200, + statusText: "OK", + json: () => Promise.resolve(VALID_GRAPH), + }); + + const result = await fetchAndValidateGraph("/knowledge-graph.json", fetchImpl); + + expect(result.status).toBe("loaded"); + expect(result.status === "loaded" && result.isKnowledge).toBe(true); + }); +}); diff --git a/understand-anything-plugin/packages/dashboard/src/utils/fetchAndValidateGraph.ts b/understand-anything-plugin/packages/dashboard/src/utils/fetchAndValidateGraph.ts new file mode 100644 index 00000000..4780a7fc --- /dev/null +++ b/understand-anything-plugin/packages/dashboard/src/utils/fetchAndValidateGraph.ts @@ -0,0 +1,77 @@ +import { validateGraph } from "@understand-anything/core/schema"; +import type { GraphIssue, ValidationResult } from "@understand-anything/core/schema"; + +type Graph = NonNullable; + +/** + * Result of fetching + validating a knowledge-graph JSON document. + * + * Mirrors the sibling fetches in `App.tsx` (meta.json, config.json, + * diff-overlay.json, domain-graph.json), all of which guard on `res.ok` + * before parsing. Previously the knowledge-graph fetch skipped that guard, + * so a 404 error body was handed to `validateGraph`, surfacing a misleading + * "Missing or invalid project metadata" instead of a clear HTTP error. + */ +export type FetchGraphResult = + | { status: "loaded"; graph: Graph; issues: GraphIssue[]; isKnowledge: boolean } + | { status: "http-error"; error: string } + | { status: "validation-error"; error: string } + | { status: "network-error"; error: string }; + +/** + * Fetch a knowledge-graph JSON document, guard on the HTTP status, then + * validate the payload. Pure with respect to the injected `fetch`, so it can + * be unit-tested with a mocked fetch implementation. + */ +export async function fetchAndValidateGraph( + url: string, + fetchImpl: typeof fetch = fetch, +): Promise { + let res: Response; + try { + res = await fetchImpl(url); + } catch (err) { + return { + status: "network-error", + error: err instanceof Error ? err.message : String(err), + }; + } + + // Mirror the sibling fetches: surface a clear HTTP-status error instead of + // parsing a 404/401 error body as if it were a graph document. + if (!res.ok) { + return { + status: "http-error", + error: `HTTP ${res.status}${res.statusText ? ` ${res.statusText}` : ""}`, + }; + } + + let data: unknown; + try { + data = await res.json(); + } catch (err) { + return { + status: "network-error", + error: err instanceof Error ? err.message : String(err), + }; + } + + const result = validateGraph(data); + if (result.success && result.data) { + const isKnowledge = + typeof data === "object" && + data !== null && + (data as Record).kind === "knowledge"; + return { + status: "loaded", + graph: result.data, + issues: result.issues, + isKnowledge, + }; + } + + return { + status: "validation-error", + error: result.fatal ?? "unknown validation error", + }; +}