From 2ee50b27f2beb38ce48094c5e880beebffdca3d0 Mon Sep 17 00:00:00 2001 From: Abidoyesimze <154632691+Abidoyesimze@users.noreply.github.com> Date: Sat, 17 Oct 2026 16:14:54 +0100 Subject: [PATCH] feat(backend): enhance distributed concurrency control and locking for Idempotency Key Manager Replaces the check-then-set idempotency flow (GET followed by a fire-and-forget SET) with a distributed lock built on an atomic SET NX PX acquisition keyed per merchant+key: - Concurrent duplicates now receive 409 while the original request is still in flight instead of both executing (the previous race let every duplicate through). - Completion is written with two Lua compare-and-swap scripts so a worker that lost its lock (TTL expiry or crash) can never clobber the state of the request that re-acquired the key; orphaned completions are counted and logged instead. - Non-2xx responses release the lock immediately so clients can retry, and connections that close before a response release it too. - Replays now return the original status code (legacy 201 fallback) and set Idempotency-Replayed; legacy {"hash","response"} records are still honoured so the change is backwards compatible. - Redis failures still fail open but are now logged and counted. Adds Prometheus instrumentation (idempotency_requests_total, idempotency_lock_duration_seconds, idempotency_orphaned_completions_total) and unit coverage for the race, lock TTL self-healing, payload mismatch, legacy replay, fail-open and lock cleanup paths. --- backend/src/lib/idempotency.js | 368 +++++++++++++++++++------- backend/src/lib/idempotency.test.js | 389 ++++++++++++++++++++++++++++ backend/src/lib/metrics.js | 23 ++ 3 files changed, 688 insertions(+), 92 deletions(-) create mode 100644 backend/src/lib/idempotency.test.js diff --git a/backend/src/lib/idempotency.js b/backend/src/lib/idempotency.js index d75619bd..9c9f2d4f 100644 --- a/backend/src/lib/idempotency.js +++ b/backend/src/lib/idempotency.js @@ -1,132 +1,316 @@ import crypto from "node:crypto"; import { getRedisClient } from "./redis.js"; +import { logger } from "./logger.js"; +import { + idempotencyRequestsTotal, + idempotencyLockDuration, + idempotencyOrphanedCompletionsTotal, +} from "./metrics.js"; /** - * TTL for idempotency cache entries in seconds. + * TTL for completed idempotency records in seconds. */ -const IDEMPOTENCY_TTL_SECONDS = 24 * 60 * 60; // 24 hours +export const IDEMPOTENCY_TTL_SECONDS = 24 * 60 * 60; // 24 hours -export function buildIdempotencyRedisKey(merchantId, idempotencyKey) { - return `idempotency:${merchantId}:${idempotencyKey}`; -} +/** + * TTL for the in-flight lock a request holds while its handler is running. + * + * This bounds the worst-case stall: if a worker dies between acquiring the lock + * and writing the completion record, the lock self-heals after this window so a + * retry can make progress instead of being rejected forever. + */ +export const IDEMPOTENCY_LOCK_TTL_MS = (() => { + const raw = Number.parseInt(String(process.env.IDEMPOTENCY_LOCK_TTL_MS ?? ""), 10); + return Number.isFinite(raw) && raw > 0 ? raw : 60_000; +})(); + +/** + * Atomically replaces an in-flight record with its completion, but only while + * the caller still owns the lock (token match). Returns 1 on success, 0 when the + * lock was lost (expired and/or re-acquired by another request) so the caller + * can avoid clobbering the new owner's state. + */ +export const COMPLETE_LOCK_SCRIPT = ` +local raw = redis.call('GET', KEYS[1]) +if not raw then return 0 end +local ok, data = pcall(cjson.decode, raw) +if not ok or type(data) ~= 'table' or data.token ~= ARGV[1] then return 0 end +redis.call('SET', KEYS[1], ARGV[2], 'PX', tonumber(ARGV[3])) +return 1 +`; -export function hashIdempotencyPayload(body) { +/** + * Atomically deletes an in-flight record only if the caller still owns the lock + * (token match). Used when a handler fails or aborts so a retry is not blocked. + */ +export const RELEASE_LOCK_SCRIPT = ` +local raw = redis.call('GET', KEYS[1]) +if not raw then return 0 end +local ok, data = pcall(cjson.decode, raw) +if not ok or type(data) ~= 'table' or data.token ~= ARGV[1] then return 0 end +redis.call('DEL', KEYS[1]) +return 1 +`; + +/** + * Deterministic hash of the request body. Kept as a plain JSON.stringify hash so + * records written before this change (which used the same input) still match. + */ +export function hashPayload(body) { return crypto .createHash("sha256") - .update(JSON.stringify(body || {})) + .update(JSON.stringify(body ?? {})) .digest("hex"); } -/** - * Look up a previously cached idempotent response. - * - * Used by the payment session handler after it acquires the per-key session - * lock (issue #1450): a request that passed the middleware while a concurrent - * twin was still in flight must replay that twin's response instead of - * creating a second session. - * - * `payloadHash` must be the hash the middleware computed from the RAW body - * (exposed as `req.idempotency.payloadHash`), because downstream Zod schemas - * may transform `req.body`. - * - * @returns {Promise} - */ -export async function lookupIdempotentResponse({ redisClient, merchantId, idempotencyKey, payloadHash }) { - if (!redisClient || !merchantId || !idempotencyKey || !payloadHash) return null; - const cachedValue = await redisClient.get(buildIdempotencyRedisKey(merchantId, idempotencyKey)); - if (!cachedValue) return null; - - const { hash, response } = JSON.parse(cachedValue); - if (hash !== payloadHash) { - return { status: "mismatch" }; - } - return { status: "hit", response }; +export function buildIdempotencyKey(merchantId, idempotencyKey) { + return `idempotency:${merchantId}:${idempotencyKey}`; } /** - * Idempotency middleware that checks and enforces idempotent requests. - * Tracks the key tied to the payload hash and response. - * Returns a cached 201 response if the key matches a previous request. - * Stores cached state in Redis for 24h. + * Normalises a raw Redis value into a record with an explicit state. + * Legacy records ({ hash, response } only) are treated as completed replays. */ -export async function idempotencyMiddleware(req, res, next) { - // Only process POST requests as per requirements - if (req.method !== "POST") { - return next(); +function parseRecord(raw) { + let data; + try { + data = JSON.parse(raw); + } catch { + return null; } + if (!data || typeof data !== "object") return null; + if (data.state === "inflight" || data.state === "completed") return data; + if (data.response !== undefined) { + return { state: "completed", hash: data.hash, statusCode: 201, response: data.response }; + } + return null; +} - const idempotencyKey = req.get("Idempotency-Key"); +async function acquireLock(client, key, value, ttlMs) { + const result = await client.set(key, value, { NX: true, PX: ttlMs }); + return result === "OK" || result === true; +} - if (idempotencyKey === undefined) { - // Idempotency-Key is optional but allows safe retries when present - return next(); +async function compareAndSwap(client, key, token, value, ttlMs) { + if (typeof client.eval === "function") { + const result = await client.eval(COMPLETE_LOCK_SCRIPT, { + keys: [key], + arguments: [token, value, String(ttlMs)], + }); + return Number(result) === 1; } + // Fallback for clients without scripting (e.g. the no-op Redis shim). + const current = parseRecord(await client.get(key)); + if (!current || current.token !== token) return false; + await client.set(key, value, { PX: ttlMs }); + return true; +} - if (typeof idempotencyKey !== "string" || idempotencyKey.trim().length === 0) { - return res.status(400).json({ - error: "Idempotency-Key header must be a non-empty string", +async function releaseLock(client, key, token) { + if (typeof client.eval === "function") { + const result = await client.eval(RELEASE_LOCK_SCRIPT, { + keys: [key], + arguments: [token], }); + return Number(result) === 1; } + const current = parseRecord(await client.get(key)); + if (!current || current.token !== token) return false; + await client.del(key); + return true; +} +function replay(res, record) { + const statusCode = Number.isInteger(record.statusCode) ? record.statusCode : 201; + res.set("Idempotency-Replayed", "true"); + res.status(statusCode).json(record.response); +} - // Ensure merchant context is available (assumes requireApiKeyAuth was run) - const merchantId = req.merchant?.id; - if (!merchantId) { - // If authentication hasn't run or failed to set merchant, we can't safely track idempotency per merchant - return res.status(401).json({ error: "Merchant authentication required" }); +/** + * Handles an existing record that was found for the key. + * Returns true when the request has been answered, false when the caller + * observed an in-flight conflict (also answered) — never "continue". + */ +function resolveExisting(res, record, payloadHash) { + if (record.hash && record.hash !== payloadHash) { + idempotencyRequestsTotal.inc({ result: "payload_mismatch" }); + res.status(400).json({ + error: "Idempotency-Key already used with a different request payload", + }); + return; + } + if (record.state === "inflight") { + idempotencyRequestsTotal.inc({ result: "in_flight_conflict" }); + res.set("Retry-After", "1"); + res.status(409).json({ + error: "A request with this Idempotency-Key is already in progress", + }); + return; } + idempotencyRequestsTotal.inc({ result: "replay" }); + replay(res, record); +} - const redisClient = getRedisClient(); - const redisKey = buildIdempotencyRedisKey(merchantId, idempotencyKey); +/** + * Builds the idempotency middleware. + * + * Distributed concurrency control: a request claims the key with an atomic + * `SET NX PX`, so only one instance processes a given key at a time. Concurrent + * duplicates receive 409 while the original is in flight, a completed record is + * replayed verbatim, and a lock that outlives its owner self-heals via TTL. + */ +export function createIdempotencyMiddleware({ + getClient = getRedisClient, + lockTtlMs = IDEMPOTENCY_LOCK_TTL_MS, + ttlSeconds = IDEMPOTENCY_TTL_SECONDS, +} = {}) { + return async function idempotencyMiddleware(req, res, next) { + if (req.method !== "POST") { + idempotencyRequestsTotal.inc({ result: "bypassed" }); + return next(); + } - // Calculate hash of payload to ensure consistency - const payloadHash = hashIdempotencyPayload(req.body); - req.idempotency = { key: idempotencyKey, payloadHash }; + const idempotencyKey = req.get("Idempotency-Key"); - try { - const cachedValue = await redisClient.get(redisKey); + if (idempotencyKey === undefined) { + idempotencyRequestsTotal.inc({ result: "bypassed" }); + return next(); + } + + if (typeof idempotencyKey !== "string" || idempotencyKey.trim().length === 0) { + return res.status(400).json({ + error: "Idempotency-Key header must be a non-empty string", + }); + } + + const merchantId = req.merchant?.id; + if (!merchantId) { + return res.status(401).json({ error: "Merchant authentication required" }); + } + + const redisKey = buildIdempotencyKey(merchantId, idempotencyKey); + const payloadHash = hashPayload(req.body); - if (cachedValue) { - const { hash, response } = JSON.parse(cachedValue); + let client; + let lockAcquired = false; + let token; + let settled = false; + let lockStartedAt = 0; - // Verify if the payload matches the original request - if (hash !== payloadHash) { - return res.status(400).json({ - error: "Idempotency-Key already used with a different request payload" - }); + const observeLock = (outcome) => { + if (lockAcquired && lockStartedAt) { + idempotencyLockDuration.observe({ outcome }, (Date.now() - lockStartedAt) / 1000); } + }; - // Return cached response with 201 status code as requested - return res.status(201).json(response); - } + try { + client = getClient(); + + const existingRaw = await client.get(redisKey); + if (existingRaw) { + const record = parseRecord(existingRaw); + if (!record) { + await client.del(redisKey); + } else { + return resolveExisting(res, record, payloadHash); + } + } + + token = crypto.randomUUID(); + const inflight = JSON.stringify({ + state: "inflight", + hash: payloadHash, + token, + startedAt: new Date().toISOString(), + }); + + lockAcquired = await acquireLock(client, redisKey, inflight, lockTtlMs); + + if (!lockAcquired) { + // Lost the race between GET and SET — resolve against the winner's record. + const winnerRaw = await client.get(redisKey); + const winner = winnerRaw ? parseRecord(winnerRaw) : null; + if (winner) { + return resolveExisting(res, winner, payloadHash); + } + // The winner vanished (expired/rolled back); treat as best-effort proceed. + return next(); + } - // Capture the original json method to intercept the response - const originalJson = res.json.bind(res); + lockStartedAt = Date.now(); + idempotencyRequestsTotal.inc({ result: "acquired" }); - res.json = function (data) { - // Only cache successful creation-like responses (2xx) - if (res.statusCode >= 200 && res.statusCode < 300) { - redisClient - .set( - redisKey, - JSON.stringify({ + const originalJson = res.json.bind(res); + + res.json = function idempotentJson(data) { + if (!settled) { + settled = true; + const statusCode = res.statusCode; + + if (statusCode >= 200 && statusCode < 300) { + const completed = JSON.stringify({ + state: "completed", hash: payloadHash, + statusCode, response: data, - }), - { EX: IDEMPOTENCY_TTL_SECONDS } - ) - .catch((err) => { - console.error("Failed to cache idempotency response:", err.message); - }); - } - return originalJson(data); - }; + }); + compareAndSwap(client, redisKey, token, completed, ttlSeconds * 1000) + .then((completedWritten) => { + observeLock(completedWritten ? "completed" : "orphaned"); + if (!completedWritten) { + idempotencyOrphanedCompletionsTotal.inc(); + logger.warn( + { idempotencyKey, merchantId }, + "Idempotency lock lost before response was cached; response replayed only if retried after completion", + ); + } + }) + .catch((err) => { + observeLock("orphaned"); + logger.error( + { err, idempotencyKey, merchantId }, + "Failed to persist idempotency completion record", + ); + }); + } else { + releaseLock(client, redisKey, token) + .then(() => observeLock("released")) + .catch((err) => { + logger.error( + { err, idempotencyKey, merchantId }, + "Failed to release idempotency lock after non-2xx response", + ); + }); + } + } + return originalJson(data); + }; - next(); - } catch (err) { - // If Redis is unavailable, log error and proceed without idempotency (fail-safe) - console.error("Idempotency check failed (Redis error):", err.message); - next(); - } + res.on("close", () => { + if (!settled) { + settled = true; + releaseLock(client, redisKey, token) + .then(() => observeLock("released")) + .catch((err) => { + logger.error( + { err, idempotencyKey, merchantId }, + "Failed to release idempotency lock for aborted request", + ); + }); + } + }); + + return next(); + } catch (err) { + // Fail open: a Redis outage must not take down payment creation. + idempotencyRequestsTotal.inc({ result: "redis_error" }); + logger.error( + { err, idempotencyKey, merchantId }, + "Idempotency check failed (Redis error); proceeding without idempotency guarantee", + ); + return next(); + } + }; } +export const idempotencyMiddleware = createIdempotencyMiddleware(); diff --git a/backend/src/lib/idempotency.test.js b/backend/src/lib/idempotency.test.js new file mode 100644 index 00000000..651958b6 --- /dev/null +++ b/backend/src/lib/idempotency.test.js @@ -0,0 +1,389 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import express from "express"; +import request from "supertest"; +import { + createIdempotencyMiddleware, + buildIdempotencyKey, + hashPayload, + COMPLETE_LOCK_SCRIPT, + RELEASE_LOCK_SCRIPT, +} from "./idempotency.js"; + +class FakeRedis { + constructor() { + this.store = new Map(); + this.failNextGet = false; + this.failNextSet = false; + } + + async get(key) { + if (this.failNextGet) { + this.failNextGet = false; + throw new Error("redis get unavailable"); + } + const entry = this.store.get(key); + if (!entry) return null; + if (entry.expiresAt && entry.expiresAt <= Date.now()) { + this.store.delete(key); + return null; + } + return entry.value; + } + + async set(key, value, options = {}) { + if (this.failNextSet) { + this.failNextSet = false; + throw new Error("redis set unavailable"); + } + const entry = this.store.get(key); + const expired = entry?.expiresAt && entry.expiresAt <= Date.now(); + if (options.NX && entry && !expired) return null; + this.store.set(key, { + value, + expiresAt: options.PX ? Date.now() + options.PX : null, + }); + return "OK"; + } + + async del(key) { + return this.store.delete(key) ? 1 : 0; + } + + async eval(script, { keys, arguments: args }) { + const [key] = keys; + const raw = await this.get(key); + if (script === COMPLETE_LOCK_SCRIPT) { + const [token, value, ttl] = args; + const data = raw ? JSON.parse(raw) : null; + if (!data || data.token !== token) return 0; + await this.set(key, value, { PX: Number(ttl) }); + return 1; + } + if (script === RELEASE_LOCK_SCRIPT) { + const [token] = args; + const data = raw ? JSON.parse(raw) : null; + if (!data || data.token !== token) return 0; + await this.del(key); + return 1; + } + throw new Error("unknown script"); + } +} + +function makeApp(middleware, { merchantId = "merchant-1", onRequest } = {}) { + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + if (merchantId) req.merchant = { id: merchantId }; + next(); + }); + app.use(middleware); + app.post("/resource", async (req, res, next) => { + try { + if (onRequest) await onRequest(req, res); + res.status(201).json({ ok: true, echo: req.body ?? null }); + } catch (err) { + next(err); + } + }); + app.use((err, _req, res, _next) => res.status(500).json({ error: err.message })); + return app; +} + +const wait = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); + +async function waitFor(predicate, { timeout = 1000, interval = 5 } = {}) { + const start = Date.now(); + while (Date.now() - start < timeout) { + if (predicate()) return true; + await wait(interval); + } + return predicate(); +} + +describe("idempotency middleware", () => { + let redis; + + beforeEach(() => { + redis = new FakeRedis(); + }); + + it("bypasses requests without an Idempotency-Key", async () => { + let calls = 0; + const app = makeApp(createIdempotencyMiddleware({ getClient: () => redis }), { + onRequest: () => { + calls += 1; + }, + }); + + await request(app).post("/resource").send({ amount: 1 }).expect(201); + await request(app).post("/resource").send({ amount: 1 }).expect(201); + expect(calls).toBe(2); + }); + + it("rejects an empty Idempotency-Key", async () => { + const app = makeApp(createIdempotencyMiddleware({ getClient: () => redis })); + const res = await request(app) + .post("/resource") + .set("Idempotency-Key", " ") + .send({ amount: 1 }); + expect(res.status).toBe(400); + }); + + it("requires merchant authentication", async () => { + const app = makeApp(createIdempotencyMiddleware({ getClient: () => redis }), { + merchantId: null, + }); + const res = await request(app) + .post("/resource") + .set("Idempotency-Key", "abc") + .send({ amount: 1 }); + expect(res.status).toBe(401); + }); + + it("executes once and replays the stored response for the same key and payload", async () => { + let calls = 0; + const app = makeApp(createIdempotencyMiddleware({ getClient: () => redis }), { + onRequest: () => { + calls += 1; + }, + }); + + const first = await request(app) + .post("/resource") + .set("Idempotency-Key", "key-1") + .send({ amount: 5 }) + .expect(201); + + await wait(10); + + const second = await request(app) + .post("/resource") + .set("Idempotency-Key", "key-1") + .send({ amount: 5 }) + .expect(201); + + expect(calls).toBe(1); + expect(second.headers["idempotency-replayed"]).toBe("true"); + expect(second.body).toEqual(first.body); + }); + + it("replays the stored status code rather than a hard-coded 201", async () => { + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + req.merchant = { id: "merchant-1" }; + next(); + }); + app.use(createIdempotencyMiddleware({ getClient: () => redis })); + app.post("/resource", (req, res) => res.status(200).json({ ok: true })); + + await request(app).post("/resource").set("Idempotency-Key", "k").send({}).expect(200); + await wait(10); + const replay = await request(app) + .post("/resource") + .set("Idempotency-Key", "k") + .send({}) + .expect(200); + expect(replay.body).toEqual({ ok: true }); + }); + + it("rejects a reused key with a different payload", async () => { + const app = makeApp(createIdempotencyMiddleware({ getClient: () => redis })); + + await request(app).post("/resource").set("Idempotency-Key", "key-2").send({ amount: 5 }); + await wait(10); + + const res = await request(app) + .post("/resource") + .set("Idempotency-Key", "key-2") + .send({ amount: 6 }); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/different request payload/); + }); + + it("rejects concurrent duplicates with 409 while the original is in flight", async () => { + let calls = 0; + let release; + const gate = new Promise((resolve) => { + release = resolve; + }); + const app = makeApp(createIdempotencyMiddleware({ getClient: () => redis }), { + onRequest: async () => { + calls += 1; + await gate; + }, + }); + + const send = () => + request(app).post("/resource").set("Idempotency-Key", "key-3").send({ amount: 7 }); + + const inflight = Array.from({ length: 5 }, () => Promise.resolve(send())); + await waitFor(() => calls === 1, { timeout: 1000 }); + await wait(50); + + release(); + const results = await Promise.all(inflight); + const statuses = results.map((r) => r.status).sort(); + + expect(calls).toBe(1); + expect(statuses).toEqual([201, 409, 409, 409, 409]); + const conflicts = results.filter((r) => r.status === 409); + expect(conflicts[0].headers["retry-after"]).toBe("1"); + }); + + it("does not cache non-2xx responses and frees the lock for a retry", async () => { + let calls = 0; + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + req.merchant = { id: "merchant-1" }; + next(); + }); + app.use(createIdempotencyMiddleware({ getClient: () => redis })); + app.post("/resource", (req, res) => { + calls += 1; + res.status(calls === 1 ? 500 : 201).json({ attempt: calls }); + }); + + await request(app).post("/resource").set("Idempotency-Key", "key-4").send({}).expect(500); + await waitFor(() => redis.store.size === 0, { timeout: 500 }); + + const retry = await request(app) + .post("/resource") + .set("Idempotency-Key", "key-4") + .send({}) + .expect(201); + expect(retry.body).toEqual({ attempt: 2 }); + expect(calls).toBe(2); + }); + + it("ignores an expired in-flight lock and lets the retry proceed", async () => { + let calls = 0; + const app = makeApp(createIdempotencyMiddleware({ getClient: () => redis }), { + onRequest: () => { + calls += 1; + }, + }); + const key = buildIdempotencyKey("merchant-1", "key-5"); + await redis.set( + key, + JSON.stringify({ + state: "inflight", + hash: hashPayload({}), + token: "dead-worker-token", + startedAt: new Date().toISOString(), + }), + { PX: 15 }, + ); + + const blocked = await request(app) + .post("/resource") + .set("Idempotency-Key", "key-5") + .send({}); + expect(blocked.status).toBe(409); + + await wait(30); + await request(app).post("/resource").set("Idempotency-Key", "key-5").send({}).expect(201); + expect(calls).toBe(1); + }); + + it("fails open when Redis is unavailable", async () => { + let calls = 0; + const app = makeApp(createIdempotencyMiddleware({ getClient: () => redis }), { + onRequest: () => { + calls += 1; + }, + }); + + redis.failNextGet = true; + await request(app) + .post("/resource") + .set("Idempotency-Key", "key-6") + .send({ amount: 1 }) + .expect(201); + await request(app) + .post("/resource") + .set("Idempotency-Key", "key-6") + .send({ amount: 1 }) + .expect(201); + expect(calls).toBe(2); + }); + + it("does not fail open when an existing payload mismatches", async () => { + const app = makeApp(createIdempotencyMiddleware({ getClient: () => redis })); + await redis.set( + buildIdempotencyKey("merchant-1", "key-7"), + JSON.stringify({ + state: "completed", + hash: hashPayload({ amount: 1 }), + statusCode: 201, + response: { ok: true }, + }), + ); + const res = await request(app) + .post("/resource") + .set("Idempotency-Key", "key-7") + .send({ amount: 2 }); + expect(res.status).toBe(400); + }); + + it("replays legacy records that predate the state machine", async () => { + const app = makeApp(createIdempotencyMiddleware({ getClient: () => redis })); + await redis.set( + buildIdempotencyKey("merchant-1", "legacy"), + JSON.stringify({ hash: hashPayload({ amount: 9 }), response: { ok: true, legacy: true } }), + ); + + const res = await request(app) + .post("/resource") + .set("Idempotency-Key", "legacy") + .send({ amount: 9 }) + .expect(201); + expect(res.body.legacy).toBe(true); + }); + + it("isolates keys per merchant", async () => { + const appA = makeApp(createIdempotencyMiddleware({ getClient: () => redis }), { + merchantId: "merchant-a", + }); + const appB = makeApp(createIdempotencyMiddleware({ getClient: () => redis }), { + merchantId: "merchant-b", + }); + + await request(appA).post("/resource").set("Idempotency-Key", "shared").send({}).expect(201); + await wait(10); + await request(appB).post("/resource").set("Idempotency-Key", "shared").send({}).expect(201); + expect(redis.store.size).toBe(2); + }); + + it("frees the lock when the connection closes before a response is sent", async () => { + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + req.merchant = { id: "merchant-1" }; + next(); + }); + app.use(createIdempotencyMiddleware({ getClient: () => redis })); + app.post("/resource", (req, res) => { + res.socket.destroy(); + }); + + await request(app) + .post("/resource") + .set("Idempotency-Key", "key-8") + .send({}) + .catch(() => {}); + + expect(await waitFor(() => redis.store.size === 0, { timeout: 1000 })).toBe(true); + }); + + it("keeps the in-flight record when the completion write fails", async () => { + redis.eval = async () => { + throw new Error("eval unavailable"); + }; + const app = makeApp(createIdempotencyMiddleware({ getClient: () => redis })); + await request(app).post("/resource").set("Idempotency-Key", "key-9").send({}).expect(201); + expect(redis.store.has(buildIdempotencyKey("merchant-1", "key-9"))).toBe(true); + }); +}); diff --git a/backend/src/lib/metrics.js b/backend/src/lib/metrics.js index e7090b0f..9019bd8a 100644 --- a/backend/src/lib/metrics.js +++ b/backend/src/lib/metrics.js @@ -1144,3 +1144,26 @@ export const auditLogQueueWaitDuration = new client.Histogram({ register.registerMetric(auditLogQueueDepth); register.registerMetric(auditLogQueueWaitDuration); + +// ── Idempotency Key Manager Metrics (Issue #1465) ─────────────────────────── +export const idempotencyRequestsTotal = new client.Counter({ + name: "idempotency_requests_total", + help: "Total number of requests processed by the idempotency middleware", + labelNames: ["result"], // acquired, replay, payload_mismatch, in_flight_conflict, bypassed, redis_error +}); + +export const idempotencyLockDuration = new client.Histogram({ + name: "idempotency_lock_duration_seconds", + help: "Time a request held the idempotency lock before completing or releasing it", + labelNames: ["outcome"], // completed, released, orphaned + buckets: [0.01, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10, 30, 60], +}); + +export const idempotencyOrphanedCompletionsTotal = new client.Counter({ + name: "idempotency_orphaned_completions_total", + help: "Total number of responses not cached because the idempotency lock was lost mid-flight", +}); + +register.registerMetric(idempotencyRequestsTotal); +register.registerMetric(idempotencyLockDuration); +register.registerMetric(idempotencyOrphanedCompletionsTotal);