The retry that charged your customer twice

The retry that charged your customer twice

A payment API times out. The client doesn't know whether the transfer went through, so it retries. The transfer goes through twice.

Everyone knows the fix: idempotency keys. The client sends a unique key with the request, the server remembers it, and a repeat of the same key returns the original result instead of doing the work again.

Four lines of code. I've written them badly at least three times, in three different ways, and each way failed somewhere I wasn't looking.

The version that looks right

public TransferResult Transfer(AccountId from, AccountId to, Money amount, string idempotencyKey)
{
    if (_seenKeys.Contains(idempotencyKey))
        return TransferResult.AlreadyProcessed();

    var entry = _ledger.Post(from, to, amount);
    _seenKeys.Add(idempotencyKey);

    return TransferResult.Success(entry);
}

This is the shape most people start with, and it is wrong in a way that is very hard to see.

Look at the gap between _ledger.Post and _seenKeys.Add. If the process dies in that gap a deploy, an OOM kill, a pod rescheduled the money has moved and the key was never recorded. The client retries. The money moves again.

Swap the two lines and you get the opposite bug: the key is recorded, the process dies, the transfer never happened, and now the retry is silently swallowed. The customer is told their payment succeeded. It didn't.

There is no ordering of those two lines that is correct. The problem isn't the order. The problem is that they're two operations.

They have to be one write

The key and the ledger entry have to land in the same atomic unit. In a relational database that means the same transaction. In my in-memory implementation it means the same lock:

public TransferResult Transfer(AccountId from, AccountId to, Money amount, string idempotencyKey)
{
    using var _ = _store.Lock(new[] { from, to });

    if (_store.TryGetResult(idempotencyKey, out var existing))
        return existing;

    var entry = _store.PostAndRecord(from, to, amount, idempotencyKey);
    return TransferResult.Success(entry);
}

PostAndRecord writes the journal entries and the idempotency record together, or writes neither.

That's the whole fix for the crash case. If you take one thing from this, take that: an idempotency key that is written separately from the work it guards is not an idempotency key, it's a race.

The second bug: what do you return?

Returning AlreadyProcessed() on a replay feels reasonable. It isn't.

The client retried because it never saw the first response. It doesn't know the transfer succeeded. It needs the result the entry ID, the resulting balances, whatever the original call returned. An "already processed" response tells it nothing it can act on, and the usual next step is a support ticket asking whether the money moved.

So the stored record has to contain the original response, not just the fact that the key was used:

if (_store.TryGetResult(idempotencyKey, out var existing))
    return existing;   // the original TransferResult, replayed verbatim

A retry should be indistinguishable from the first call, from the client's point of view. That is the actual definition of idempotent, and "we already did that" fails it.

The third bug: same key, different payload

This is the one I think is genuinely underrated.

A client reuses an idempotency key bad client code, a key derived from something not unique enough, an order ID recycled after a cancellation. The second request has the same key but asks to transfer a different amount, or to a different account.

If you only check the key, you return the first result. The client believes its second, different request succeeded. It didn't. You've now reported success for an operation you never performed, and the two systems have quietly diverged.

The fix is to store a fingerprint of the request alongside the key and compare:

var fingerprint = Fingerprint.Of(from, to, amount);

if (_store.TryGetRecord(idempotencyKey, out var record))
{
    if (record.Fingerprint != fingerprint)
        throw new IdempotencyConflictException(idempotencyKey);

    return record.Result;
}

Same key, same request replay the result. Same key, different request fail loudly. Stripe returns a 409 for this case and they are right to. A conflict is a bug in the caller, and the worst thing you can do with a bug in the caller is hide it.

What I'd check in a review

Four questions, in order of how much damage they do when the answer is wrong:

1. Is the key written in the same transaction as the work? If it's a separate write, there's a crash window and it will eventually be found.
2. Does a replay return the original response, or an acknowledgement? If it's an acknowledgement, the client still doesn't know what happened.
3. Is the request fingerprinted? If not, key reuse silently corrupts.
4. Does the key expire, and does anything depend on it after it does? Most implementations keep keys for 24 hours. That's a sensible default and a terrible surprise if your client retries a failed batch three days later.

None of these show up in a unit test that calls Transfer twice in a row and asserts the balance. That test passes on every broken version above. The failures live in the gaps a crash between two writes, a client that didn't see a response, a key reused in a way nobody designed for.

Where the code is

The ledger, the idempotency handling, and the concurrency tests are here:

github.com/sunny56/ledger-core

It's a small .NET project that implements the same double-entry ledger three ways naive, pessimistic locking, and optimistic concurrency with a test suite that reproduces the money loss under concurrent access and proves its prevention.

If you've hit a fourth way to get this wrong, I'd like to hear it. I'm fairly sure the list isn't finished.