mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-12 14:16:12 +00:00
fix(api): take a new idempotency key once the server has answered
A create form kept its key until a save succeeded. The server stores a refusal under the key for a day, so after a 422 or 409 the corrected retry ran as a different body without any protection, and a timeout on it could store the entry twice; an unchanged retry after the cause was gone replayed the stale refusal until the form was closed. Any answer now settles the key. Only a timeout, a dropped connection, or the 409 that says the first request under the key is still running (`X-Idempotent-Replay: false`, now `ApiError.inFlight`) keeps it, so those retries still replay the first answer.
This commit is contained in:
1 parent
0c77eaa085
commit
9683daf4f2
3 files changed
+73
-8
No files matched your search
@@ -63,6 +63,53 @@ describe("useIdempotentPost", () => {
|
||||
expect(keyOf(1)).not.toBe(keyOf(0));
|
||||
});
|
||||
|
||||
it("takes a new key once the server refused the entry", async () => {
|
||||
// The refusal is stored under the key, so the corrected retry under it
|
||||
// ran unprotected and an unchanged one replayed the refusal.
|
||||
fetchMock
|
||||
.mockResolvedValueOnce(
|
||||
new Response(
|
||||
JSON.stringify({ data: null, error: "Name taken", meta: {} }),
|
||||
{ status: 409, headers: { "Content-Type": "application/json" } },
|
||||
),
|
||||
)
|
||||
.mockResolvedValueOnce(stored());
|
||||
const post = openForm();
|
||||
|
||||
await expect(
|
||||
post("/api/custom-metrics", { name: "Steps" }),
|
||||
).rejects.toThrow();
|
||||
await post("/api/custom-metrics", { name: "Steps" });
|
||||
|
||||
expect(keyOf(1)).not.toBe(keyOf(0));
|
||||
});
|
||||
|
||||
it("keeps the key while the first request is still running", async () => {
|
||||
fetchMock
|
||||
.mockResolvedValueOnce(
|
||||
new Response(
|
||||
JSON.stringify({
|
||||
data: null,
|
||||
error: "A request with this Idempotency-Key is already in progress",
|
||||
}),
|
||||
{
|
||||
status: 409,
|
||||
headers: {
|
||||
"Content-Type": "application/json",
|
||||
"X-Idempotent-Replay": "false",
|
||||
},
|
||||
},
|
||||
),
|
||||
)
|
||||
.mockResolvedValueOnce(stored());
|
||||
const post = openForm();
|
||||
|
||||
await expect(post("/api/mood-entries", { mood: "GOOD" })).rejects.toThrow();
|
||||
await post("/api/mood-entries", { mood: "GOOD" });
|
||||
|
||||
expect(keyOf(1)).toBe(keyOf(0));
|
||||
});
|
||||
|
||||
it("keeps a header the caller set", async () => {
|
||||
fetchMock.mockResolvedValueOnce(stored());
|
||||
const post = openForm();
|
||||
|
||||
@@ -79,6 +79,12 @@ export class ApiError extends Error {
|
||||
* so a form can mark the one the server refused. Empty when it named none.
|
||||
*/
|
||||
readonly issuePaths: readonly string[];
|
||||
/**
|
||||
* The idempotency layer's "a request under this key is still running" (a
|
||||
* 409 with `X-Idempotent-Replay: false`): nothing was decided, and a retry
|
||||
* under the same key replays the first request's answer.
|
||||
*/
|
||||
readonly inFlight: boolean;
|
||||
|
||||
constructor(
|
||||
message: string,
|
||||
@@ -86,12 +92,14 @@ export class ApiError extends Error {
|
||||
meta?: Record<string, unknown>,
|
||||
requestId?: string | null,
|
||||
issuePaths: readonly string[] = [],
|
||||
inFlight = false,
|
||||
) {
|
||||
super(message);
|
||||
this.name = "ApiError";
|
||||
this.status = status;
|
||||
this.meta = meta;
|
||||
this.issuePaths = issuePaths;
|
||||
this.inFlight = inFlight;
|
||||
const fromMeta =
|
||||
typeof meta?.requestId === "string" && meta.requestId.length > 0
|
||||
? meta.requestId
|
||||
@@ -133,6 +141,7 @@ export async function throwApiError(res: Response): Promise<never> {
|
||||
meta,
|
||||
res.headers.get("x-request-id"),
|
||||
issuePaths,
|
||||
res.status === 409 && res.headers.get("x-idempotent-replay") === "false",
|
||||
);
|
||||
// A server-side failure is worth an id in the session's support info,
|
||||
// whether or not the call site ends up showing it.
|
||||
|
||||
@@ -2,19 +2,23 @@
|
||||
|
||||
import { useRef } from "react";
|
||||
|
||||
import { apiPost } from "@/lib/api/api-fetch";
|
||||
import { ApiError, apiPost } from "@/lib/api/api-fetch";
|
||||
import { randomId } from "@/lib/random-id";
|
||||
|
||||
/**
|
||||
* `apiPost` for a create form, under one `Idempotency-Key` until a save lands.
|
||||
* `apiPost` for a create form, under one `Idempotency-Key` until an answer
|
||||
* arrives.
|
||||
*
|
||||
* The transport mints a fresh key per call, which covers a replayed request
|
||||
* but not a second tap on Save: when the first answer never arrived (the 15 s
|
||||
* timeout on a weak signal) the entry is already stored, and the retry is a
|
||||
* new call with a new key, so the server stored it twice. Every submit
|
||||
* through this function sends the same key until one succeeds, so the retry
|
||||
* replays the first answer instead; a retry with a changed body is a
|
||||
* different request and runs as one. The next entry gets a new key.
|
||||
* through this function sends the same key until the server answers, so the
|
||||
* retry replays the first answer instead. An answer settles the key, a
|
||||
* refusal included: the server keeps a refusal under its key for a day, so a
|
||||
* corrected retry under the old key was never protected and an unchanged one
|
||||
* replayed a refusal that no longer held. Only the answer that the first
|
||||
* request is still running keeps it. The next entry gets a new key.
|
||||
*/
|
||||
export function useIdempotentPost() {
|
||||
const key = useRef<string | null>(null);
|
||||
@@ -26,8 +30,13 @@ export function useIdempotentPost() {
|
||||
key.current ??= randomId();
|
||||
const headers = new Headers(init?.headers);
|
||||
headers.set("Idempotency-Key", key.current);
|
||||
const result = await apiPost<T>(path, body, { ...init, headers });
|
||||
key.current = null;
|
||||
return result;
|
||||
try {
|
||||
const result = await apiPost<T>(path, body, { ...init, headers });
|
||||
key.current = null;
|
||||
return result;
|
||||
} catch (error) {
|
||||
if (error instanceof ApiError && !error.inFlight) key.current = null;
|
||||
throw error;
|
||||
}
|
||||
};
|
||||
}
|
||||
Reference in new issue
Block a user