mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-14 06:40:50 +00:00
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.
This commit is contained in:
1 parent
fea1d1e0b5
commit
c145843aa3
8 files changed
+183
-43
No files matched your search
@@ -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([]);
|
||||
});
|
||||
});
|
||||
@@ -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<ComprehensiveData>("/api/insights/comprehensive");
|
||||
} catch {
|
||||
throw new Error(t("insights.loadError"));
|
||||
}
|
||||
},
|
||||
queryFn: () => apiGet<ComprehensiveData>("/api/insights/comprehensive"),
|
||||
enabled: isAuthenticated,
|
||||
});
|
||||
|
||||
|
||||
@@ -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<typeof import("@/lib/api/api-fetch")>()),
|
||||
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("<EcgSection>", () => {
|
||||
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<unknown>;
|
||||
};
|
||||
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(
|
||||
<I18nProvider initialLocale="en">
|
||||
<EcgSection />
|
||||
</I18nProvider>,
|
||||
);
|
||||
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(
|
||||
|
||||
@@ -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<typeof import("@/lib/api/api-fetch")>()),
|
||||
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("<RhythmEventsCard>", () => {
|
||||
it("hands the read's ApiError through, so a 401 offers sign-in", async () => {
|
||||
useQueryMock.mockReturnValue({ data: undefined, isLoading: true });
|
||||
renderToStaticMarkup(
|
||||
<I18nProvider initialLocale="en">
|
||||
<RhythmEventsCard />
|
||||
</I18nProvider>,
|
||||
);
|
||||
const { queryFn } = useQueryMock.mock.calls.at(-1)![0] as {
|
||||
queryFn: () => Promise<unknown>;
|
||||
};
|
||||
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(
|
||||
|
||||
@@ -59,13 +59,7 @@ export function EcgCrossLink({ enabled = true, className }: EcgCrossLinkProps) {
|
||||
|
||||
const { data } = useQuery({
|
||||
queryKey: queryKeys.insightsEcgList(),
|
||||
queryFn: async () => {
|
||||
try {
|
||||
return await apiGet<EcgListResponse>("/api/insights/ecg");
|
||||
} catch {
|
||||
throw new Error(t("insights.ecg.loadError"));
|
||||
}
|
||||
},
|
||||
queryFn: () => apiGet<EcgListResponse>("/api/insights/ecg"),
|
||||
enabled: enabled && isAuthenticated,
|
||||
});
|
||||
|
||||
|
||||
@@ -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<EcgDetailResponse>(
|
||||
`/api/insights/ecg/${recordingId}`,
|
||||
);
|
||||
} catch {
|
||||
throw new Error(t("insights.ecg.loadError"));
|
||||
}
|
||||
},
|
||||
queryFn: () =>
|
||||
apiGet<EcgDetailResponse>(`/api/insights/ecg/${recordingId}`),
|
||||
});
|
||||
|
||||
if (isError) {
|
||||
|
||||
@@ -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<EcgListResponse>("/api/insights/ecg");
|
||||
} catch {
|
||||
throw new Error(t("insights.ecg.loadError"));
|
||||
}
|
||||
},
|
||||
queryFn: () => apiGet<EcgListResponse>("/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 (
|
||||
<section
|
||||
id="ecg"
|
||||
data-slot="ecg-section"
|
||||
aria-label={t("insights.ecg.sectionTitle")}
|
||||
className={cn("scroll-mt-24 space-y-3", className)}
|
||||
>
|
||||
{!hideHeading && (
|
||||
<SectionHeading
|
||||
icon={Activity}
|
||||
title={t("insights.ecg.sectionTitle")}
|
||||
/>
|
||||
)}
|
||||
<QueryErrorRow
|
||||
error={error}
|
||||
slot="ecg-section-error"
|
||||
message={t("insights.ecg.loadError")}
|
||||
onRetry={() => void refetch()}
|
||||
/>
|
||||
</section>
|
||||
);
|
||||
}
|
||||
|
||||
// Data-availability gate — never paint an empty card.
|
||||
if (isLoading || !data || !data.hasRecordings) return null;
|
||||
|
||||
|
||||
@@ -93,15 +93,9 @@ export function RhythmEventsCard({
|
||||
|
||||
const { data, isLoading, isError, error, refetch } = useQuery({
|
||||
queryKey: queryKeys.insightsRhythmEvents(),
|
||||
queryFn: async () => {
|
||||
try {
|
||||
return await apiGet<RhythmEventsResponse>(
|
||||
"/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<RhythmEventsResponse>("/api/insights/rhythm-events"),
|
||||
enabled: enabled && isAuthenticated,
|
||||
});
|
||||
|
||||
|
||||
Reference in new issue
Block a user