DEV Community

Cover image for My idempotency library had one job. A dropped connection made it run the payment twice.
Pedro Rogério
Pedro Rogério

Posted on

My idempotency library had one job. A dropped connection made it run the payment twice.

quayside is my idempotency library for Node.js. The whole API fits in one sentence: execute(key, fn) runs a function exactly once per key — if it already ran, you get the stored result back; if it is running right now, you don't run it again. Pluggable storage (memory, Redis, Postgres, MySQL), HTTP adapters for Express, Fastify, Hono and NestJS, fencing tokens enforced inside the store so a holder that lost its lock can never overwrite a newer execution. Zero runtime dependencies.

It shipped at 1.0.0 last week. This post is about the bug that outlived everything I threw at the code before release — a 100% mutation score across 1,254 mutants, a storage contract suite running against real servers, an adversarial review whose 26 verified findings I fixed or triaged — and waited for the final security pass to be found.

It lived in four lines I had written days earlier, to fix a different bug. And it broke the library on the one scenario idempotency libraries exist for.

the one job

Here is the canonical story every idempotency library tells in its README. A client POSTs a payment. The connection drops before the response arrives. The client has no idea whether the charge happened, so it retries with the same Idempotency-Key. Without protection, the customer gets charged twice. With protection, the retry either waits, gets told the first attempt is still running, or replays the stored response.

quayside's mechanism for this is deliberately boring: the atomic create-if-absent write is the lock. execute(key, fn) writes an IN_PROGRESS record before your function runs — there is no separate locking step, so there is no gap between "checked" and "locked". Success transitions the record to COMPLETED and replays it for a result TTL. Failure deletes it so retries run fresh. A crashed process is handled by the lock TTL. And every transition out of IN_PROGRESS is guarded by a fencing token validated inside the storage — Lua on Redis, token-conditional UPDATEs on SQL — so a holder that stalled through a GC pause and lost its lease gets a FencingError from the store itself instead of silently overwriting the new holder's result.

import { Idempotency } from 'quayside'
import { RedisStorage } from 'quayside/redis'

const idempotency = new Idempotency({
  storage: new RedisStorage(redis),
  resultTtl: '24h',   // how long a completed result stays replayable
  lockTtl: '30s'      // how long a crashed execution blocks the key
})

const result = await idempotency.execute('invoice:123', async () => {
  return createPayment()   // runs once; later calls replay the result
})
Enter fullscreen mode Exit fullscreen mode

Dropped connection, then a retry. That is the use case. Hold that thought.

by the numbers, I had permission to feel safe

I have written before about what mutation testing did to a test suite with 100% coverage, so quayside got the same treatment from the start: every mutant dead, none suppressed — when a mutant was equivalent, the rule was to delete or restructure the code until it wasn't, never to annotate it away.

The storage adapters all pass one shared contract suite against real servers via Testcontainers: 50-way concurrency races, SIGKILL crash recovery, split-brain fencing where a stale holder tries to overwrite a newer execution and must be rejected by the store itself. On top of that, an adversarial review pass: 38 candidate findings, 26 surviving verification, the ten worst fixed before the tag.

I am listing all of this not to brag but to set up the fall. The numbers measure the code you wrote. They say nothing about the code you wrote last Tuesday to fix a review finding, which is where this story actually starts.

the review that found a stuck lock

One of those 26 findings was in the Fastify adapter. Fastify's pipeline is hooks, not a wrapping function, so the adapter bridges the engine's continuation across two hooks: preHandler acquires the lock and stashes a deferred; onSend resolves it with the captured response, which the engine then stores.

The finding: onSend is not guaranteed to run. reply.hijack() — the documented escape hatch for SSE, streaming, proxying — skips it. A handler that never answers skips it. In those cases the deferred never settles, and the key stays locked until the lock TTL expires. Thirty seconds of 409s for every retry, with nothing actually running. A real bug, confirmed, worth fixing.

The fix looked obvious. Node's raw ServerResponse emits close for every request, no matter what the framework's lifecycle does. So: settle the capture when the connection closes.

// onSend never runs for a hijacked reply, an aborted connection or a
// handler that never answers. Without this backstop the capture would
// never settle and the key would stay locked until its TTL expired.
reply.raw.on('close', () => { resolveCapture(null) })
Enter fullscreen mode Exit fullscreen mode

Tests green. Mutation score still perfect. I even wrote a test asserting the new behavior — "a reply that never reaches onSend releases the key on close" — and it passed, proudly.

That test was asserting the bug.

the fix was the bug

The security pass flagged those four lines as its single HIGH finding, and the exploit chain reads like a checklist of things I knew individually and failed to compose:

  1. A client POSTs /payments with Idempotency-Key: K. The handler starts the charge.
  2. Fifty milliseconds in, the client aborts the connection. Maybe a flaky mobile network. Maybe on purpose.
  3. Node does not cancel the handler — the charge keeps running in the background.
  4. But close fires on the raw response immediately. My backstop resolves the capture with null.
  5. To the engine, null means "the response was served but is not cacheable" — the code path built for oversized and binary bodies. It releases the record. Nothing is stored under K. The key is free.
  6. The handler finishes the charge. onSend finally runs, tries to resolve the already-settled deferred — a no-op. No error, no warning. Perfect silence.
  7. The client retries with the same key K. The slot is empty. The handler runs again.

