From e52a0469c82569dcc0a253ace75cba02d7aaf816 Mon Sep 17 00:00:00 2001 From: iris Date: Wed, 29 Jul 2026 20:16:39 +0200 Subject: [PATCH] fix retried submissions double-booking a sale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server dedupes on client_uuid, but the tablet minted a fresh uuid on every confirm() call — including retries after a timeout/dropped connection, exactly the case the idempotency key exists to guard against. The dedup check never fired on a real retry, so a flaky-Wi-Fi resend could book the same sale twice. Client: generate one uuid per pending cart (a ref, lazily created), reuse it across retries of the same submission, reset it only when the cart is cleared (success or cancel) so the next cart gets its own id. Server: the existing-row dedup check and the insert straddled the db.transaction() boundary, so a genuine UNIQUE-violation race would have surfaced as a raw 500 instead of the idempotent response. Catch that specific violation and fall back to re-reading the row. --- client/src/tablet/Sale.tsx | 14 ++++++++++++- server/src/routes/public.ts | 40 +++++++++++++++++++++++++++---------- 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/client/src/tablet/Sale.tsx b/client/src/tablet/Sale.tsx index 9177c14..d7ec122 100644 --- a/client/src/tablet/Sale.tsx +++ b/client/src/tablet/Sale.tsx @@ -22,6 +22,17 @@ export function Sale({ config, onChangeBar }: Props) { const longPressTimer = useRef(null); const longPressFired = useRef(false); + // One idempotency key per pending cart, reused across retries of the same + // logical submission (e.g. after a timeout/dropped-connection error) so + // the server's client_uuid dedup can actually recognise a resend. Reset + // whenever the cart is cleared (submitted successfully or cancelled), so + // the *next* cart gets its own fresh id. + const cartUuidRef = useRef(null); + function cartUuid() { + if (!cartUuidRef.current) cartUuidRef.current = uuid(); + return cartUuidRef.current; + } + const drinkById = useMemo(() => { const m = new Map(); drinks.forEach(d => m.set(d.id, d)); @@ -72,6 +83,7 @@ export function Sale({ config, onChangeBar }: Props) { function clear() { setLines([]); setPfandReturns(0); + cartUuidRef.current = null; } function startLongPress(d: Drink) { @@ -115,7 +127,7 @@ export function Sale({ config, onChangeBar }: Props) { const items: CartItem[] = lines.map(l => ({ drink_id: l.drink_id, qty: l.qty })); try { await api.createTransaction({ - client_uuid: uuid(), + client_uuid: cartUuid(), bar_id: bar.id, crew, items, diff --git a/server/src/routes/public.ts b/server/src/routes/public.ts index d607017..96a4e53 100644 --- a/server/src/routes/public.ts +++ b/server/src/routes/public.ts @@ -55,9 +55,12 @@ export function registerPublicRoutes(app: FastifyInstance, db: DB) { return reply.code(400).send({ error: 'empty transaction' }); } - const existing = db - .prepare('SELECT id, total_cents FROM transactions WHERE client_uuid = ?') - .get(client_uuid) as { id: number; total_cents: number } | undefined; + const findByUuid = () => + db + .prepare('SELECT id, total_cents FROM transactions WHERE client_uuid = ?') + .get(client_uuid) as { id: number; total_cents: number } | undefined; + + const existing = findByUuid(); if (existing) { const res: CreateTransactionResponse = { id: existing.id, total_cents: existing.total_cents }; return res; @@ -118,14 +121,31 @@ export function registerPublicRoutes(app: FastifyInstance, db: DB) { ); const createdAt = new Date().toISOString(); - const txId = db.transaction(() => { - const info = insertTx.run(bar_id, createdAt, paidTotal, crew ? 1 : 0, pfand_returns, clientIp, client_uuid); - const id = Number(info.lastInsertRowid); - for (const p of priced) { - insertItem.run(id, p.drink_id, p.qty, p.unit_price_cents, p.pfand_cents_per_unit); + let txId: number; + try { + txId = db.transaction(() => { + const info = insertTx.run(bar_id, createdAt, paidTotal, crew ? 1 : 0, pfand_returns, clientIp, client_uuid); + const id = Number(info.lastInsertRowid); + for (const p of priced) { + insertItem.run(id, p.drink_id, p.qty, p.unit_price_cents, p.pfand_cents_per_unit); + } + return id; + })(); + } catch (e) { + // A UNIQUE violation on client_uuid means a concurrent/near-simultaneous + // retry of this exact submission won the race between our dedup check + // above and this insert — fall back to the idempotent response instead + // of surfacing a raw 500 for what is, from the client's perspective, a + // successful (already-processed) submission. + if (e instanceof Error && e.message.includes('UNIQUE') && e.message.includes('client_uuid')) { + const race = findByUuid(); + if (race) { + const res: CreateTransactionResponse = { id: race.id, total_cents: race.total_cents }; + return res; + } } - return id; - })(); + throw e; + } const res: CreateTransactionResponse = { id: txId, total_cents: paidTotal }; return res;