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); }); });