One idempotency key. Two charges. Zero log lines.

And notice what the trigger is: a dropped connection followed by a retry. Not an exotic corner. Not an adversarial contortion. The exact scenario from the README, the one the library has as its single job. The deadlock I fixed was real but bounded — thirty seconds of 409s, annoying, visible. The fix I replaced it with was unbounded and invisible: it spent the library's entire reason to exist, silently, on its most common path.

To be precise about what was at stake: this was caught before it ever reached a registry, in the last review gate before release. Nobody was double-charged. But it survived a mutation score of 100% and a full adversarial code review, because both of those examine the code that exists — and this was a composition failure between a lifecycle event and the semantics of null, each of which was individually correct.

a close is a fact about the connection, not about the work

The actual fix is one condition:

reply.raw.on('close', () => {
  if (reply.raw.writableEnded) resolveCapture(null)
})
Enter fullscreen mode Exit fullscreen mode

writableEnded distinguishes the two worlds that close had been conflating. A hijacked reply finished its response before the connection closed — settle it, release the key, that is the stuck-lock fix still working. A connection that died before the response finished tells you nothing about the handler, because the handler is still running. Those keep the lock.

What happens next is the part I find genuinely satisfying. The still-running handler eventually completes, and onSend — which does run in this case, late — stores the outcome. So the retry that arrives during the charge gets a 409 with Retry-After, and the retry that arrives after it gets the stored response replayed. The late completion is not lost work. It is exactly the result the retry came back for.

If the process dies instead, the lock TTL reclaims the key — the same path that has always governed a crashed execution, tested by the same SIGKILL test. The abort case did not need new machinery. It needed to stop being special-cased into the wrong bucket.

the asymmetry that decides it

The lesson I wrote into the design doc afterwards, in bold, because I intend to re-read it:

Releasing a lock early is an integrity failure. Holding one too long is an availability cost. When the two trade off, hold.

The costs are not symmetric, and the pressure on you is inverted from the risk. A held lock is loud — users see 409s, dashboards light up, someone files an issue, and the damage is bounded by a TTL you chose. An early release is silent — everything returns 200, the work just quietly happens twice, and the bound is whatever a duplicated side effect costs your business. When I wrote the close backstop I was optimizing away the loud, bounded problem, and I paid for it with the silent, unbounded one.

The same asymmetry shows up anywhere a lease, a lock or a saga step can be torn down by an event that merely correlates with the work being done: process managers reaping "idle" workers mid-transaction, orchestrators treating a lost heartbeat as a finished task, HTTP servers treating a gone client as a gone request. A connection event is a fact about the connection. The work has its own lifecycle, and only the work gets to say when it is over.

what else the adversarial passes caught

Three more, briefly, because they rhyme with the theme — every one of them was invisible to coverage and visible to an adversary:

  • The wait loop trusted the key's identity across time. With onConflict: 'wait', a waiter polls until the holder finishes. But the holder's lock can expire mid-wait and a different payload can take the key over. The waiter was replaying whatever outcome showed up, including one that belonged to someone else's request body. It now re-checks the payload fingerprint on every poll and rejects with the same key-reuse error it would have thrown at the front door.
  • The HTTP kernel stored control flow as state. "This response must not be cached" used to travel as a sentinel error persisted into the store and deleted right after — and a concurrent waiter could observe it in that window and be waved through with no protection at all. The engine now has a first-class ctx.doNotStore(): keep the lock while running, store nothing, leave nothing for anyone to race. Storing control flow as shared state is a race you just haven't met yet.
  • A client-supplied key over the length cap returned a 500. It is client input arriving in a header; it is a 400, with a stable error code, like every other policy limit. Blaming the server pages the wrong person.

try it

quayside is at 1.1.0 — the release after 1.0.0 routed persisted failures through the configurable codec too, so if you encrypt results at rest, your error messages and stack traces are no longer sitting next to the ciphertext in plaintext. Everything in this post ships tested: the abort scenario, the takeover scenario, the sentinel window, all pinned by regression tests that fail on the vulnerable versions.

npm install quayside
Enter fullscreen mode Exit fullscreen mode
import { Idempotency } from 'quayside'
import { PostgresStorage } from 'quayside/postgres'

const idempotency = new Idempotency({ storage: new PostgresStorage(pool) })

// core, no HTTP required — queue consumers, cron jobs, CLIs
await idempotency.execute({ payload: message.value, ignoreFields: ['meta.timestamp'] },
  () => processOrder(message.value))
Enter fullscreen mode Exit fullscreen mode

Four storage backends behind one contract suite, HTTP adapters that replay status, headers and body faithfully per the IETF Idempotency-Key draft, Prometheus and OpenTelemetry integrations, zero runtime dependencies. Repo: github.com/pinceladasdaweb/quayside — the docs include the full state machine and the design notes behind everything above.

If you take one thing from this post, take the asymmetry. The next time an event tempts you to release a lock early because holding it is inconvenient: the inconvenience is bounded and visible, the early release is unbounded and silent. Hold.

Top comments (0)