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 1/9] 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, }); From 0057b03699a3fb19f8b99757882c43f80e942cf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 09:39:50 +0200 Subject: [PATCH 2/9] refactor(openapi): derive the http code family from the status table The catalogue listed the status defaults a second time by hand. It now reads them from DEFAULT_ERROR_CODES, the table the handler applies. --- .../insights/targets/__tests__/route.test.ts | 4 ++-- src/lib/openapi/error-codes.ts | 24 ++++--------------- 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/src/app/api/insights/targets/__tests__/route.test.ts b/src/app/api/insights/targets/__tests__/route.test.ts index 9c46f189c..b948897de 100644 --- a/src/app/api/insights/targets/__tests__/route.test.ts +++ b/src/app/api/insights/targets/__tests__/route.test.ts @@ -322,8 +322,8 @@ describe("GET /api/insights/targets — mood-rollup tier swap", () => { }); it("reads latest-ever-per-type one index probe per type, floored at one year", async () => { - // Audit High finding 3 established the 365-day floor. The read is the - // shared `readLatestEver`: driven from the type list, one LIMIT 1 probe + // The read is floored at 365 days. It is the shared `readLatestEver`: + // driven from the type list, one LIMIT 1 probe // per type, so a dense type never sorts a year of raw rows to keep one. await callGet(makeReq()); diff --git a/src/lib/openapi/error-codes.ts b/src/lib/openapi/error-codes.ts index 2d34d120b..8352923ab 100644 --- a/src/lib/openapi/error-codes.ts +++ b/src/lib/openapi/error-codes.ts @@ -1,3 +1,5 @@ +import { DEFAULT_ERROR_CODES } from "@/lib/api-response"; + /** * Every `meta.errorCode` the API emits, grouped by the surface that emits it. * @@ -211,25 +213,9 @@ export const ERROR_CODE_CATALOGUE: Readonly> = "health_score_config.invalid", "health_score_config.too_narrow", ], - http: [ - "http.bad_gateway", - "http.bad_request", - "http.client_error", - "http.conflict", - "http.forbidden", - "http.gateway_timeout", - "http.gone", - "http.internal", - "http.invalid", - "http.method_not_allowed", - "http.not_found", - "http.payload_too_large", - "http.rate_limited", - "http.server_error", - "http.unauthenticated", - "http.unavailable", - "http.unsupported_media_type", - ], + // The status defaults, derived from the table `apiHandler` applies so the + // two cannot drift apart. + http: [...new Set(Object.values(DEFAULT_ERROR_CODES))].sort(), illness: [ "illness.day-log.invalid", "illness.disabled", From c300574a8ee5cb92d7924344f273fdde86607ffa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 09:48:15 +0200 Subject: [PATCH 3/9] fix(api): read the error code before the status A read-failure primitive decided from the status alone, so a 401 from a step-up or re-proof gate would have read as an ended session and sent the reader to sign-in. The code now decides first and the status is the fallback for codes it does not know. The two invitation routes answer an unknown name or e-mail with sharing.invite.no_account. Both have a second 404 with a different next step, and since every error now carries a code the clients match on it instead of on the status. The status defaults move into a module the client can import, and the contract text no longer says these arms carry no code. --- docs/api/openapi.yaml | 48 +++++++++--------- .../sharing-recoverable-errors.test.tsx | 5 +- .../zod-refusal-issue-list-guard.test.ts | 2 +- src/app/api/account/grants/route.ts | 10 +++- .../managed-profiles/[id]/guardians/route.ts | 25 ++++++---- .../managed-profile-affordances.test.tsx | 30 ++++++++--- .../settings/access/grant-invite-card.tsx | 5 +- .../access/managed-profile-guardians.tsx | 23 +++------ .../__tests__/query-error-primitives.test.tsx | 15 ++++++ .../api-handler-error-request-id.test.ts | 2 +- src/lib/api-response.ts | 50 ++----------------- src/lib/api/default-error-codes.ts | 45 +++++++++++++++++ src/lib/api/read-failure.ts | 36 +++++++++++++ src/lib/openapi/error-codes.ts | 3 +- src/lib/openapi/routes/account-sharing.ts | 13 ++--- src/lib/openapi/routes/auth.ts | 2 +- src/lib/openapi/routes/profile.ts | 2 +- src/lib/sharing/grant-view.ts | 7 +++ .../managed-profile-surface.test.ts | 15 +++--- tests/integration/sharing-lifecycle.test.ts | 2 + 20 files changed, 212 insertions(+), 128 deletions(-) create mode 100644 src/lib/api/default-error-codes.ts diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 3230be4c9..a6ba6f244 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -3516,9 +3516,10 @@ paths: $ref: "#/components/schemas/ApiTokenRevokeEnvelope" "401": *a1 "403": - description: The operator has switched the API off instance-wide (`AppSettings.apiGlobal`). No `meta.errorCode`; nothing - was revoked. The sibling list is deliberately not gated this way — a token stays live while the switch is - off, so its owner can still see it here even though this call will not kill it. + description: The operator has switched the API off instance-wide (`AppSettings.apiGlobal`). `meta.errorCode` is the + status default `http.forbidden`; nothing was revoked. The sibling list is deliberately not gated this way — + a token stays live while the switch is off, so its owner can still see it here even though this call will + not kill it. content: application/json: schema: @@ -7706,7 +7707,7 @@ paths: `health_score_config.invalid` — the body failed validation, with the per-issue list under `details.issues`; `invalid_base_updated_at` — `baseUpdatedAt` was present but unparseable, `null` included, which is rejected rather than falling back to the unconditional write. A body that is not JSON at all returns the bare message - `health-score-config.body.invalid_json` with no errorCode." + `health-score-config.body.invalid_json` with the status default `meta.errorCode: http.invalid`." content: application/json: schema: @@ -21519,7 +21520,7 @@ paths: schema: $ref: "#/components/schemas/ErrorEnvelope" "404": - description: No account on this instance carries that identifier. + description: "No account on this instance carries that identifier (`meta.errorCode: sharing.invite.no_account`)." content: application/json: schema: @@ -21710,8 +21711,8 @@ paths: $ref: "#/components/schemas/ErrorEnvelope" "422": *a4 "429": - description: More than ten profiles from one caller within the hour. No error code — rate-limit refusals carry none - anywhere in this API; branch on the status. Nothing was created. + description: "More than ten profiles from one caller within the hour (`meta.errorCode: http.rate_limited`, the status + default; a 429 has one meaning here, so the status alone is enough to branch on). Nothing was created." content: application/json: schema: @@ -21794,8 +21795,9 @@ paths: $ref: "#/components/schemas/ErrorEnvelope" "422": *a4 "429": - description: More than ten edits from one caller within the hour — the ceiling creation carries. No error code; branch - on the status. Nothing was changed, and the refusal lands before the record named in the path is looked at. + description: "More than ten edits from one caller within the hour — the ceiling creation carries. `meta.errorCode: + http.rate_limited`, the status default. Nothing was changed, and the refusal lands before the record named + in the path is looked at." content: application/json: schema: @@ -21880,11 +21882,10 @@ paths: summary: Invite another Guardian description: "Offers somebody else a share in looking after this profile, at MANAGE over the whole record. Cookie-only and step-up gated, like every act in this family — the read beside it is not. **Resolve refusals on - `meta.errorCode` first and fall back to the status only for the two arms that carry none**, because this route - answers 404 twice and the codeless one is the less serious of the two: an identifier that matches no account - carries no code, while `managed_profile.not_found` means the caller is not a Guardian of that profile at all. A - client branching on the status first would tell somebody to check a spelling when the real answer is that the - record is no longer theirs to administer." + `meta.errorCode`, not on the status**, because this route answers 404 twice and the two need different next + steps: `sharing.invite.no_account` means no account carries that identifier, while `managed_profile.not_found` + means the caller is not a Guardian of that profile at all. A client branching on the status would tell somebody + to check a spelling when the real answer is that the record is no longer theirs to administer." parameters: - in: path name: id @@ -21911,11 +21912,11 @@ paths: schema: $ref: "#/components/schemas/ErrorEnvelope" "404": - description: "Two refusals share this status and only one carries a code. **No `meta.errorCode`**: no account on this - instance carries that identifier — the same deliberate disclosure to an authenticated caller that `POST - /api/account/grants` makes, and codeless there too, so this family does not answer one way here and another - way one page over. `meta.errorCode: managed_profile.not_found`: no such managed profile, or the caller is - not one of its Guardians." + description: "Two refusals share this status, each with its own code. `meta.errorCode: sharing.invite.no_account`: no + account on this instance carries that identifier — the same deliberate disclosure to an authenticated caller + that `POST /api/account/grants` makes, with the same code, so this family does not answer one way here and + another way one page over. `meta.errorCode: managed_profile.not_found`: no such managed profile, or the + caller is not one of its Guardians." content: application/json: schema: @@ -21938,7 +21939,8 @@ paths: schema: $ref: "#/components/schemas/ErrorEnvelope" "429": - description: More than ten invitations from one caller within the hour. No error code, as above; branch on the status. + description: "More than ten invitations from one caller within the hour (`meta.errorCode: http.rate_limited`, the status + default)." content: application/json: schema: @@ -36227,9 +36229,9 @@ components: share-link.selection.forbidden_leaf, share-link.selection.unknown_leaf; sharing — sharing.accept.expired, sharing.accept.not_found, sharing.accept.not_pending, sharing.accept.revoked, sharing.access.denied, sharing.invite.duplicate, sharing.invite.invalid, sharing.invite.invalid_scope, - sharing.invite.manage_browser_only, sharing.invite.refused, sharing.invite.self, sharing.not_permitted, - sharing.renounce.already_ended, sharing.renounce.not_found, sharing.revoke.already_ended, - sharing.revoke.not_found, sharing.session.changed, sharing.switch.invalid, + sharing.invite.manage_browser_only, sharing.invite.no_account, sharing.invite.refused, + sharing.invite.self, sharing.not_permitted, sharing.renounce.already_ended, sharing.renounce.not_found, + sharing.revoke.already_ended, sharing.revoke.not_found, sharing.session.changed, sharing.switch.invalid, sharing.switch.wrong_transport; source-priority — source-priority.body.invalid_json, source-priority.body.invalid_shape; symptoms — symptoms.definition.limitReached, symptoms.definition.notFound, symptoms.episode.notOpen, symptoms.event.notFound, symptoms.invalid; diff --git a/src/__tests__/sharing-recoverable-errors.test.tsx b/src/__tests__/sharing-recoverable-errors.test.tsx index 67f84b3ff..26dafe0fb 100644 --- a/src/__tests__/sharing-recoverable-errors.test.tsx +++ b/src/__tests__/sharing-recoverable-errors.test.tsx @@ -335,9 +335,12 @@ describe("a refused invitation keeps what was typed", () => { expect(inviteErrorKey(new TypeError("Failed to fetch"))).toBe( "recordSharing.invite.errorOffline", ); - expect(inviteErrorKey(apiError(404))).toBe( + expect(inviteErrorKey(apiError(404, "sharing.invite.no_account"))).toBe( "recordSharing.invite.errorNoAccount", ); + expect(inviteErrorKey(apiError(404, "http.not_found"))).toBe( + "recordSharing.invite.errorFailed", + ); expect(inviteErrorKey(apiError(409, "sharing.invite.duplicate"))).toBe( "recordSharing.invite.errorDuplicate", ); diff --git a/src/__tests__/zod-refusal-issue-list-guard.test.ts b/src/__tests__/zod-refusal-issue-list-guard.test.ts index 57426b41b..242353000 100644 --- a/src/__tests__/zod-refusal-issue-list-guard.test.ts +++ b/src/__tests__/zod-refusal-issue-list-guard.test.ts @@ -188,7 +188,7 @@ const TOKEN_IN_ERROR_422: Record = { "src/app/api/auth/me/health-score-config/route.ts health-score-config.body.invalid_json": "The published 422 description spells this exact body out as what a non-JSON request earns. It is the one operation whose contract promises the token, so the token is what it answers with.", "src/app/api/coach/reminder-suggestions/route.ts coach.suggestion.unknownCadence": - "Predates this sweep and carries no `meta.errorCode` yet; the Coach reminder surfaces move as one piece or not at all.", + "Predates this sweep and carries only the status default code; the Coach reminder surfaces move as one piece or not at all.", "src/app/api/coach/reminders/route.ts coach.reminder.invalidWhen": "Predates this sweep, same surface.", "src/app/api/coach/reminders/[id]/route.ts coach.reminder.invalidWhen": diff --git a/src/app/api/account/grants/route.ts b/src/app/api/account/grants/route.ts index 5d093073b..2851c662e 100644 --- a/src/app/api/account/grants/route.ts +++ b/src/app/api/account/grants/route.ts @@ -66,7 +66,11 @@ import { prisma } from "@/lib/db"; import { getAuditLogRetentionDays } from "@/lib/jobs/audit-log-cleanup"; import { annotate } from "@/lib/logging/context"; import { checkRateLimit } from "@/lib/rate-limit"; -import { GRANT_PARTY_SELECT, toGrantView } from "@/lib/sharing/grant-view"; +import { + GRANT_PARTY_SELECT, + INVITE_NO_ACCOUNT_CODE, + toGrantView, +} from "@/lib/sharing/grant-view"; import { GrantError, inviteGrant } from "@/lib/sharing/grants"; import { inviteGrantSchema } from "@/lib/validations/account-sharing"; import { apiListSuccess, capRows, overfetch } from "@/lib/api/list-cap"; @@ -221,7 +225,9 @@ export const POST = apiHandler(async (request: NextRequest) => { select: GRANT_PARTY_SELECT, }); if (!invitee) { - return apiError("No account with that name or e-mail", 404); + return apiError("No account with that name or e-mail", 404, { + errorCode: INVITE_NO_ACCOUNT_CODE, + }); } await auth.commitElevation(); diff --git a/src/app/api/managed-profiles/[id]/guardians/route.ts b/src/app/api/managed-profiles/[id]/guardians/route.ts index 8afbb7e28..01e51b55b 100644 --- a/src/app/api/managed-profiles/[id]/guardians/route.ts +++ b/src/app/api/managed-profiles/[id]/guardians/route.ts @@ -20,7 +20,10 @@ import { } from "@/lib/managed-profiles/guardian-list"; import { ManagedProfileLifecycleError } from "@/lib/managed-profiles/lifecycle"; import { checkRateLimit } from "@/lib/rate-limit"; -import { GRANT_PARTY_SELECT } from "@/lib/sharing/grant-view"; +import { + GRANT_PARTY_SELECT, + INVITE_NO_ACCOUNT_CODE, +} from "@/lib/sharing/grant-view"; import { GrantError, inviteManagedProfileGuardian } from "@/lib/sharing/grants"; import { inviteManagedProfileGuardianSchema } from "@/lib/validations/managed-profiles"; @@ -106,16 +109,16 @@ export const POST = apiHandler( }, select: GRANT_PARTY_SELECT, }); - // No `errorCode`, deliberately, and the same for the 429 above. Rate-limit - // refusals carry none anywhere in this application, and the sibling - // invitation route (`POST /api/account/grants`) answers an unknown - // identifier with a bare 404 too. A code here would make one route in the - // family unlike both. The published contract says so, and says that a - // client resolves on `meta.errorCode` FIRST: this route's other 404 — - // "you are not a Guardian of that profile" — does carry one, and branching - // on the status would tell somebody to check a spelling when the real - // answer is that the profile is no longer theirs to administer. - if (!invitee) return apiError("No account with that name or e-mail", 404); + // The same code the sibling invitation route (`POST /api/account/grants`) + // answers an unknown identifier with. This route has a second 404 — "you + // are not a Guardian of that profile", `managed_profile.not_found` — and + // the two need different next steps, so each names itself and a client + // never has to tell them apart by status. + if (!invitee) { + return apiError("No account with that name or e-mail", 404, { + errorCode: INVITE_NO_ACCOUNT_CODE, + }); + } try { const grant = await inviteManagedProfileGuardian({ diff --git a/src/components/settings/access/__tests__/managed-profile-affordances.test.tsx b/src/components/settings/access/__tests__/managed-profile-affordances.test.tsx index 1ccb24492..4601462d1 100644 --- a/src/components/settings/access/__tests__/managed-profile-affordances.test.tsx +++ b/src/components/settings/access/__tests__/managed-profile-affordances.test.tsx @@ -837,14 +837,16 @@ describe("what a refused guardian invitation says", () => { why: "a 404 WITH a code — not the same as an unknown identifier", }, { - err: new ApiError("refused", 404), + err: new ApiError("refused", 404, { + errorCode: "sharing.invite.no_account", + }), key: "recordSharing.managed.guardianErrorNoAccount", - why: "the codeless 404: no account carries that name or e-mail", + why: "the other 404: no account carries that name or e-mail", }, { - err: new ApiError("refused", 429), + err: new ApiError("refused", 429, { errorCode: "http.rate_limited" }), key: "recordSharing.managed.guardianErrorRateLimit", - why: "the other codeless arm, where waiting is the whole recovery", + why: "a 429 has one meaning, where waiting is the whole recovery", }, { err: new ApiError("refused", 401, { @@ -872,16 +874,28 @@ describe("what a refused guardian invitation says", () => { } it("tells the two 404s apart, which is the whole reason for the order", () => { - // The route publishes a code on one 404 and not on the other. Branching on - // status first would tell somebody to check a spelling when the real - // answer is that they are no longer a Guardian of that profile. + // Each 404 names itself. Branching on the status would tell somebody to + // check a spelling when the real answer is that they are no longer a + // Guardian of that profile, and a 404 that names neither is not an + // unknown name either. expect( guardianInviteErrorKey( new ApiError("refused", 404, { errorCode: "managed_profile.not_found", }), ), - ).not.toBe(guardianInviteErrorKey(new ApiError("refused", 404))); + ).not.toBe( + guardianInviteErrorKey( + new ApiError("refused", 404, { + errorCode: "sharing.invite.no_account", + }), + ), + ); + expect( + guardianInviteErrorKey( + new ApiError("refused", 404, { errorCode: "http.not_found" }), + ), + ).toBe("recordSharing.managed.guardianErrorFailed"); // And all nine cases above resolve to distinct sentences rather than one: // the failure this table is written against is one message for everything. expect( diff --git a/src/components/settings/access/grant-invite-card.tsx b/src/components/settings/access/grant-invite-card.tsx index a4828182a..eb3544759 100644 --- a/src/components/settings/access/grant-invite-card.tsx +++ b/src/components/settings/access/grant-invite-card.tsx @@ -747,12 +747,11 @@ export function inviteErrorKey(err: unknown): string { // sent" is not. case "sharing.invite.manage_browser_only": return "recordSharing.invite.errorManageBrowserOnly"; + case "sharing.invite.no_account": + return "recordSharing.invite.errorNoAccount"; default: break; } - if (err instanceof ApiError && err.status === 404) { - return "recordSharing.invite.errorNoAccount"; - } if (err instanceof ApiError && err.status === 429) { return "recordSharing.invite.errorRateLimited"; } diff --git a/src/components/settings/access/managed-profile-guardians.tsx b/src/components/settings/access/managed-profile-guardians.tsx index b6f831e2f..0526868ba 100644 --- a/src/components/settings/access/managed-profile-guardians.tsx +++ b/src/components/settings/access/managed-profile-guardians.tsx @@ -308,18 +308,12 @@ export function ManagedProfileGuardians({ /** * The message for a refused Guardian invitation. * - * Six refusals, and **two of them carry no `errorCode`** — the rate limit - * (`guardians/route.ts`, before the body is even read) and the unknown-account - * 404. That asymmetry is the route's actual shape, not laziness here: the - * other four arms publish a stable code and are matched on it, and these two - * have to be told apart by status because there is nothing else to branch on. - * Recorded in `02-13.1-CONTRACT-FINDINGS.md` so whoever freezes the contract - * decides deliberately whether to close it. - * - * The code switch runs FIRST, and the order matters: `managed_profile.not_found` - * is a 404 WITH a code, and an unknown account is a 404 without one. Branching - * on status first would tell somebody to check the spelling of a name when the - * real answer is that they are not a Guardian of that profile any more. + * Six refusals, matched on their code. The route answers 404 twice, and the + * two need different next steps: `sharing.invite.no_account` asks somebody to + * check the spelling of a name, `managed_profile.not_found` says they are not + * a Guardian of that profile any more. Branching on the status would collapse + * them, so only the statuses with a single meaning fall back to the status: a + * 429 (wait, whatever the code) and a 401 (the step-up gate). */ export function guardianInviteErrorKey(err: unknown): string { if (!(err instanceof ApiError)) return "recordSharing.managed.errorOffline"; @@ -340,14 +334,13 @@ export function guardianInviteErrorKey(err: unknown): string { return "recordSharing.managed.guardianErrorManagedInvitee"; case "managed_profile.not_found": return "recordSharing.actionError.profileNotFound"; + case "sharing.invite.no_account": + return "recordSharing.managed.guardianErrorNoAccount"; default: break; } - // The two codeless arms. Waiting is the recovery for one and correcting the - // identifier is the recovery for the other, so they must not collapse. if (err.status === 429) return "recordSharing.managed.guardianErrorRateLimit"; - if (err.status === 404) return "recordSharing.managed.guardianErrorNoAccount"; // The step-up gate this whole family carries. Not a failure — and it comes // in two shapes, because an account with no factor enrolled cannot clear it. if (err.status === 401) return stepUpErrorKey(err); diff --git a/src/components/ui/__tests__/query-error-primitives.test.tsx b/src/components/ui/__tests__/query-error-primitives.test.tsx index 3edb66fa3..bd70c4a46 100644 --- a/src/components/ui/__tests__/query-error-primitives.test.tsx +++ b/src/components/ui/__tests__/query-error-primitives.test.tsx @@ -58,6 +58,21 @@ describe("readFailureKind", () => { expect(readFailureKind(new TypeError("Failed to fetch"))).toBe("failed"); expect(readFailureKind(null)).toBe("failed"); }); + + it("reads the code before the status", () => { + const withCode = (status: number, errorCode: string) => + readFailureKind(new ApiError("x", status, { errorCode })); + // A step-up or re-proof gate answers 401 on a session that is still good: + // sending that reader to sign-in would end it. + expect(withCode(401, "auth.stepup.required")).toBe("failed"); + expect(withCode(401, "auth.reproof.required")).toBe("failed"); + expect(withCode(401, "auth.mfa.code_invalid")).toBe("failed"); + expect(withCode(401, "auth.token.expired")).toBe("unauthenticated"); + expect(withCode(401, "http.unauthenticated")).toBe("unauthenticated"); + // An unknown code falls back to the status. + expect(withCode(401, "auth.something.new")).toBe("unauthenticated"); + expect(withCode(403, "http.forbidden")).toBe("refused"); + }); }); describe.each([ diff --git a/src/lib/__tests__/api-handler-error-request-id.test.ts b/src/lib/__tests__/api-handler-error-request-id.test.ts index de8e74f76..334f86c7d 100644 --- a/src/lib/__tests__/api-handler-error-request-id.test.ts +++ b/src/lib/__tests__/api-handler-error-request-id.test.ts @@ -24,9 +24,9 @@ import { apiHandler, HttpError } from "@/lib/api-handler"; import { apiError, apiSuccess, - defaultErrorCodeForStatus, withErrorEnvelopeDefaults, } from "@/lib/api-response"; +import { defaultErrorCodeForStatus } from "@/lib/api/default-error-codes"; const INCOMING = "3f9a2c1b-0000-4000-8000-000000000001"; diff --git a/src/lib/api-response.ts b/src/lib/api-response.ts index 03b3e60c0..81edb9819 100644 --- a/src/lib/api-response.ts +++ b/src/lib/api-response.ts @@ -2,6 +2,8 @@ import { isIP } from "node:net"; import { NextResponse } from "next/server"; import type { ZodError, ZodIssue } from "zod/v4"; +import { defaultErrorCodeForStatus } from "@/lib/api/default-error-codes"; + export function apiSuccess(data: T, status = 200) { return NextResponse.json({ data, error: null }, { status }); } @@ -308,52 +310,6 @@ function isPlainObject(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } -/** - * The `meta.errorCode` an error envelope carries when its route named none. - * - * `meta.errorCode` is what a client is told to branch on, and roughly half of - * the refusals in the tree carried one: the rest answered prose alone, so a - * client could branch on the code for one 404 and had to fall back to the - * status for the next. Every error envelope now has a code. A route that names - * its own keeps it; one that does not gets the status's code from this table, - * so the generic family is predictable from the status alone and a client - * never has to special-case an absent field. - * - * Deliberately coarse. These say which kind of refusal it was, never why — - * a route that has a reason worth branching on names its own code. Statuses - * not listed fall back to `http.client_error` / `http.server_error`. - */ -export const DEFAULT_ERROR_CODES = { - 400: "http.bad_request", - 401: "http.unauthenticated", - 403: "http.forbidden", - 404: "http.not_found", - 405: "http.method_not_allowed", - 409: "http.conflict", - 410: "http.gone", - 413: "http.payload_too_large", - 415: "http.unsupported_media_type", - 422: "http.invalid", - 429: "http.rate_limited", - 500: "http.internal", - 502: "http.bad_gateway", - 503: "http.unavailable", - 504: "http.gateway_timeout", - client: "http.client_error", - server: "http.server_error", -} as const; - -/** The default code for an error status, see {@link DEFAULT_ERROR_CODES}. */ -export function defaultErrorCodeForStatus(status: number): string { - const listed = (DEFAULT_ERROR_CODES as Record)[ - status - ]; - if (listed) return listed; - return status >= 500 - ? DEFAULT_ERROR_CODES.server - : DEFAULT_ERROR_CODES.client; -} - /** * Complete an error envelope: `meta.requestId` always, and `meta.errorCode` * from the status when the route named none. @@ -362,7 +318,7 @@ export function defaultErrorCodeForStatus(status: number): string { * envelope carries the full request id beside the `x-request-id` header, so a * client behind a proxy that strips headers can still show the id a person * reads out to the operator, who finds the event in the admin log with it. - * The code follows {@link DEFAULT_ERROR_CODES}. + * The code follows `DEFAULT_ERROR_CODES` (`src/lib/api/default-error-codes.ts`). * * Applied once, by `apiHandler`, to whatever the handler answered, so a route * that builds its error with `apiError`, with `NextResponse.json` by hand or by diff --git a/src/lib/api/default-error-codes.ts b/src/lib/api/default-error-codes.ts new file mode 100644 index 000000000..84c039c28 --- /dev/null +++ b/src/lib/api/default-error-codes.ts @@ -0,0 +1,45 @@ +/** + * The `meta.errorCode` an error envelope carries when its route named none. + * + * `meta.errorCode` is what a client is told to branch on, and roughly half of + * the refusals in the tree carried one: the rest answered prose alone, so a + * client could branch on the code for one 404 and had to fall back to the + * status for the next. Every error envelope now has a code. A route that names + * its own keeps it; one that does not gets the status's code from this table, + * so the generic family is predictable from the status alone and a client + * never has to special-case an absent field. + * + * Deliberately coarse. These say which kind of refusal it was, never why — + * a route that has a reason worth branching on names its own code. Statuses + * not listed fall back to `http.client_error` / `http.server_error`. + */ +export const DEFAULT_ERROR_CODES = { + 400: "http.bad_request", + 401: "http.unauthenticated", + 403: "http.forbidden", + 404: "http.not_found", + 405: "http.method_not_allowed", + 409: "http.conflict", + 410: "http.gone", + 413: "http.payload_too_large", + 415: "http.unsupported_media_type", + 422: "http.invalid", + 429: "http.rate_limited", + 500: "http.internal", + 502: "http.bad_gateway", + 503: "http.unavailable", + 504: "http.gateway_timeout", + client: "http.client_error", + server: "http.server_error", +} as const; + +/** The default code for an error status, see {@link DEFAULT_ERROR_CODES}. */ +export function defaultErrorCodeForStatus(status: number): string { + const listed = (DEFAULT_ERROR_CODES as Record)[ + status + ]; + if (listed) return listed; + return status >= 500 + ? DEFAULT_ERROR_CODES.server + : DEFAULT_ERROR_CODES.client; +} diff --git a/src/lib/api/read-failure.ts b/src/lib/api/read-failure.ts index 0a6c8b388..9a87a0476 100644 --- a/src/lib/api/read-failure.ts +++ b/src/lib/api/read-failure.ts @@ -1,3 +1,6 @@ +import { AUTH_ERROR_CODES, REPROOF_REQUIRED_CODE } from "@/lib/api-errors"; +import { DEFAULT_ERROR_CODES } from "@/lib/api/default-error-codes"; + import { ApiError } from "./api-fetch"; /** @@ -12,12 +15,45 @@ import { ApiError } from "./api-fetch"; * - `throttled`: 429. The read budget is spent for now; an immediate retry * would only be refused again. * - `failed`: everything else, a real read failure worth a retry. + * + * The code decides before the status. A 401 is not always an ended session: + * a step-up or re-proof gate answers 401 too, with the session intact, and + * sending that reader to sign-in would end a session that was fine. So a + * code that names a missing proof reads as `failed`, a code that names an + * ended credential reads as `unauthenticated` whatever the status, and only + * a code this table does not know (or none, from a proxy or an older server) + * falls back to the status. */ export type ReadFailureKind = "unauthenticated" | "refused" | "throttled" | "failed"; +/** Codes that say the credential itself is gone: sign in again. */ +const SESSION_ENDED_CODES: ReadonlySet = new Set([ + AUTH_ERROR_CODES.missing, + AUTH_ERROR_CODES.expired, + AUTH_ERROR_CODES.invalid, + DEFAULT_ERROR_CODES[401], + "auth.refresh.invalid", + "auth.refresh.reuse", + "auth.refresh.revoked", +]); + +/** Codes that ask for a fresh proof on a session that is still good. */ +const PROOF_NEEDED_CODES: ReadonlySet = new Set([ + "auth.stepup.required", + REPROOF_REQUIRED_CODE, + "auth.reproof.failed", + "auth.reproof.too_weak", + AUTH_ERROR_CODES.mfaCode, +]); + export function readFailureKind(error: unknown): ReadFailureKind { if (error instanceof ApiError) { + const code = error.meta?.errorCode; + if (typeof code === "string") { + if (SESSION_ENDED_CODES.has(code)) return "unauthenticated"; + if (PROOF_NEEDED_CODES.has(code)) return "failed"; + } if (error.status === 401) return "unauthenticated"; if (error.status === 403) return "refused"; if (error.status === 429) return "throttled"; diff --git a/src/lib/openapi/error-codes.ts b/src/lib/openapi/error-codes.ts index 8352923ab..460074912 100644 --- a/src/lib/openapi/error-codes.ts +++ b/src/lib/openapi/error-codes.ts @@ -1,4 +1,4 @@ -import { DEFAULT_ERROR_CODES } from "@/lib/api-response"; +import { DEFAULT_ERROR_CODES } from "@/lib/api/default-error-codes"; /** * Every `meta.errorCode` the API emits, grouped by the surface that emits it. @@ -412,6 +412,7 @@ export const ERROR_CODE_CATALOGUE: Readonly> = "sharing.invite.invalid", "sharing.invite.invalid_scope", "sharing.invite.manage_browser_only", + "sharing.invite.no_account", "sharing.invite.refused", "sharing.invite.self", "sharing.not_permitted", diff --git a/src/lib/openapi/routes/account-sharing.ts b/src/lib/openapi/routes/account-sharing.ts index c12a73a2d..589147f18 100644 --- a/src/lib/openapi/routes/account-sharing.ts +++ b/src/lib/openapi/routes/account-sharing.ts @@ -945,7 +945,8 @@ export const accountSharingPaths: NonNullable = { content: { "application/json": { schema: errorEnvelope } }, }, "404": { - description: "No account on this instance carries that identifier.", + description: + "No account on this instance carries that identifier (`meta.errorCode: sharing.invite.no_account`).", content: { "application/json": { schema: errorEnvelope } }, }, "409": { @@ -1099,7 +1100,7 @@ export const accountSharingPaths: NonNullable = { }, "429": { description: - "More than ten profiles from one caller within the hour. No error code — rate-limit refusals carry none anywhere in this API; branch on the status. Nothing was created.", + "More than ten profiles from one caller within the hour (`meta.errorCode: http.rate_limited`, the status default; a 429 has one meaning here, so the status alone is enough to branch on). Nothing was created.", content: { "application/json": { schema: errorEnvelope } }, }, }, @@ -1167,7 +1168,7 @@ export const accountSharingPaths: NonNullable = { }, "429": { description: - "More than ten edits from one caller within the hour — the ceiling creation carries. No error code; branch on the status. Nothing was changed, and the refusal lands before the record named in the path is looked at.", + "More than ten edits from one caller within the hour — the ceiling creation carries. `meta.errorCode: http.rate_limited`, the status default. Nothing was changed, and the refusal lands before the record named in the path is looked at.", content: { "application/json": { schema: errorEnvelope } }, }, }, @@ -1239,7 +1240,7 @@ export const accountSharingPaths: NonNullable = { tags: ["Account sharing"], summary: "Invite another Guardian", description: - "Offers somebody else a share in looking after this profile, at MANAGE over the whole record. Cookie-only and step-up gated, like every act in this family — the read beside it is not. **Resolve refusals on `meta.errorCode` first and fall back to the status only for the two arms that carry none**, because this route answers 404 twice and the codeless one is the less serious of the two: an identifier that matches no account carries no code, while `managed_profile.not_found` means the caller is not a Guardian of that profile at all. A client branching on the status first would tell somebody to check a spelling when the real answer is that the record is no longer theirs to administer.", + "Offers somebody else a share in looking after this profile, at MANAGE over the whole record. Cookie-only and step-up gated, like every act in this family — the read beside it is not. **Resolve refusals on `meta.errorCode`, not on the status**, because this route answers 404 twice and the two need different next steps: `sharing.invite.no_account` means no account carries that identifier, while `managed_profile.not_found` means the caller is not a Guardian of that profile at all. A client branching on the status would tell somebody to check a spelling when the real answer is that the record is no longer theirs to administer.", requestParams: { path: z.object({ id: z.string() }), }, @@ -1272,7 +1273,7 @@ export const accountSharingPaths: NonNullable = { }, "404": { description: - "Two refusals share this status and only one carries a code. **No `meta.errorCode`**: no account on this instance carries that identifier — the same deliberate disclosure to an authenticated caller that `POST /api/account/grants` makes, and codeless there too, so this family does not answer one way here and another way one page over. `meta.errorCode: managed_profile.not_found`: no such managed profile, or the caller is not one of its Guardians.", + "Two refusals share this status, each with its own code. `meta.errorCode: sharing.invite.no_account`: no account on this instance carries that identifier — the same deliberate disclosure to an authenticated caller that `POST /api/account/grants` makes, with the same code, so this family does not answer one way here and another way one page over. `meta.errorCode: managed_profile.not_found`: no such managed profile, or the caller is not one of its Guardians.", content: { "application/json": { schema: errorEnvelope } }, }, "409": { @@ -1287,7 +1288,7 @@ export const accountSharingPaths: NonNullable = { }, "429": { description: - "More than ten invitations from one caller within the hour. No error code, as above; branch on the status.", + "More than ten invitations from one caller within the hour (`meta.errorCode: http.rate_limited`, the status default).", content: { "application/json": { schema: errorEnvelope } }, }, }, diff --git a/src/lib/openapi/routes/auth.ts b/src/lib/openapi/routes/auth.ts index e99193678..71e0a5424 100644 --- a/src/lib/openapi/routes/auth.ts +++ b/src/lib/openapi/routes/auth.ts @@ -2158,7 +2158,7 @@ export const authPaths: NonNullable = { }, "403": { description: - "The operator has switched the API off instance-wide (`AppSettings.apiGlobal`). No `meta.errorCode`; nothing was revoked. The sibling list is deliberately not gated this way — a token stays live while the switch is off, so its owner can still see it here even though this call will not kill it.", + "The operator has switched the API off instance-wide (`AppSettings.apiGlobal`). `meta.errorCode` is the status default `http.forbidden`; nothing was revoked. The sibling list is deliberately not gated this way — a token stays live while the switch is off, so its owner can still see it here even though this call will not kill it.", content: { "application/json": { schema: errorEnvelope } }, }, "404": { diff --git a/src/lib/openapi/routes/profile.ts b/src/lib/openapi/routes/profile.ts index 665b8a23e..8c55acee5 100644 --- a/src/lib/openapi/routes/profile.ts +++ b/src/lib/openapi/routes/profile.ts @@ -1426,7 +1426,7 @@ export const profilePaths: NonNullable = { // quietly omits the refusal a person actually hits. "422": { description: - "The write was refused and nothing was stored. `meta.errorCode` distinguishes the cases: `health_score_config.too_narrow` — the selection cannot produce a score (`meta.reason` is `no_pillars_selected`, and the `error` string is the sentence to show the person). Since v1.38 the empty selection is the ONLY one refused: two areas of health, or one, now save and produce a score that labels its own breadth, so a client must not pre-empt the write with a three-area rule of its own. `health_score_config.invalid` — the body failed validation, with the per-issue list under `details.issues`; `invalid_base_updated_at` — `baseUpdatedAt` was present but unparseable, `null` included, which is rejected rather than falling back to the unconditional write. A body that is not JSON at all returns the bare message `health-score-config.body.invalid_json` with no errorCode.", + "The write was refused and nothing was stored. `meta.errorCode` distinguishes the cases: `health_score_config.too_narrow` — the selection cannot produce a score (`meta.reason` is `no_pillars_selected`, and the `error` string is the sentence to show the person). Since v1.38 the empty selection is the ONLY one refused: two areas of health, or one, now save and produce a score that labels its own breadth, so a client must not pre-empt the write with a three-area rule of its own. `health_score_config.invalid` — the body failed validation, with the per-issue list under `details.issues`; `invalid_base_updated_at` — `baseUpdatedAt` was present but unparseable, `null` included, which is rejected rather than falling back to the unconditional write. A body that is not JSON at all returns the bare message `health-score-config.body.invalid_json` with the status default `meta.errorCode: http.invalid`.", content: { "application/json": { schema: errorEnvelope } }, }, }, diff --git a/src/lib/sharing/grant-view.ts b/src/lib/sharing/grant-view.ts index 7eb36b4d1..0f3c97337 100644 --- a/src/lib/sharing/grant-view.ts +++ b/src/lib/sharing/grant-view.ts @@ -112,3 +112,10 @@ export function toGrantView( revokedBy: grant.revokedBy, }; } + +/** + * The 404 both invitation routes answer when no account carries the typed + * name or e-mail. Its own code, because each route has a second 404 that + * needs a different next step. + */ +export const INVITE_NO_ACCOUNT_CODE = "sharing.invite.no_account"; diff --git a/tests/integration/managed-profile-surface.test.ts b/tests/integration/managed-profile-surface.test.ts index 1758a5a52..b135ce059 100644 --- a/tests/integration/managed-profile-surface.test.ts +++ b/tests/integration/managed-profile-surface.test.ts @@ -718,16 +718,17 @@ describe("every call the managed-profile card can make", () => { ); }); - it("answers an unknown identifier with a 404 carrying NO error code", async () => { - // The asymmetry the client has to branch on status for. Four arms of this - // route publish a code; this one and the rate limit do not. + it("answers an unknown identifier with a 404 that names itself", async () => { + // This route has a second 404 (`managed_profile.not_found`) with a + // different next step, so the unknown name carries its own code and a + // client never has to tell the two apart by status. const { creator, profile } = await profileWithPendingSecondGuardian(); signIn(creator); const response = await inviteGuardian(profile.id, { identifier: "nobody-here", }); expect(response.status).toBe(404); - expect(await errorCodeOf(response)).toBeUndefined(); + expect(await errorCodeOf(response)).toBe("sharing.invite.no_account"); }); it("refuses an invitation from somebody who is not a Guardian", async () => { @@ -947,9 +948,9 @@ describe("how many profiles one account may create in an hour", () => { timezone: "UTC", }); expect(refused.status).toBe(429); - // No error code, like every other rate-limit refusal in this family — the - // client branches on status, which is recorded in the findings. - expect(await errorCodeOf(refused)).toBeUndefined(); + // The status default. A 429 has one meaning in this family (wait), so the + // client branches on the status and needs no code of its own. + expect(await errorCodeOf(refused)).toBe("http.rate_limited"); // And nothing was created by the refused call. expect( await getPrismaClient().user.count({ diff --git a/tests/integration/sharing-lifecycle.test.ts b/tests/integration/sharing-lifecycle.test.ts index 1aa9574a0..c9a1cd724 100644 --- a/tests/integration/sharing-lifecycle.test.ts +++ b/tests/integration/sharing-lifecycle.test.ts @@ -491,6 +491,8 @@ describe("who may do what to a grant", () => { const response = await invite("nobody-here@example.test"); expect(response.status).toBe(404); + const body = (await response.json()) as { meta?: { errorCode?: string } }; + expect(body.meta?.errorCode).toBe("sharing.invite.no_account"); expect(await getPrismaClient().accountGrant.count()).toBe(0); }); }); From 2f7f04ecab31d93cb71962885f43dc1da5588e94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 09:48:23 +0200 Subject: [PATCH 4/9] fix(auth): answer a malformed session cookie as no session The session lookup turned every query error into a 503, so a cookie holding a NUL byte reached the legacy id path, Postgres refused the value, and an anonymous caller could produce server errors at will. A cookie that cannot be a secret or a legacy id is now answered as no session before the database is asked, and only an unreachable store (connection errors, timeouts, an exhausted pool) is a 503. Any other query error is answered like a value that matches nothing. --- .../session-locale-reconcile.test.ts | 7 +- src/lib/auth/__tests__/session.test.ts | 79 ++++++++++++++----- src/lib/auth/session.ts | 36 ++++++++- src/lib/prisma-errors.ts | 24 ++++++ 4 files changed, 124 insertions(+), 22 deletions(-) diff --git a/src/lib/auth/__tests__/session-locale-reconcile.test.ts b/src/lib/auth/__tests__/session-locale-reconcile.test.ts index 4766eaf1f..01b356880 100644 --- a/src/lib/auth/__tests__/session-locale-reconcile.test.ts +++ b/src/lib/auth/__tests__/session-locale-reconcile.test.ts @@ -66,14 +66,17 @@ vi.mock("next/headers", () => ({ import { prisma } from "@/lib/db"; import { getSession } from "../session"; +/** A cookie value in the shape `mintSessionSecret` produces. */ +const SECRET = `hls_${"a".repeat(64)}`; + const THIRTY_DAYS_MS = 30 * 24 * 60 * 60 * 1000; function seedSession(userLocale: string | null) { - cookieState.sessionId = "hls_raw-secret"; + cookieState.sessionId = SECRET; store.row = { id: "sess-1", userId: "user-1", - tokenHash: "hash:hls_raw-secret", + tokenHash: `hash:${SECRET}`, // Far enough out that the sliding-expiry refresh stays quiet. expiresAt: new Date(Date.now() + THIRTY_DAYS_MS), lastActiveAt: new Date(), diff --git a/src/lib/auth/__tests__/session.test.ts b/src/lib/auth/__tests__/session.test.ts index 91ec94f5f..3646fbda9 100644 --- a/src/lib/auth/__tests__/session.test.ts +++ b/src/lib/auth/__tests__/session.test.ts @@ -91,6 +91,11 @@ import { } from "../session"; import { SessionStoreUnavailableError } from "@/lib/api-errors"; +/** A cookie value in the shape `mintSessionSecret` produces. */ +const SECRET = `hls_${"a".repeat(64)}`; +const FORGED_SECRET = `hls_${"b".repeat(64)}`; +const UNKNOWN_SECRET = `hls_${"c".repeat(64)}`; + const THIRTY_DAYS_MS = 30 * 24 * 60 * 60 * 1000; function seed(row: Partial & { id: string }): SessionRow { @@ -155,21 +160,21 @@ describe("createSession", () => { describe("getSession — cookie resolution", () => { it("resolves a modern cookie by its hash, not by primary key", async () => { - const row = seed({ id: "sess_cuid_1", tokenHash: "hash:hls_abc" }); - cookieState.sessionId = "hls_abc"; + const row = seed({ id: "sess_cuid_1", tokenHash: `hash:${SECRET}` }); + cookieState.sessionId = SECRET; const result = await getSession(); expect(result?.session.id).toBe(row.id); // The lookup went through the hash column. expect(prisma.session.findUnique).toHaveBeenCalledWith( - expect.objectContaining({ where: { tokenHash: "hash:hls_abc" } }), + expect.objectContaining({ where: { tokenHash: `hash:${SECRET}` } }), ); }); it("refuses a modern-shaped cookie whose hash matches nothing", async () => { seed({ id: "sess_cuid_1", tokenHash: "hash:hls_real" }); - cookieState.sessionId = "hls_forged"; + cookieState.sessionId = FORGED_SECRET; await expect(getSession()).resolves.toBeNull(); }); @@ -202,10 +207,10 @@ describe("getSession — sliding expiry", () => { it("extends a modern session and re-emits the same secret", async () => { seed({ id: "sess_cuid_1", - tokenHash: "hash:hls_abc", + tokenHash: `hash:${SECRET}`, expiresAt: new Date(Date.now() + 2 * 24 * 60 * 60 * 1000), }); - cookieState.sessionId = "hls_abc"; + cookieState.sessionId = SECRET; await getSession(); @@ -215,7 +220,7 @@ describe("getSession — sliding expiry", () => { const cookieValue = cookieState.set.mock.calls.find( (c) => c[0] === "healthlog_session", )![1]; - expect(cookieValue).toBe("hls_abc"); + expect(cookieValue).toBe(SECRET); }); it("withholds the extension from a legacy session so the id path drains", async () => { @@ -270,7 +275,7 @@ describe("a session lookup that fails is not a missing session", () => { } it("getSession raises a 503 with a retryable code", async () => { - cookieState.sessionId = "hls_abc"; + cookieState.sessionId = SECRET; failLookup(); const err = await getSession().catch((e: unknown) => e); expect(err).toBeInstanceOf(SessionStoreUnavailableError); @@ -284,26 +289,64 @@ describe("a session lookup that fails is not a missing session", () => { it("validateSessionFromCookieValue raises instead of answering null", async () => { failLookup(); - await expect( - validateSessionFromCookieValue("hls_abc"), - ).rejects.toBeInstanceOf(SessionStoreUnavailableError); + await expect(validateSessionFromCookieValue(SECRET)).rejects.toBeInstanceOf( + SessionStoreUnavailableError, + ); }); it("validateSessionWithCreatedAt raises instead of answering null", async () => { failLookup(); - await expect( - validateSessionWithCreatedAt("hls_abc"), - ).rejects.toBeInstanceOf(SessionStoreUnavailableError); + await expect(validateSessionWithCreatedAt(SECRET)).rejects.toBeInstanceOf( + SessionStoreUnavailableError, + ); + }); + + it("raises for a pool that had no connection free (P2024)", async () => { + vi.mocked(prisma.session.findUnique).mockRejectedValueOnce( + Object.assign(new Error("connection pool exhausted"), { + code: "P2024", + }) as never, + ); + await expect(validateSessionFromCookieValue(SECRET)).rejects.toBeInstanceOf( + SessionStoreUnavailableError, + ); + }); + + it("answers null, not 503, when the query refused the value itself", async () => { + // Postgres 22021: a byte sequence it will not accept in text. Nothing is + // wrong with the store; a 503 here would let any anonymous caller produce + // server errors on demand. + vi.mocked(prisma.session.findUnique).mockRejectedValueOnce( + Object.assign(new Error("invalid byte sequence for encoding"), { + code: "P2010", + meta: { driverAdapterError: { cause: { originalCode: "22021" } } }, + }) as never, + ); + await expect(validateSessionFromCookieValue("sess_legacy")).resolves.toBe( + null, + ); + }); + + it.each([ + ["a NUL byte", "\u0000"], + ["a percent-encoded NUL", "%00"], + ["a short secret", "hls_abc"], + ["a secret with non-hex", `hls_${"z".repeat(64)}`], + ["an overlong legacy id", "c".repeat(65)], + ])("never asks the database about %s", async (_name, value) => { + vi.mocked(prisma.session.findUnique).mockClear(); + await expect(validateSessionFromCookieValue(value)).resolves.toBe(null); + expect(prisma.session.findUnique).not.toHaveBeenCalled(); }); it("still answers null for a cookie that matches nothing", async () => { - await expect(validateSessionFromCookieValue("hls_nope")).resolves.toBe( + await expect(validateSessionFromCookieValue(UNKNOWN_SECRET)).resolves.toBe( null, ); }); it("logout still clears the cookie when the lookup fails", async () => { - cookieState.sessionId = "hls_abc"; + cookieState.sessionId = SECRET; failLookup(); await expect(destroySession()).resolves.toBeUndefined(); expect(cookieState.delete).toHaveBeenCalledWith("healthlog_session"); @@ -314,8 +357,8 @@ describe("destroySession", () => { it("deletes the row the secret resolves to, not the cookie value", async () => { // The cookie is no longer the primary key, so a logout that deletes by // cookie value would no-op and leave the session alive. - seed({ id: "sess_cuid_1", tokenHash: "hash:hls_abc" }); - cookieState.sessionId = "hls_abc"; + seed({ id: "sess_cuid_1", tokenHash: `hash:${SECRET}` }); + cookieState.sessionId = SECRET; await destroySession(); diff --git a/src/lib/auth/session.ts b/src/lib/auth/session.ts index 5d2d53640..d779fa9c5 100644 --- a/src/lib/auth/session.ts +++ b/src/lib/auth/session.ts @@ -6,7 +6,7 @@ import { hashToken } from "@/lib/auth/hmac"; import { ensureDbCompatibility } from "@/lib/db-compat"; import { getEvent } from "@/lib/logging/context"; import { shouldEmitSecureCookie } from "@/lib/auth/secure-cookie"; -import { isP2025 } from "@/lib/prisma-errors"; +import { isDatabaseUnavailable, isP2025 } from "@/lib/prisma-errors"; import { SessionStoreUnavailableError } from "@/lib/api-errors"; import { locales, type Locale } from "@/lib/i18n/config"; import { LOCALE_COOKIE, setLocaleCookie } from "@/lib/i18n/locale-cookie"; @@ -41,6 +41,19 @@ function mintSessionSecret(): string { return `${SESSION_TOKEN_PREFIX}${randomBytes(SESSION_SECRET_BYTES).toString("hex")}`; } +/** Exactly what `mintSessionSecret` produces. */ +const SESSION_SECRET_SHAPE = new RegExp( + `^${SESSION_TOKEN_PREFIX}[0-9a-f]{${SESSION_SECRET_BYTES * 2}}$`, +); + +/** + * What a legacy cookie (a `Session.id`, a cuid) can look like. Deliberately a + * character class rather than the cuid grammar: it exists to keep a value that + * cannot be an id away from the query, where a NUL byte or invalid UTF-8 makes + * Postgres raise instead of finding nothing. + */ +const LEGACY_SESSION_ID_SHAPE = /^[A-Za-z0-9_-]{1,64}$/; + /** * Resolve the session row a cookie value refers to, or null. * @@ -74,6 +87,14 @@ function mintSessionSecret(): string { * login from the deploy onward is on a secret immediately. */ async function findSessionByCookie(cookieValue: string) { + // A malformed value matches no session: it is answered as one, before the + // database is asked, so an anonymous caller cannot turn a crafted cookie into + // a query error. + if (cookieValue.startsWith(SESSION_TOKEN_PREFIX)) { + if (!SESSION_SECRET_SHAPE.test(cookieValue)) return null; + } else if (!LEGACY_SESSION_ID_SHAPE.test(cookieValue)) { + return null; + } try { if (cookieValue.startsWith(SESSION_TOKEN_PREFIX)) { return await prisma.session.findUnique({ @@ -94,7 +115,18 @@ async function findSessionByCookie(cookieValue: string) { // the callers, so a database restart answered 401 and every client // signed its person out. It is a 503 now, with a code a client can retry // on (`auth.session.unavailable`). - throw new SessionStoreUnavailableError(error); + // + // Only for a store that could not be reached. Any other query error says + // something about the value asked for, not about the store, and is + // answered like a value that matches nothing: a 503 there would let any + // anonymous caller produce server errors at will. + if (isDatabaseUnavailable(error)) { + throw new SessionStoreUnavailableError(error); + } + getEvent()?.addWarning( + `session lookup refused the value: ${error instanceof Error ? error.name : "unknown"}`, + ); + return null; } } diff --git a/src/lib/prisma-errors.ts b/src/lib/prisma-errors.ts index 67745ad7a..d5612ea8e 100644 --- a/src/lib/prisma-errors.ts +++ b/src/lib/prisma-errors.ts @@ -13,6 +13,8 @@ * unique-constraint code. */ +import { classifyRestoreFailure } from "@/lib/export/restore-failure-cause"; + /** Narrow a thrown Prisma error to a specific known-request code. */ function hasPrismaCode(err: unknown, code: string): boolean { return ( @@ -37,3 +39,25 @@ export function isP2002(err: unknown): boolean { export function isP2025(err: unknown): boolean { return hasPrismaCode(err, "P2025"); } + +/** + * Whether a query failed because the database could not be reached, as opposed + * to refusing what it was asked. The first is worth a retry and is a 503; the + * second is an answer about the input. + * + * Reads the same signals the restore classifier does (`connection` and + * `timeout`: a connection SQLSTATE, a server shutting down, a statement or + * idle timeout, Prisma's P1001 / P1002 / P1017 / P2028, a terminated + * connection), plus the two that only a pooled request meets: P2024, no + * connection free in the pool within its timeout, and P1008, an operation that + * timed out. + */ +export function isDatabaseUnavailable(err: unknown): boolean { + if (hasPrismaCode(err, "P2024") || hasPrismaCode(err, "P1008")) return true; + const { cause } = classifyRestoreFailure(err); + if (cause === "connection" || cause === "timeout") return true; + const message = err instanceof Error ? err.message : ""; + return /ECONNREFUSED|ECONNRESET|ETIMEDOUT|Can't reach database server|timed out/i.test( + message, + ); +} From 6e54f3390171afee5aaa699b637a5c2cffefffcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 09:53:31 +0200 Subject: [PATCH 5/9] refactor(i18n): ask the catalogue for a sentence in one helper `t()` answers a missing key with the key, so "does this code have a sentence" was a comparison written inline at every site that wanted a code's sentence or a fallback, twenty-odd times in several spellings. translateOrNull, translateCodeOrNull and translateCode in src/lib/i18n/translate-code.ts now hold that probe, every site uses them, and a guard refuses the inline comparison anywhere else. --- src/__tests__/translate-probe-guard.test.ts | 96 +++++++++++++++++++ src/app/insights/workouts/page-client.tsx | 5 +- .../admin/notification-test-failures.ts | 11 +-- src/components/charts/health-chart.tsx | 4 +- .../dashboard/recent-workouts-tile.tsx | 4 +- .../insights/coach-panel/assumption-line.tsx | 4 +- .../insights/coach-panel/chat-bubble.tsx | 21 ++-- .../coach-panel/coach-conversation.tsx | 6 +- .../insights/coach-panel/message-thread.tsx | 4 +- .../insights/coach-panel/turn-activity.tsx | 9 +- .../insights/workout-detail/header.tsx | 5 +- src/components/insights/workout-list.tsx | 4 +- src/components/links/related-records.tsx | 6 +- .../vorsorge-dashboard-card.tsx | 4 +- .../vorsorge-section.tsx | 8 +- .../vorsorge-settings.tsx | 4 +- .../onboarding/baseline-form-utils.ts | 8 +- .../settings/account-section/index.tsx | 6 +- .../integrations/integration-actions.tsx | 12 ++- src/components/settings/ntfy-card.tsx | 13 +-- .../settings/test-connection-button.tsx | 14 +-- src/components/settings/webhook-card.tsx | 13 +-- .../__tests__/no-template-key-guard.test.ts | 8 +- .../timeline/readiness-inventory.tsx | 6 +- src/lib/ai/coach/results/projections.ts | 4 +- src/lib/api/localized-error.ts | 16 ++-- src/lib/daily/load-digest.ts | 4 +- src/lib/daily/today-overview.ts | 4 +- src/lib/dashboard/snapshot.ts | 6 +- src/lib/i18n/__tests__/translate-code.test.ts | 45 +++++++++ src/lib/i18n/translate-code.ts | 47 +++++++++ src/lib/insights/correlation-discovery.ts | 9 +- 32 files changed, 291 insertions(+), 119 deletions(-) create mode 100644 src/__tests__/translate-probe-guard.test.ts create mode 100644 src/lib/i18n/__tests__/translate-code.test.ts create mode 100644 src/lib/i18n/translate-code.ts diff --git a/src/__tests__/translate-probe-guard.test.ts b/src/__tests__/translate-probe-guard.test.ts new file mode 100644 index 000000000..19896de8a --- /dev/null +++ b/src/__tests__/translate-probe-guard.test.ts @@ -0,0 +1,96 @@ +/** + * Whether the catalogue has a sentence is asked in one place. + * + * `t()` answers a missing key with the key itself, so "does this code have a + * sentence" is a comparison of the answer with the question. Written inline it + * was copied once per surface that wanted a code's sentence or else a + * fallback. It lives in `src/lib/i18n/translate-code.ts` now + * (`translateOrNull`, `translateCode`), and this guard refuses the comparison + * anywhere else under `src/`. + * + * Two shapes are read: a translation held in a variable and compared with the + * key it was asked for (`const v = t(key); v === key`), and the inline form + * (`t(k) !== k`). The helper itself must match, so a matcher that stops + * finding the shape fails instead of passing over nothing. + */ +import { readdirSync, readFileSync } from "node:fs"; +import { join, relative } from "node:path"; +import { describe, expect, it } from "vitest"; + +const SRC = join(__dirname, ".."); +const HELPER = "lib/i18n/translate-code.ts"; + +function walk(dir: string, out: string[] = []): 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 (/\.tsx?$/.test(entry.name) && !entry.name.includes(".test.")) + out.push(path); + } + return out; +} + +function escape(text: string): string { + return text.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +/** The probes in one source text, one line each. */ +function probes(source: string): string[] { + const found: string[] = []; + // Inline: `t(K) !== K` / `t(K, p) === K`, K an identifier or a template. + const inline = + /\bt\(\s*(`[^`]*`|[A-Za-z_$][\w$]*)\s*(?:,[^()]*)?\)\s*[!=]==\s*(`[^`]*`|[A-Za-z_$][\w$]*)/g; + for (const m of source.matchAll(inline)) { + if (m[1] === m[2]) found.push(m[0]); + } + // Held: `const v = t(K…)` (or `K ? t(K) : null`) and later `v === K` or + // `K !== v`. + const held = + /\b(?:const|let)\s+([A-Za-z_$][\w$]*)\s*=\s*(?:[\w$.]+\s*\?\s*)?t\(\s*([A-Za-z_$][\w$]*)\b/g; + for (const m of source.matchAll(held)) { + const [, value, key] = m; + const compare = new RegExp( + `\\b${escape(value)}\\s*[!=]==\\s*${escape(key)}\\b|\\b${escape(key)}\\s*[!=]==\\s*${escape(value)}\\b`, + ); + if (compare.test(source.slice(m.index))) found.push(m[0]); + } + return found; +} + +const files = walk(SRC).map((file) => ({ + file: relative(SRC, file), + source: readFileSync(file, "utf8"), +})); + +describe("the catalogue probe lives in one helper", () => { + it("recognises both shapes", () => { + expect(probes("const s = t(key);\nreturn s === key ? a : s;")).toHaveLength( + 1, + ); + expect(probes("if (t(`x.${c}`) !== `x.${c}`) go();")).toHaveLength(1); + expect( + probes("const label = t(key, params); if (key !== label) {}"), + ).toHaveLength(1); + expect( + probes("const v = key ? t(key) : null; return v && v !== key ? v : f;"), + ).toHaveLength(1); + expect(probes("const s = t(key); return s;")).toEqual([]); + expect(probes("if (row.key === key) {}")).toEqual([]); + }); + + it("finds the probe in the helper", () => { + const helper = files.find(({ file }) => file === HELPER); + expect(helper).toBeDefined(); + expect(probes(helper!.source).length).toBeGreaterThan(0); + }); + + it("finds it nowhere else", () => { + const offenders = files + .filter(({ file }) => file !== HELPER) + .flatMap(({ file, source }) => + probes(source).map((p) => `${file}: ${p}`), + ); + expect(offenders).toEqual([]); + }); +}); diff --git a/src/app/insights/workouts/page-client.tsx b/src/app/insights/workouts/page-client.tsx index d1592e047..0cf9cdd60 100644 --- a/src/app/insights/workouts/page-client.tsx +++ b/src/app/insights/workouts/page-client.tsx @@ -19,6 +19,7 @@ import { useAuth } from "@/hooks/use-auth"; import { useRecordCapabilities } from "@/hooks/use-record-capabilities"; import { canLogWorkout } from "@/lib/workouts/manual-entry"; import { QueryErrorCard } from "@/components/ui/query-error-card"; +import { translateOrNull } from "@/lib/i18n/translate-code"; /** * v1.4.32 — `/insights/workouts`. @@ -82,9 +83,7 @@ export default function InsightsWorkoutsPageClient() { const sportLabel = (sport: string) => { // A sport enum the bundle has no copy for renders as its canonical // string rather than a raw key — same fallback the list rows use. - const key = `insights.workouts.sport.${sport}`; - const label = t(key); - return label === key ? sport : label; + return translateOrNull(t, `insights.workouts.sport.${sport}`) ?? sport; }; return ( diff --git a/src/components/admin/notification-test-failures.ts b/src/components/admin/notification-test-failures.ts index caf578b50..9cf7b6726 100644 --- a/src/components/admin/notification-test-failures.ts +++ b/src/components/admin/notification-test-failures.ts @@ -2,6 +2,7 @@ import { CHANNEL_TYPE_LABELS, type ChannelType, } from "@/lib/notifications/types"; +import { translateCodeOrNull } from "@/lib/i18n/translate-code"; /** One channel's row in the admin notification test response. */ export interface ChannelTestResult { @@ -27,14 +28,10 @@ export function describeChannelTestFailures( .filter((r) => !r.success) .map((r) => { const label = CHANNEL_TYPE_LABELS[r.channel as ChannelType] ?? r.channel; - const key = r.errorCode - ? `settings.testConnection.errors.${r.errorCode}` - : null; - const translated = key ? t(key) : null; const cause = - translated && translated !== key - ? translated - : (r.error ?? t("admin.notificationTestFailed")); + translateCodeOrNull(t, "settings.testConnection.errors", r.errorCode) ?? + r.error ?? + t("admin.notificationTestFailed"); const code = typeof r.upstreamStatus === "number" ? ` (HTTP ${r.upstreamStatus})` diff --git a/src/components/charts/health-chart.tsx b/src/components/charts/health-chart.tsx index 640c8db11..fdbe42f25 100644 --- a/src/components/charts/health-chart.tsx +++ b/src/components/charts/health-chart.tsx @@ -89,6 +89,7 @@ import { DEFAULT_CHART_RANGE, rangeWindowDays, } from "./chart-range-tabs"; +import { translateOrNull } from "@/lib/i18n/translate-code"; // The range tabs (7 / 30 / 90 / All, a calendar-day window ending now) and // the "All" tab's ten-year fetch span live with the shared @@ -399,8 +400,7 @@ function getTypeLabel( const key = BASE_TYPE_LABEL_KEYS[type]; if (key) return t(key); const measurementKey = measurementLabelKey(type); - const label = t(measurementKey); - return label === measurementKey ? type : label; + return translateOrNull(t, measurementKey) ?? type; } /** diff --git a/src/components/dashboard/recent-workouts-tile.tsx b/src/components/dashboard/recent-workouts-tile.tsx index 0b75c76f6..a9770c5b4 100644 --- a/src/components/dashboard/recent-workouts-tile.tsx +++ b/src/components/dashboard/recent-workouts-tile.tsx @@ -12,6 +12,7 @@ import { TileHeader } from "@/components/insights/tile-header"; import { Skeleton } from "@/components/ui/skeleton"; import { QueryErrorCard } from "@/components/ui/query-error-card"; import { iconForSport } from "@/lib/workouts/sport-icons"; +import { translateOrNull } from "@/lib/i18n/translate-code"; /** * v1.4.32 — ``. @@ -122,9 +123,8 @@ export function RecentWorkoutsTile() {
    {workouts.map((workout) => { const sportLabelKey = `insights.workouts.sport.${workout.sportType}`; - const sportLabel = t(sportLabelKey); const sportName = - sportLabel === sportLabelKey ? workout.sportType : sportLabel; + translateOrNull(t, sportLabelKey) ?? workout.sportType; return (
  • string, option: CoachAssumption["value"], ): string { - const local = t(option.labelKey); - return local === option.labelKey ? option.label : local; + return translateOrNull(t, option.labelKey) ?? option.label; } export interface CoachAssumptionLineProps { diff --git a/src/components/insights/coach-panel/chat-bubble.tsx b/src/components/insights/coach-panel/chat-bubble.tsx index 96125f191..0c0cec5b3 100644 --- a/src/components/insights/coach-panel/chat-bubble.tsx +++ b/src/components/insights/coach-panel/chat-bubble.tsx @@ -38,6 +38,7 @@ import type { CoachMemoryNote as CoachMemoryNoteFrame, CoachProvenanceMetric, } from "@/lib/ai/coach/types"; +import { translateOrNull } from "@/lib/i18n/translate-code"; /** * Per-message bubble renderer for the Coach thread, split out of @@ -239,8 +240,8 @@ function coachChartTitle( ): string { const override = CHART_TITLE_KEY_OVERRIDE[metric]; if (override) { - const resolved = t(override); - if (resolved !== override) return resolved; + const resolved = translateOrNull(t, override); + if (resolved !== null) return resolved; } const camel = metric .toLowerCase() @@ -250,8 +251,7 @@ function coachChartTitle( ) .join(""); const key = `measurements.type${camel.charAt(0).toUpperCase()}${camel.slice(1)}`; - const resolved = t(key); - return resolved === key ? metric.replace(/_/g, " ").toLowerCase() : resolved; + return translateOrNull(t, key) ?? metric.replace(/_/g, " ").toLowerCase(); } interface ChatBubbleProps { @@ -564,16 +564,11 @@ function ChatBubbleImpl({ // provider load (retry in ~5 min). Codes that have no dedicated // translation fall back to the generic provider-unavailable copy. const errorKey = errorCode ? errorCodeToI18nKey(errorCode) : null; - const errorMessage = errorKey ? t(errorKey, {}) : null; - // When a translated message comes back unchanged (i.e. key missing) - // we fall back to a generic provider error string so the bubble - // doesn't surface raw `coach.http.503` text to the user. + // A code without a sentence falls back to the generic provider error, so + // the bubble never surfaces raw `coach.http.503` text to the user. const safeError = - errorMessage && errorMessage !== errorKey - ? errorMessage - : errorCode - ? t("insights.coach.errorProvider") - : null; + (errorKey ? translateOrNull(t, errorKey) : null) ?? + (errorCode ? t("insights.coach.errorProvider") : null); const method = metricSource?.method ?? null; diff --git a/src/components/insights/coach-panel/coach-conversation.tsx b/src/components/insights/coach-panel/coach-conversation.tsx index f758f694c..733e78050 100644 --- a/src/components/insights/coach-panel/coach-conversation.tsx +++ b/src/components/insights/coach-panel/coach-conversation.tsx @@ -67,6 +67,7 @@ import { useDetachCoachDocument, useSendCoachMessage, } from "./use-coach"; +import { translateOrNull } from "@/lib/i18n/translate-code"; /** * v1.12.0 (Coach v2 #6) — shared chat surface for both the Coach drawer @@ -987,11 +988,8 @@ export function CoachConversation({ // from a metric surface names the metric there until its first turn. const a2Metric = launchScope?.metric ?? null; - // `t()` returns the key string itself when a key is missing, so the only - // reliable "is this key defined" signal is `t(key) !== key`. function tOrNull(key: string): string | null { - const resolved = t(key); - return resolved === key ? null : resolved; + return translateOrNull(t, key); } // The resolved metric label: prefers a per-source i18n key, falls back to diff --git a/src/components/insights/coach-panel/message-thread.tsx b/src/components/insights/coach-panel/message-thread.tsx index e274a11ef..2ea6f4301 100644 --- a/src/components/insights/coach-panel/message-thread.tsx +++ b/src/components/insights/coach-panel/message-thread.tsx @@ -42,6 +42,7 @@ import { COACH_MEMORY_KEYS, COACH_PLAN_KEYS, } from "@/lib/ai/coach/memory/shared"; +import { translateOrNull } from "@/lib/i18n/translate-code"; // v1.28.26 file-size split (pure code motion): the bubble renderer + // per-message actions live in `chat-bubble.tsx`, the read-aloud stack in @@ -215,8 +216,7 @@ export function decisionReplies( ] : choices; for (const choice of ordered.slice(0, 4)) { - const local = t(choice.labelKey); - const label = local === choice.labelKey ? choice.label : local; + const label = translateOrNull(t, choice.labelKey) ?? choice.label; out.push({ id: `clarify-${choice.id}`, label, diff --git a/src/components/insights/coach-panel/turn-activity.tsx b/src/components/insights/coach-panel/turn-activity.tsx index 081ecf30c..b22b4e1ac 100644 --- a/src/components/insights/coach-panel/turn-activity.tsx +++ b/src/components/insights/coach-panel/turn-activity.tsx @@ -82,6 +82,7 @@ import type { import { COACH_FOCUS_RING } from "./focus-ring"; import { CoachMethodLine } from "./method-line"; +import { translateOrNull } from "@/lib/i18n/translate-code"; /** The shortest gap between two screen-reader announcements. */ export const ANNOUNCE_THROTTLE_MS = 1500; @@ -106,8 +107,7 @@ function tOr( fallback: string, params?: Record, ) { - const value = t(key, params); - return value === key ? fallback : value; + return translateOrNull(t, key, params) ?? fallback; } // ── Steps (messages saved before v1.41, and the fetch rows) ──────────────── @@ -180,9 +180,8 @@ export function legacyAreaLabels( const out: string[] = []; for (const area of areas ?? []) { if (area === "general") continue; - const key = `insights.coach.metric.${area}`; - const label = t(key); - if (label === key || out.includes(label)) continue; + const label = translateOrNull(t, `insights.coach.metric.${area}`); + if (label === null || out.includes(label)) continue; out.push(label); } return out; diff --git a/src/components/insights/workout-detail/header.tsx b/src/components/insights/workout-detail/header.tsx index c4b399229..1082b1788 100644 --- a/src/components/insights/workout-detail/header.tsx +++ b/src/components/insights/workout-detail/header.tsx @@ -7,6 +7,7 @@ import type { WorkoutDetailPayload } from "@/hooks/use-workouts"; import { useUnitDisplay } from "@/hooks/use-unit-display"; import { formatDuration, formatDistance } from "./format"; +import { translateOrNull } from "@/lib/i18n/translate-code"; /** * Hero header — the at-a-glance verdict row. Sport icon badge, source @@ -34,9 +35,7 @@ export function WorkoutDetailHeader({ workout }: WorkoutDetailHeaderProps) { const fmt = useFormatters(); const { preference } = useUnitDisplay(); const sportLabelKey = `insights.workouts.sport.${workout.sportType}`; - const sportLabel = t(sportLabelKey); - const sportName = - sportLabel === sportLabelKey ? workout.sportType : sportLabel; + const sportName = translateOrNull(t, sportLabelKey) ?? workout.sportType; return (
    { const Icon = iconForSport(workout.sportType); const sportLabelKey = `insights.workouts.sport.${workout.sportType}`; - const sportLabel = t(sportLabelKey); // Translation keys missing from one locale fall back to the // canonical sport-type string so a new HK enum value never // renders the raw `insights.workouts.sport.x` placeholder. const sportName = - sportLabel === sportLabelKey ? workout.sportType : sportLabel; + translateOrNull(t, sportLabelKey) ?? workout.sportType; return ( // v1.42 — the row opens the workout and its date opens the day: diff --git a/src/components/links/related-records.tsx b/src/components/links/related-records.tsx index 010cfacfa..aadcfd923 100644 --- a/src/components/links/related-records.tsx +++ b/src/components/links/related-records.tsx @@ -63,6 +63,7 @@ import type { EncounterDTO } from "@/lib/encounters/dto"; import type { IllnessEpisodeDTO } from "@/lib/illness/dto"; import type { RelatedLink } from "@/lib/links/related"; import type { EncounterKind } from "@/generated/prisma/client"; +import { translateOrNull } from "@/lib/i18n/translate-code"; type Translate = ReturnType["t"]; type RelatedKind = RelatedLink["kind"]; @@ -114,9 +115,8 @@ function rowLabel(t: Translate, row: RelatedLink): string { // A dose without a written name arrives as its catalogue slug ("tetanus"), // named here from the reader's bundle the way the dose picker names it. if (row.kind === "vaccination" && /^[a-z0-9_-]+$/.test(row.label)) { - const key = `vaccinations.catalog.${row.label}`; - const name = t(key); - if (name !== key) return name; + const name = translateOrNull(t, `vaccinations.catalog.${row.label}`); + if (name !== null) return name; } return row.label; } diff --git a/src/components/measurement-reminders/vorsorge-dashboard-card.tsx b/src/components/measurement-reminders/vorsorge-dashboard-card.tsx index a586e60c3..e0552eeaa 100644 --- a/src/components/measurement-reminders/vorsorge-dashboard-card.tsx +++ b/src/components/measurement-reminders/vorsorge-dashboard-card.tsx @@ -44,6 +44,7 @@ import { useMeasurementReminderMutations, type MeasurementReminder, } from "@/hooks/use-measurement-reminders"; +import { translateOrNull } from "@/lib/i18n/translate-code"; const SUMMARY_LIMIT = 3; @@ -61,8 +62,7 @@ function resolveLabel( t: (key: string) => string, ): string { if (reminder.origin === "COACH") { - const translated = t(reminder.label); - return translated === reminder.label ? reminder.label : translated; + return translateOrNull(t, reminder.label) ?? reminder.label; } return reminder.label; } diff --git a/src/components/measurement-reminders/vorsorge-section.tsx b/src/components/measurement-reminders/vorsorge-section.tsx index 7407ef3df..d813ebbcd 100644 --- a/src/components/measurement-reminders/vorsorge-section.tsx +++ b/src/components/measurement-reminders/vorsorge-section.tsx @@ -103,6 +103,7 @@ import { shiftDateKey, userDayKey, } from "@/lib/tz/format"; +import { translateOrNull } from "@/lib/i18n/translate-code"; // v1.18.1 — the active-measurement set, grouped so the dropdown // doesn't read as a flat 15-item list. Mirrors the Zod allow-list in @@ -169,10 +170,9 @@ function resolveReminderLabel( t: (key: string) => string, ): string { if (reminder.origin === "COACH") { - const translated = t(reminder.label); - // `t()` echoes the key back on a miss; fall back to the raw value so a - // future catalog addition never surfaces a bare key. - return translated === reminder.label ? reminder.label : translated; + // A label the catalogue has no sentence for shows as written, so a future + // catalog addition never surfaces a bare key. + return translateOrNull(t, reminder.label) ?? reminder.label; } return reminder.label; } diff --git a/src/components/measurement-reminders/vorsorge-settings.tsx b/src/components/measurement-reminders/vorsorge-settings.tsx index 26f2475d3..e6649a806 100644 --- a/src/components/measurement-reminders/vorsorge-settings.tsx +++ b/src/components/measurement-reminders/vorsorge-settings.tsx @@ -29,14 +29,14 @@ import { useMeasurementReminderMutations, type MeasurementReminder, } from "@/hooks/use-measurement-reminders"; +import { translateOrNull } from "@/lib/i18n/translate-code"; function resolveLabel( reminder: MeasurementReminder, t: (key: string) => string, ): string { if (reminder.origin === "COACH") { - const translated = t(reminder.label); - return translated === reminder.label ? reminder.label : translated; + return translateOrNull(t, reminder.label) ?? reminder.label; } return reminder.label; } diff --git a/src/components/onboarding/baseline-form-utils.ts b/src/components/onboarding/baseline-form-utils.ts index c51a95722..5fefd2ffb 100644 --- a/src/components/onboarding/baseline-form-utils.ts +++ b/src/components/onboarding/baseline-form-utils.ts @@ -5,6 +5,7 @@ import { type RejectedProfileField, } from "@/lib/profile/rejected-fields"; import { toProfileSex } from "@/lib/profile/sex"; +import { translateCode } from "@/lib/i18n/translate-code"; /** * Reading the answer to the onboarding baseline step's profile write. @@ -181,10 +182,5 @@ function localizedCode( errorCode: string | undefined, t: (key: string, params?: Record) => string, ): string { - if (errorCode) { - const key = `apiErrors.${errorCode}`; - const translated = t(key); - if (translated !== key) return translated; - } - return t("onboarding.errorGeneric"); + return translateCode(t, "apiErrors", errorCode, "onboarding.errorGeneric"); } diff --git a/src/components/settings/account-section/index.tsx b/src/components/settings/account-section/index.tsx index 76dcbfc95..860586e40 100644 --- a/src/components/settings/account-section/index.tsx +++ b/src/components/settings/account-section/index.tsx @@ -58,6 +58,7 @@ import { import { AvatarSection } from "./avatar-section"; import { SetupCard } from "./setup-card"; import { dateOnlyKey } from "@/lib/tz/date-only"; +import { translateCodeOrNull } from "@/lib/i18n/translate-code"; export { resolveInitialTimezone } from "./account-section-utils"; @@ -368,10 +369,7 @@ export function AccountSection() { setFieldErrors(describeRejectedProfileFields(json?.details?.issues, t)); if (errorCode === "profile.update.nothingSaved" && field) { setSaveMsg({ key: "settings.profileNothingSaved", params: { field } }); - } else if ( - errorCode && - t(`apiErrors.${errorCode}`) !== `apiErrors.${errorCode}` - ) { + } else if (translateCodeOrNull(t, "apiErrors", errorCode) !== null) { // Only a code with a sentence of its own: every refusal carries a // code now, and the generic `http.*` family has no translation. setSaveMsg({ key: `apiErrors.${errorCode}` }); diff --git a/src/components/settings/integrations/integration-actions.tsx b/src/components/settings/integrations/integration-actions.tsx index 797813c35..bfa4cfc05 100644 --- a/src/components/settings/integrations/integration-actions.tsx +++ b/src/components/settings/integrations/integration-actions.tsx @@ -30,6 +30,7 @@ import { useSyncOutcomeMessage, type SyncOutcomeState, } from "./sync-outcome"; +import { translateCode } from "@/lib/i18n/translate-code"; export interface ProviderSync { syncing: boolean; @@ -151,11 +152,12 @@ export function ActionResultLines({ const { t } = useTranslations(); function describeError(errorCode: string): string { - const key = `settings.testConnection.errors.${errorCode}`; - const translated = t(key); - return translated === key - ? t("settings.testConnection.errors.generic") - : translated; + return translateCode( + t, + "settings.testConnection.errors", + errorCode, + "settings.testConnection.errors.generic", + ); } const busy = sync?.syncing || test?.testing; diff --git a/src/components/settings/ntfy-card.tsx b/src/components/settings/ntfy-card.tsx index 1c02767ab..92fe4546d 100644 --- a/src/components/settings/ntfy-card.tsx +++ b/src/components/settings/ntfy-card.tsx @@ -22,6 +22,7 @@ import { import { useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; import { apiFetchRaw, apiGet } from "@/lib/api/api-fetch"; +import { translateCodeOrNull } from "@/lib/i18n/translate-code"; interface NtfySettings { enabled: boolean; @@ -80,13 +81,13 @@ export function NtfyCard({ isAuthenticated }: { isAuthenticated: boolean }) { }; // A coded refusal (the private-origin policy) translates like the // test button's; anything else shows the server sentence. - const code = json.meta?.errorCode; - const key = code ? `settings.testConnection.errors.${code}` : null; - const translated = key ? t(key) : null; throw new Error( - translated && translated !== key - ? translated - : json.error || t("common.error"), + translateCodeOrNull( + t, + "settings.testConnection.errors", + json.meta?.errorCode, + ) ?? + (json.error || t("common.error")), ); } }, diff --git a/src/components/settings/test-connection-button.tsx b/src/components/settings/test-connection-button.tsx index 568e7cb9b..d0db1a295 100644 --- a/src/components/settings/test-connection-button.tsx +++ b/src/components/settings/test-connection-button.tsx @@ -6,6 +6,7 @@ import { CheckCircle2, Loader2, Plug, XCircle } from "lucide-react"; import { Button } from "@/components/ui/button"; import { useTranslations } from "@/lib/i18n/context"; import { apiFetchRaw } from "@/lib/api/api-fetch"; +import { translateCode } from "@/lib/i18n/translate-code"; /** * Shared "Test connection" UX for the settings integrations + notifications @@ -151,13 +152,12 @@ export function TestConnectionButton({ } function describeError(errorCode: string): string { - const key = `settings.testConnection.errors.${errorCode}`; - const translated = t(key); - // `t()` returns the raw key when missing — fall back to generic. - if (translated === key) { - return t("settings.testConnection.errors.generic"); - } - return translated; + return translateCode( + t, + "settings.testConnection.errors", + errorCode, + "settings.testConnection.errors.generic", + ); } return ( diff --git a/src/components/settings/webhook-card.tsx b/src/components/settings/webhook-card.tsx index 632fd3691..f09528c7e 100644 --- a/src/components/settings/webhook-card.tsx +++ b/src/components/settings/webhook-card.tsx @@ -24,6 +24,7 @@ import { useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; import { apiFetchRaw, apiGet } from "@/lib/api/api-fetch"; import type { WebhookPayloadFormat } from "@/lib/notifications/types"; +import { translateCodeOrNull } from "@/lib/i18n/translate-code"; interface WebhookSettings { enabled: boolean; @@ -89,13 +90,13 @@ export function WebhookCard({ isAuthenticated }: { isAuthenticated: boolean }) { }; // A coded refusal (the private-origin policy) translates like the // test button's; anything else shows the server sentence. - const code = json.meta?.errorCode; - const key = code ? `settings.testConnection.errors.${code}` : null; - const translated = key ? t(key) : null; throw new Error( - translated && translated !== key - ? translated - : json.error || t("common.error"), + translateCodeOrNull( + t, + "settings.testConnection.errors", + json.meta?.errorCode, + ) ?? + (json.error || t("common.error")), ); } }, diff --git a/src/components/timeline/__tests__/no-template-key-guard.test.ts b/src/components/timeline/__tests__/no-template-key-guard.test.ts index 7c460c90c..65060f24f 100644 --- a/src/components/timeline/__tests__/no-template-key-guard.test.ts +++ b/src/components/timeline/__tests__/no-template-key-guard.test.ts @@ -24,8 +24,12 @@ import { const ROOTS = ["src/components/timeline", "src/components/day"] as const; -/** `t(` or `tCount(` whose key is a template literal, across line breaks. */ -const TEMPLATE_KEY = /(?> = { "src/components/timeline/readiness-inventory.tsx": { diff --git a/src/components/timeline/readiness-inventory.tsx b/src/components/timeline/readiness-inventory.tsx index 68c27df50..84c004e96 100644 --- a/src/components/timeline/readiness-inventory.tsx +++ b/src/components/timeline/readiness-inventory.tsx @@ -34,6 +34,7 @@ import { } from "./label-keys"; import { formatMonthYear } from "./timeline-dates"; import { orderedReadinessLanes, readinessTally } from "./readiness-model"; +import { translateOrNull } from "@/lib/i18n/translate-code"; type Translate = ReturnType["t"]; type TranslateCount = ReturnType["tCount"]; @@ -65,11 +66,10 @@ export function wordKey( params: Record, ): string | null { const count = typeof params.count === "number" ? params.count : null; - if (count !== null && t(`${base}Other`) !== `${base}Other`) { + if (count !== null && translateOrNull(t, `${base}Other`) !== null) { return tCount(base, count, params); } - const single = t(base, params); - return single === base ? null : single; + return translateOrNull(t, base, params); } export function laneLabel(t: Translate, key: TimelineReadinessLane["key"]) { diff --git a/src/lib/ai/coach/results/projections.ts b/src/lib/ai/coach/results/projections.ts index c4c3202b4..fa0cdb6f7 100644 --- a/src/lib/ai/coach/results/projections.ts +++ b/src/lib/ai/coach/results/projections.ts @@ -27,6 +27,7 @@ import { coachDomainLabelKey, coachPeriodLabelKey, } from "@/lib/ai/coach/dialog-keys"; +import { translateOrNull } from "@/lib/i18n/translate-code"; export interface ProjectionContext { ref: string; @@ -58,9 +59,8 @@ export function projectWorkouts( for (const entry of perSport) { if (!isRecord(entry) || typeof entry.sport !== "string") continue; const labelKey = `insights.workouts.sport.${entry.sport}`; - const label = t(labelKey); rows.push([ - label === labelKey ? entry.sport : label, + translateOrNull(t, labelKey) ?? entry.sport, numberOrNull(entry.count), numberOrNull(entry.totalDurationMin), ]); diff --git a/src/lib/api/localized-error.ts b/src/lib/api/localized-error.ts index fecd1110d..085429ee2 100644 --- a/src/lib/api/localized-error.ts +++ b/src/lib/api/localized-error.ts @@ -1,4 +1,5 @@ import { ApiError } from "./api-fetch"; +import { translateCode } from "@/lib/i18n/translate-code"; type Translate = (key: string) => string; @@ -12,14 +13,9 @@ export function localizedApiError( t: Translate, fallbackKey: string, ): string { - if (error instanceof ApiError) { - const errorCode = error.meta?.errorCode; - if (typeof errorCode === "string") { - const key = `apiErrors.${errorCode}`; - const translated = t(key); - if (translated !== key) return translated; - } - } - - return t(fallbackKey); + const errorCode = + error instanceof ApiError && typeof error.meta?.errorCode === "string" + ? error.meta.errorCode + : null; + return translateCode(t, "apiErrors", errorCode, fallbackKey); } diff --git a/src/lib/daily/load-digest.ts b/src/lib/daily/load-digest.ts index 0d4d9dcad..d08043635 100644 --- a/src/lib/daily/load-digest.ts +++ b/src/lib/daily/load-digest.ts @@ -98,6 +98,7 @@ import type { TodaySleep, TodayVital, } from "@/lib/daily/today-overview"; +import { translateOrNull } from "@/lib/i18n/translate-code"; /** Integration states that mean "your action is needed to keep data flowing". */ const SYNC_ISSUE_STATES = ["error_reauth", "parked"] as const; @@ -907,8 +908,7 @@ export async function loadDailyDigest( if (!staysDue && row.nextDueAt.getTime() > now.getTime()) continue; let label = row.label; if (row.origin === "COACH") { - const translated = t(row.label); - if (translated !== row.label) label = translated; + label = translateOrNull(t, row.label) ?? label; } preventiveDue.push({ label, staysDue }); } diff --git a/src/lib/daily/today-overview.ts b/src/lib/daily/today-overview.ts index 105eec8ef..8aa5b25cc 100644 --- a/src/lib/daily/today-overview.ts +++ b/src/lib/daily/today-overview.ts @@ -27,6 +27,7 @@ import { formatDurationMinutes } from "@/lib/i18n/duration"; import { isFromToday } from "@/lib/insights/measurement-freshness"; import type { ModuleKey } from "@/lib/modules/registry"; import type { PriorityItem } from "@/lib/daily/priority-item"; +import { translateOrNull } from "@/lib/i18n/translate-code"; type Translate = ( key: string, @@ -510,8 +511,7 @@ function outsideBy(v: TodayVital): number { function vitalLabel(t: Translate, type: string): string { const key = `daily.today.vital.${type}`; - const label = t(key); - return label === key ? t("daily.today.vital.generic") : label; + return translateOrNull(t, key) ?? t("daily.today.vital.generic"); } function sleepDiff(sleep: TodaySleep): number | null { diff --git a/src/lib/dashboard/snapshot.ts b/src/lib/dashboard/snapshot.ts index ac58730d3..5008cffda 100644 --- a/src/lib/dashboard/snapshot.ts +++ b/src/lib/dashboard/snapshot.ts @@ -145,6 +145,7 @@ import { hideModuleWidgets, unavailableWidgetIds, } from "@/lib/dashboard/widget-modules"; +import { translateOrNull } from "@/lib/i18n/translate-code"; /** * v1.7.0 — HealthLog `MeasurementType` → iOS `MetricKind` raw value. @@ -1217,9 +1218,8 @@ function localisedMeasurementLabel( ) .join(""); const key = `measurements.type${camel.charAt(0).toUpperCase()}${camel.slice(1)}`; - const resolved = t(key); - // The server translator echoes the key on a miss; fall back to a readable form. - return resolved === key ? type.replace(/_/g, " ").toLowerCase() : resolved; + // A type without a sentence falls back to a readable form. + return translateOrNull(t, key) ?? type.replace(/_/g, " ").toLowerCase(); } /** diff --git a/src/lib/i18n/__tests__/translate-code.test.ts b/src/lib/i18n/__tests__/translate-code.test.ts new file mode 100644 index 000000000..3bfd05f56 --- /dev/null +++ b/src/lib/i18n/__tests__/translate-code.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from "vitest"; + +import { + translateCode, + translateCodeOrNull, + translateOrNull, +} from "../translate-code"; + +const CATALOGUE: Record = { + "apiErrors.sharing.invite.self": "That is your own record.", + "common.error": "Something went wrong.", + "greeting.hello": "Hello {name}", +}; + +// Echoes the key on a miss, the way the real translator does. +function t(key: string, params?: Record): string { + const value = CATALOGUE[key]; + if (value === undefined) return key; + return params ? value.replace("{name}", String(params.name)) : value; +} + +describe("translate-code", () => { + it("answers null for a key the catalogue does not carry", () => { + expect(translateOrNull(t, "missing.key")).toBeNull(); + expect(translateOrNull(t, "greeting.hello", { name: "A" })).toBe("Hello A"); + }); + + it("translates a code under its namespace, or answers null", () => { + expect(translateCodeOrNull(t, "apiErrors", "sharing.invite.self")).toBe( + "That is your own record.", + ); + expect(translateCodeOrNull(t, "apiErrors", "http.not_found")).toBeNull(); + expect(translateCodeOrNull(t, "apiErrors", undefined)).toBeNull(); + expect(translateCodeOrNull(t, "apiErrors", "")).toBeNull(); + }); + + it("falls back to the fallback key's sentence", () => { + expect( + translateCode(t, "apiErrors", "http.not_found", "common.error"), + ).toBe("Something went wrong."); + expect(translateCode(t, "apiErrors", null, "common.error")).toBe( + "Something went wrong.", + ); + }); +}); diff --git a/src/lib/i18n/translate-code.ts b/src/lib/i18n/translate-code.ts new file mode 100644 index 000000000..ff7fa19fd --- /dev/null +++ b/src/lib/i18n/translate-code.ts @@ -0,0 +1,47 @@ +/** + * Asking the catalogue whether it has a sentence. + * + * `t()` answers a missing key with the key itself, so the only way to learn + * whether a key exists is to compare the answer with the question. That probe + * lives here and nowhere else: it had been written inline at each call site + * that wanted "a code's sentence, or else the fallback", once per site. + * `src/__tests__/translate-probe-guard.test.ts` refuses it outside this file. + */ + +type Translate = ( + key: string, + params?: Record, +) => string; + +/** The catalogue's sentence for `key`, or null when the bundle has none. */ +export function translateOrNull( + t: Translate, + key: string, + params?: Record, +): string | null { + const value = t(key, params); + return value === key ? null : value; +} + +/** + * The sentence for a machine code under `namespace` (`apiErrors`, + * `settings.testConnection.errors`, …), or null when there is no code or the + * code has no sentence of its own. + */ +export function translateCodeOrNull( + t: Translate, + namespace: string, + code: string | null | undefined, +): string | null { + return code ? translateOrNull(t, `${namespace}.${code}`) : null; +} + +/** The code's sentence, or the fallback key's when it has none. */ +export function translateCode( + t: Translate, + namespace: string, + code: string | null | undefined, + fallbackKey: string, +): string { + return translateCodeOrNull(t, namespace, code) ?? t(fallbackKey); +} diff --git a/src/lib/insights/correlation-discovery.ts b/src/lib/insights/correlation-discovery.ts index 1fc18f798..db16e66c0 100644 --- a/src/lib/insights/correlation-discovery.ts +++ b/src/lib/insights/correlation-discovery.ts @@ -55,6 +55,7 @@ import { import { type Locale } from "@/lib/i18n/config"; import { getServerTranslator } from "@/lib/i18n/server-translator"; import { shiftDateKey } from "@/lib/tz/format"; +import { translateOrNull } from "@/lib/i18n/translate-code"; /** * Locale-aware string builder — the same signature the server translator @@ -417,11 +418,9 @@ function humanise(key: string, t: Translate): string { return t("insights.correlation.channelFactor", { factor }); } const labelKey = `insights.correlation.channel.${key}`; - const label = t(labelKey); - // The server translator echoes the key on a miss; fall back to a readable - // prettified form so an unmapped channel degrades gracefully, never English- - // leaks a raw key. - return label === labelKey ? key.replace(/_/g, " ").toLowerCase() : label; + // An unmapped channel falls back to a readable prettified form, never a + // raw key. + return translateOrNull(t, labelKey) ?? key.replace(/_/g, " ").toLowerCase(); } /** Namespace prefix for a RATED-mood-factor discovery channel (v1.14.0). */ From 3fbd16a0a6d7b845b6f1d3e5f06579c99271e311 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 10:10:31 +0200 Subject: [PATCH 6/9] fix(ui): let the chart error card decide from the error too ChartErrorState took no error, so a chart whose read was refused or whose session had ended offered a retry that could not help. It now takes the error and branches the way QueryErrorCard and QueryErrorRow do, the chunk boundary hands it the error it caught, and the error-prop guard covers it. QueryErrorRow gains an inset form for a failure inside a card body, and its frame takes the md:p-6 step. QueryErrorCard drops its fixed px-6 for the card's own inset and both icons move onto the size scale. --- .../query-error-error-prop-guard.test.ts | 4 +- .../__tests__/chart-error-state.test.tsx | 28 ++++++-- src/components/charts/chart-error-state.tsx | 68 +++++++++++++------ src/components/charts/health-chart.tsx | 5 +- .../charts/medication-compliance-chart.tsx | 3 +- src/components/charts/mood-chart.tsx | 3 +- .../insights/derived/score-trend-chart.tsx | 1 + src/components/ui/query-error-card.tsx | 6 +- src/components/ui/query-error-row.tsx | 9 ++- 9 files changed, 91 insertions(+), 36 deletions(-) diff --git a/src/__tests__/query-error-error-prop-guard.test.ts b/src/__tests__/query-error-error-prop-guard.test.ts index 6544cba0c..57befcb56 100644 --- a/src/__tests__/query-error-error-prop-guard.test.ts +++ b/src/__tests__/query-error-error-prop-guard.test.ts @@ -1,7 +1,7 @@ /** * Every read-failure primitive is handed the error it reports. * - * `QueryErrorRow` and `QueryErrorCard` decide what a failed read offers from + * `QueryErrorRow`, `QueryErrorCard` and `ChartErrorState` decide what a failed read offers from * the error itself: nothing (or the surface's refused state) for a 403, a * note without a retry for a 429, sign-in for a 401, a retry otherwise. A * call site that renders one without the error falls back to "failed" for @@ -49,7 +49,7 @@ function walk( /** The opening tag's attribute text, from the name to its closing `>`. */ function openingTags(source: string): string[] { const tags: string[] = []; - const open = /<(QueryErrorRow|QueryErrorCard)(?=[\s/>])/g; + const open = /<(QueryErrorRow|QueryErrorCard|ChartErrorState)(?=[\s/>])/g; for (let m = open.exec(source); m; m = open.exec(source)) { let depth = 0; let i = m.index + m[0].length; diff --git a/src/components/charts/__tests__/chart-error-state.test.tsx b/src/components/charts/__tests__/chart-error-state.test.tsx index 9197fce14..df069e970 100644 --- a/src/components/charts/__tests__/chart-error-state.test.tsx +++ b/src/components/charts/__tests__/chart-error-state.test.tsx @@ -130,16 +130,32 @@ describe(" — announcement, sizing, accessible name", () => { props: Partial = {}, ) { const { ChartErrorState } = await import("../chart-error-state"); + const { I18nProvider } = await import("@/lib/i18n/context"); return renderToStaticMarkup( - undefined} - {...props} - />, + + undefined} + {...props} + /> + , ); } + it("decides from the error, like the other read-failure primitives", async () => { + const { ApiError } = await import("@/lib/api/api-fetch"); + // A refused read draws nothing, not a retry that cannot help. + expect(await renderState({ error: new ApiError("x", 403) })).toBe(""); + const ended = await renderState({ error: new ApiError("x", 401) }); + expect(ended).toContain('data-slot="query-error-sign-in"'); + expect(ended).not.toContain("Retry"); + const throttled = await renderState({ error: new ApiError("x", 429) }); + expect(throttled).not.toContain(" { const html = await renderState(); expect(html).toContain('role="status"'); diff --git a/src/components/charts/chart-error-state.tsx b/src/components/charts/chart-error-state.tsx index b81af179c..b75814847 100644 --- a/src/components/charts/chart-error-state.tsx +++ b/src/components/charts/chart-error-state.tsx @@ -4,6 +4,7 @@ import { Component, type ReactElement, type ReactNode } from "react"; import { AlertTriangle, RotateCw } from "lucide-react"; import { Button } from "@/components/ui/button"; +import { useReadFailure } from "@/hooks/use-read-failure"; import { useTranslations } from "@/lib/i18n/context"; import { cn } from "@/lib/utils"; @@ -22,9 +23,18 @@ import { cn } from "@/lib/utils"; * reflows — with a compact action button. `` wraps * each lazy chart mount so a failed chunk degrades to one card with a * reload affordance instead of taking the page down. + * + * Like `QueryErrorCard` and `QueryErrorRow`, the card reads the failure + * itself (`error`): a refusal draws `refused` (nothing by default), a spent + * budget says so without an action, an ended session offers sign-in, and only + * a real failure offers `onAction`. */ export interface ChartErrorStateProps { + /** The failure. It decides what the card offers. */ + error: unknown; + /** What a refused (403) read draws instead of the card. */ + refused?: ReactNode; /** Translated headline, e.g. "Data could not be loaded". */ title: string; /** Translated action-button label, e.g. "Retry". */ @@ -54,6 +64,8 @@ export interface ChartErrorStateProps { } export function ChartErrorState({ + error, + refused = null, title, actionLabel, onAction, @@ -61,6 +73,10 @@ export function ChartErrorState({ actionContext, role = "status", }: ChartErrorStateProps): ReactElement { + const failure = useReadFailure(error, { message: title, onRetry: onAction }); + if (!failure) return <>{refused}; + const { action } = failure; + const label = action?.kind === "retry" ? actionLabel : action?.label; return (
    ); } interface BoundaryProps { - fallback: ReactNode; + /** Drawn once caught; a function receives the caught error. */ + fallback: ReactNode | ((error: Error) => ReactNode); children: ReactNode; /** Called once the boundary has caught, after the fallback commits. */ onError?: (error: Error) => void; } interface BoundaryState { - failed: boolean; + error: Error | null; } /** Exported for its unit test; mount `` instead. */ export class ChartBoundary extends Component { - state: BoundaryState = { failed: false }; + state: BoundaryState = { error: null }; - static getDerivedStateFromError(): BoundaryState { - return { failed: true }; + static getDerivedStateFromError(error: Error): BoundaryState { + return { error }; } componentDidCatch(error: Error): void { @@ -125,7 +147,10 @@ export class ChartBoundary extends Component { } render(): ReactNode { - return this.state.failed ? this.props.fallback : this.props.children; + const { error } = this.state; + if (!error) return this.props.children; + const { fallback } = this.props; + return typeof fallback === "function" ? fallback(error) : fallback; } } @@ -152,7 +177,7 @@ export function ChartErrorBoundary({ return ( ( // Wrapped in the chart-card shell so a chunk failure renders // as one solid card among the other cards, not a bare dashed // box; `role="alert"` because the whole chart is gone, not @@ -162,13 +187,14 @@ export function ChartErrorBoundary({ className="bg-card border-border rounded-xl border p-4 md:p-6" > window.location.reload()} role="alert" /> - } + )} > {children} diff --git a/src/components/charts/health-chart.tsx b/src/components/charts/health-chart.tsx index fdbe42f25..1cdbca5a8 100644 --- a/src/components/charts/health-chart.tsx +++ b/src/components/charts/health-chart.tsx @@ -820,7 +820,7 @@ export function HealthChart({ ); }, [preloadedSeries, preloadedCoverageDays, fetchWindow.windowDays, types]); - const { data, isLoading, isError, isPlaceholderData, refetch } = useQuery({ + const chartQuery = useQuery({ // A range-tab change re-keys the query; without placeholder data every // switch dropped the painted chart to a skeleton for the fetch round-trip. // Keeping the previous range's series visible is only honest because the @@ -1096,6 +1096,8 @@ export function HealthChart({ ), enabled: isAuthenticated, }); + const { data, isLoading, isError, error, isPlaceholderData, refetch } = + chartQuery; // v1.16.0 — report the settled data query to the dashboard's shared // reveal gate. `isLoading` only covers the INITIAL fetch (a later @@ -1921,6 +1923,7 @@ export function HealthChart({ // Pre-fix `isError` fell through to the empty branch and an // outage read as "you have no measurements". => { return apiGet( @@ -548,6 +548,7 @@ export function MedicationComplianceChart({ // through to the empty branch (data undefined → hasData false) // and an outage read as "no doses tracked". => { return apiGet("/api/mood/analytics"); @@ -756,6 +756,7 @@ export function MoodChart({ // v1.16.8 — a failed query paints as an ERROR with a retry // affordance instead of unmounting the card (see the gate above). ) : query.isError ? (