From c145843aa3b82c9b93135490d9a26fd4106d9d14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 09:38:57 +0200 Subject: [PATCH] fix(insights): let a read's ApiError reach the failure primitives Five queryFns caught the ApiError and threw a plain Error with a translated line, so the read-failure primitives always saw a generic failure: an ended session offered Retry instead of Sign in, and a refused share drew an error card. The queryFns now pass the error through; the message lives on the primitive. The ECG list also says a failed read instead of rendering nothing. The guard now also reads every queryFn and refuses a catch that re-throws a plain Error. --- .../query-error-error-prop-guard.test.ts | 91 ++++++++++++++++++- src/app/insights/page-client.tsx | 8 +- .../insights/__tests__/ecg-section.test.tsx | 37 ++++++++ .../__tests__/rhythm-events-card.test.tsx | 22 +++++ src/components/insights/ecg-cross-link.tsx | 8 +- src/components/insights/ecg-detail.tsx | 11 +-- src/components/insights/ecg-section.tsx | 37 ++++++-- .../insights/rhythm-events-card.tsx | 12 +-- 8 files changed, 183 insertions(+), 43 deletions(-) diff --git a/src/__tests__/query-error-error-prop-guard.test.ts b/src/__tests__/query-error-error-prop-guard.test.ts index 0a61c87f1..6544cba0c 100644 --- a/src/__tests__/query-error-error-prop-guard.test.ts +++ b/src/__tests__/query-error-error-prop-guard.test.ts @@ -14,6 +14,16 @@ * and refuses both. The matcher is whitespace-tolerant (tags span lines) and * the match count is asserted, so a matcher that stops finding the tags fails * rather than passing over nothing. + * + * Handing the error over is only half of it: the error has to still be the + * `ApiError` the read threw. A `queryFn` that catches it and throws + * `new Error(t("…loadError"))` instead strips the status, so every primitive + * downstream reads "failed" and an ended session offers a retry. The second + * block reads every `queryFn` value under `src/` (`.ts` and `.tsx`, since + * query hooks live in both and a cache key is shared across files) and + * refuses a catch that re-throws a plain `Error`, or a translated message + * thrown in its place. The message belongs on the primitive (`message` / + * `title`), not inside the error. */ import { readdirSync, readFileSync } from "node:fs"; import { join, relative } from "node:path"; @@ -21,12 +31,16 @@ import { describe, expect, it } from "vitest"; const SRC = join(__dirname, ".."); -function walk(dir: string, out: string[] = []): string[] { +function walk( + dir: string, + out: string[] = [], + ext: RegExp = /\.tsx$/, +): string[] { for (const entry of readdirSync(dir, { withFileTypes: true })) { if (entry.name === "__tests__" || entry.name === "generated") continue; const path = join(dir, entry.name); - if (entry.isDirectory()) walk(path, out); - else if (entry.name.endsWith(".tsx") && !entry.name.includes(".test.")) + if (entry.isDirectory()) walk(path, out, ext); + else if (ext.test(entry.name) && !entry.name.includes(".test.")) out.push(path); } return out; @@ -80,3 +94,74 @@ describe("read-failure primitives carry their error", () => { expect(offenders).toEqual([]); }); }); + +/** + * The text of every `queryFn` value: from the key to the comma, or the + * closing bracket, that ends the property at its own depth. + */ +function queryFnBodies(source: string): string[] { + const bodies: string[] = []; + const key = /\bqueryFn\s*:/g; + for (let m = key.exec(source); m; m = key.exec(source)) { + let depth = 0; + let i = m.index + m[0].length; + for (; i < source.length; i++) { + const ch = source[i]; + if (ch === "(" || ch === "{" || ch === "[") depth++; + else if (ch === ")" || ch === "}" || ch === "]") { + if (depth === 0) break; + depth--; + } else if (ch === "," && depth === 0) break; + } + bodies.push(source.slice(m.index, i)); + } + return bodies; +} + +/** A catch that throws a plain Error, or a translated line thrown as one. */ +function masksTheReadError(body: string): boolean { + return ( + /\bcatch\b\s*(\([^)]*\))?\s*\{[^}]*\bthrow\s+new\s+Error\(/.test(body) || + /\bthrow\s+new\s+Error\(\s*t\(/.test(body) + ); +} + +const queryFns = walk(SRC, [], /\.tsx?$/).flatMap((file) => + queryFnBodies(stripComments(readFileSync(file, "utf8"))).map((body) => ({ + file: relative(SRC, file), + body, + })), +); + +describe("a read's ApiError reaches the primitive intact", () => { + it("finds the queryFns it guards", () => { + // About 310 at the time of writing. + expect(queryFns.length).toBeGreaterThan(250); + }); + + it("recognises the masking shape it refuses", () => { + expect( + masksTheReadError( + 'queryFn: async () => { try { return await apiGet(u); } catch { throw new Error(t("x.loadError")); } }', + ), + ).toBe(true); + expect( + masksTheReadError( + 'queryFn: async () => { try { return await apiGet(u); } catch (e) { throw new Error("failed"); } }', + ), + ).toBe(true); + expect(masksTheReadError("queryFn: () => apiGet(u)")).toBe(false); + expect( + masksTheReadError( + 'queryFn: async () => { if (!id) throw new Error("missing id"); return apiGet(u); }', + ), + ).toBe(false); + }); + + it("never swaps the read's error for a plain Error", () => { + const offenders = queryFns + .filter(({ body }) => masksTheReadError(body)) + .map(({ file, body }) => `${file}: ${body.split("\n")[0]}`); + expect(offenders).toEqual([]); + }); +}); diff --git a/src/app/insights/page-client.tsx b/src/app/insights/page-client.tsx index 55c1e4812..3904bf638 100644 --- a/src/app/insights/page-client.tsx +++ b/src/app/insights/page-client.tsx @@ -327,13 +327,7 @@ export default function InsightsPageClient() { // the regular shell and the tiles fill in as their data lands. const { data, isLoading, isFetched, isError } = useQuery({ queryKey: queryKeys.insightsComprehensive(), - queryFn: async () => { - try { - return await apiGet("/api/insights/comprehensive"); - } catch { - throw new Error(t("insights.loadError")); - } - }, + queryFn: () => apiGet("/api/insights/comprehensive"), enabled: isAuthenticated, }); diff --git a/src/components/insights/__tests__/ecg-section.test.tsx b/src/components/insights/__tests__/ecg-section.test.tsx index 8dff451da..cd19e8293 100644 --- a/src/components/insights/__tests__/ecg-section.test.tsx +++ b/src/components/insights/__tests__/ecg-section.test.tsx @@ -28,7 +28,14 @@ vi.mock("@tanstack/react-query", () => ({ useQuery: (opts: unknown) => useQueryMock(opts), })); +const apiGetMock = vi.fn(); +vi.mock("@/lib/api/api-fetch", async (importOriginal) => ({ + ...(await importOriginal()), + apiGet: (...args: unknown[]) => apiGetMock(...args), +})); + const { EcgSection, ECG_OVERVIEW_LIMIT } = await import("../ecg-section"); +const { ApiError } = await import("@/lib/api/api-fetch"); type EcgRecordingListItem = import("../ecg-section").EcgRecordingListItem; const IRREGULAR_REC: EcgRecordingListItem = { @@ -68,6 +75,36 @@ function countRows(html: string): number { } describe("", () => { + it("hands the read's ApiError through untouched", async () => { + // The error row decides sign-in vs retry from the status; a queryFn that + // re-threw a plain Error would turn an ended session into a retry. + useQueryMock.mockReturnValue({ data: undefined, isLoading: true }); + renderSection(undefined); + const { queryFn } = useQueryMock.mock.calls.at(-1)![0] as { + queryFn: () => Promise; + }; + const refusal = new ApiError("Unauthorized", 401); + apiGetMock.mockRejectedValueOnce(refusal); + await expect(queryFn()).rejects.toBe(refusal); + }); + + it("says an ended session instead of falling through to nothing", () => { + useQueryMock.mockReturnValue({ + data: undefined, + isLoading: false, + isError: true, + error: new ApiError("Unauthorized", 401), + refetch: vi.fn(), + }); + const html = renderToStaticMarkup( + + + , + ); + expect(html).toContain('data-slot="ecg-section-error"'); + expect(html).toContain('data-slot="query-error-sign-in"'); + }); + it("renders nothing before the payload resolves", () => { useQueryMock.mockReturnValue({ data: undefined, isLoading: true }); const html = renderToStaticMarkup( diff --git a/src/components/insights/__tests__/rhythm-events-card.test.tsx b/src/components/insights/__tests__/rhythm-events-card.test.tsx index 7de61bc59..f7665e785 100644 --- a/src/components/insights/__tests__/rhythm-events-card.test.tsx +++ b/src/components/insights/__tests__/rhythm-events-card.test.tsx @@ -28,7 +28,14 @@ vi.mock("@tanstack/react-query", () => ({ useQuery: (opts: unknown) => useQueryMock(opts), })); +const apiGetMock = vi.fn(); +vi.mock("@/lib/api/api-fetch", async (importOriginal) => ({ + ...(await importOriginal()), + apiGet: (...args: unknown[]) => apiGetMock(...args), +})); + const { RhythmEventsCard } = await import("../rhythm-events-card"); +const { ApiError } = await import("@/lib/api/api-fetch"); interface RhythmEvent { id: string; @@ -69,6 +76,21 @@ const STEADINESS_EVENT: RhythmEvent = { }; describe("", () => { + it("hands the read's ApiError through, so a 401 offers sign-in", async () => { + useQueryMock.mockReturnValue({ data: undefined, isLoading: true }); + renderToStaticMarkup( + + + , + ); + const { queryFn } = useQueryMock.mock.calls.at(-1)![0] as { + queryFn: () => Promise; + }; + const refusal = new ApiError("Unauthorized", 401); + apiGetMock.mockRejectedValueOnce(refusal); + await expect(queryFn()).rejects.toBe(refusal); + }); + it("renders nothing before the payload resolves", () => { useQueryMock.mockReturnValue({ data: undefined, isLoading: true }); const html = renderToStaticMarkup( diff --git a/src/components/insights/ecg-cross-link.tsx b/src/components/insights/ecg-cross-link.tsx index 15174c2a7..86596545c 100644 --- a/src/components/insights/ecg-cross-link.tsx +++ b/src/components/insights/ecg-cross-link.tsx @@ -59,13 +59,7 @@ export function EcgCrossLink({ enabled = true, className }: EcgCrossLinkProps) { const { data } = useQuery({ queryKey: queryKeys.insightsEcgList(), - queryFn: async () => { - try { - return await apiGet("/api/insights/ecg"); - } catch { - throw new Error(t("insights.ecg.loadError")); - } - }, + queryFn: () => apiGet("/api/insights/ecg"), enabled: enabled && isAuthenticated, }); diff --git a/src/components/insights/ecg-detail.tsx b/src/components/insights/ecg-detail.tsx index 0624ba083..ff840a099 100644 --- a/src/components/insights/ecg-detail.tsx +++ b/src/components/insights/ecg-detail.tsx @@ -47,15 +47,8 @@ export function EcgDetail({ recordingId }: { recordingId: string }) { const { data, isLoading, isError, error, refetch } = useQuery({ queryKey: queryKeys.insightsEcgDetail(recordingId, false), - queryFn: async () => { - try { - return await apiGet( - `/api/insights/ecg/${recordingId}`, - ); - } catch { - throw new Error(t("insights.ecg.loadError")); - } - }, + queryFn: () => + apiGet(`/api/insights/ecg/${recordingId}`), }); if (isError) { diff --git a/src/components/insights/ecg-section.tsx b/src/components/insights/ecg-section.tsx index 2741a7d8c..2d6016503 100644 --- a/src/components/insights/ecg-section.tsx +++ b/src/components/insights/ecg-section.tsx @@ -10,6 +10,7 @@ import { apiGet } from "@/lib/api/api-fetch"; import { useTranslations, useFormatters } from "@/lib/i18n/context"; import { cn } from "@/lib/utils"; import { SectionHeading } from "@/components/ui/section-heading"; +import { QueryErrorRow } from "@/components/ui/query-error-row"; import { ecgResultLabel, type EcgClassification, @@ -93,18 +94,38 @@ export function EcgSection({ const { t } = useTranslations(); const fmt = useFormatters(); - const { data, isLoading } = useQuery({ + const { data, isLoading, isError, error, refetch } = useQuery({ queryKey: queryKeys.insightsEcgList(), - queryFn: async () => { - try { - return await apiGet("/api/insights/ecg"); - } catch { - throw new Error(t("insights.ecg.loadError")); - } - }, + queryFn: () => apiGet("/api/insights/ecg"), enabled: enabled && isAuthenticated, }); + // A failed read is said, never swallowed into the empty gate below: on the + // dedicated page this list is the whole content. + if (isError && !data) { + return ( +
+ {!hideHeading && ( + + )} + void refetch()} + /> +
+ ); + } + // Data-availability gate — never paint an empty card. if (isLoading || !data || !data.hasRecordings) return null; diff --git a/src/components/insights/rhythm-events-card.tsx b/src/components/insights/rhythm-events-card.tsx index 6c953a550..279d00d13 100644 --- a/src/components/insights/rhythm-events-card.tsx +++ b/src/components/insights/rhythm-events-card.tsx @@ -93,15 +93,9 @@ export function RhythmEventsCard({ const { data, isLoading, isError, error, refetch } = useQuery({ queryKey: queryKeys.insightsRhythmEvents(), - queryFn: async () => { - try { - return await apiGet( - "/api/insights/rhythm-events", - ); - } catch { - throw new Error(t("insights.rhythmEvents.loadError")); - } - }, + // The ApiError passes through untouched: the error row reads its status + // to tell an ended session from a real failure. + queryFn: () => apiGet("/api/insights/rhythm-events"), enabled: enabled && isAuthenticated, });