The Idempotency Bug That Only Showed Up Under Retries

We had done everything the blog posts tell you to do: every write endpoint required an Idempotency-Key header, we stored keys in Redis, and we checked for an existing key before processing a request. And a customer still got double-charged, from a single client-side retry, because the naive "check then write" pattern has a race condition sitting right in the middle of it that only shows up under real concurrent retries — which is, ironically, the exact scenario idempotency keys exist to handle.

The naive version (this is what most idempotency guides show you)

func HandleCharge(ctx context.Context, key string, req ChargeRequest) (*ChargeResult, error) {
    existing, found := cache.Get(ctx, key)
    if found {
        return existing.(*ChargeResult), nil // already processed, return cached result
    }

    result, err := processCharge(ctx, req) // calls the payment provider
    if err != nil {
        return nil, err
    }

    cache.Set(ctx, key, result, 24*time.Hour)
    return result, nil
}

The race: two requests, one key, one gap

A mobile client on a flaky connection sent a charge request, didn't receive the response in time (the response was actually fine — it was just slow to arrive over a bad connection), and retried with the same idempotency key, as it's supposed to. Both requests hit our service roughly 40ms apart — well within the same request-processing window.

1. Request A checks the cache for `key-123`. Not found.
2. Request B (the retry) checks the cache for `key-123`, 40ms later. Also not found — request A hasn't written its result yet, because it's still waiting on the payment provider's API call.
3. Request A calls the payment provider, gets a successful charge, writes the result to cache under `key-123`.
4. Request B, having already decided "not found" 40ms earlier, proceeds to also call the payment provider — with the same amount, same card — and gets a second successful charge.
5. Request B writes its own result to cache under `key-123`, overwriting request A's (harmless at this point — the damage is already done).

sequenceDiagram
    participant A as Request A
    participant Cache as Redis (idempotency key)
    participant B as Request B (retry, +40ms)
    participant PSP as Payment provider

    A->>Cache: GET key-123
    Cache-->>A: not found
    B->>Cache: GET key-123
    Cache-->>B: not found (A hasn't written yet)
    A->>PSP: charge card
    PSP-->>A: success
    A->>Cache: SET key-123 = result
    B->>PSP: charge card (same amount, same card)
    PSP-->>B: success (2nd charge!)
    B->>Cache: SET key-123 = result (overwrites A's)

The fix: make the check-and-claim atomic

The fix isn't a smarter cache — it's collapsing "check" and "claim" into a single atomic operation, so there's no gap for a second request to slip through. Redis's SET key value NX (set-if-not-exists) does exactly this in one round trip: it either claims the key and proceeds, or discovers someone already claimed it and backs off, atomically, with no window in between.

func HandleCharge(ctx context.Context, key string, req ChargeRequest) (*ChargeResult, error) {
    claimed, err := redisClient.SetNX(ctx, key, "processing", 30*time.Second).Result()
    if err != nil {
        return nil, err
    }

    if !claimed {
        // Someone already claimed this key — either it's still processing,
        // or it's done. Poll for the real result instead of processing again.
        return waitForResult(ctx, key)
    }

    result, err := processCharge(ctx, req)
    if err != nil {
        redisClient.Del(ctx, key) // release the claim so a real retry can proceed
        return nil, err
    }

    redisClient.Set(ctx, key, result, 24*time.Hour)
    return result, nil
}

func waitForResult(ctx context.Context, key string) (*ChargeResult, error) {
    for i := 0; i < 20; i++ { // poll up to ~2s for the in-flight request to finish
        val, err := redisClient.Get(ctx, key).Result()
        if err == nil && val != "processing" {
            return parseResult(val), nil
        }
        time.Sleep(100 * time.Millisecond)
    }
    return nil, errStillProcessing
}

The second gotcha: what if the process crashes mid-charge?

Claiming the key with a TTL (30*time.Second above) matters for a reason beyond the obvious: if the process crashes *after* claiming the key but *before* calling the payment provider, the key would otherwise stay claimed forever, permanently blocking any real retry. The TTL ensures a crashed, stuck claim eventually expires and a genuine retry can proceed — trading a small window of possible duplicate processing (only if a crash happens at exactly the wrong moment and a retry arrives after the TTL expires) for guaranteed forward progress. We chose 30 seconds because it's comfortably longer than our payment provider's worst-case response time, but short enough that a real crash doesn't block a legitimate retry for long.

• - Read-then-write is never safe for anything meant to prevent duplicates under concurrency, no matter how small the code looks.
• - Use an atomic primitive for the claim step: `SETNX` in Redis, a unique constraint + `INSERT ... ON CONFLICT` in Postgres, or a compare-and-swap in whatever store you're using.
• - Always attach a TTL or expiry to the claim, so a crash mid-processing doesn't permanently block legitimate retries.
• - Test this specifically with real concurrent requests in your test suite — not sequential ones. A sequential test will never catch this bug, because the race only exists when the gap between check and write is actually contended.