mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-10 05:12:14 +00:00
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.
This commit is contained in:
1 parent
0057b03699
commit
c300574a8e
20 files changed
+212
-128
No files matched your search
@@ -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(
|
||||
|
||||
@@ -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";
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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([
|
||||
|
||||
Reference in new issue
Block a user