Reviewing our storefront's order code this week turned up a bug that had been there since the store launched. It never fired, as far as I know. It would have, eventually, on the worst possible day.
The shape of it
Creating an order did three things in sequence:
- Read the product's inventory and check it covers the quantity.
- Call Stripe to create a payment intent.
- Decrement the inventory.
Each step is fine. The order of them is the bug.
Between step one and step three sits a network round trip to Stripe, which takes hundreds of milliseconds. During that window a second customer can arrive, read the same inventory, pass the same check, make their own Stripe call, and decrement too.
Two customers buying the last unit. Both read inventory: 1. Both pass. Both decrement. Stock lands at -1, two orders exist for one item, and nothing anywhere logs an error, because every individual operation succeeded.
This is a classic pattern called check-then-act, and it fails whenever the check and the act are separate operations with a gap between them. The Stripe call made the gap unusually wide, which made the bug unusually likely under any real concurrency.
The fix is to stop checking
The instinct is to add a lock. The better move is to make the database do the check and the decrement as a single operation, by moving the condition into the update's filter:
const result = await products.updateOne(
{ sku, active: true, inventory: { $gte: quantity } },
{ $inc: { inventory: -quantity } }
);
if (result.modifiedCount === 0) {
// someone else took it, or it is gone
}
The database evaluates the filter and applies the update atomically. Two concurrent requests for the last unit both send this; the database processes them one at a time; the first matches and decrements to zero; the second finds inventory >= 1 false and matches nothing. No lock, no retry loop, no window. The loser gets modifiedCount: 0 instead of a negative balance.
The reservation also moved ahead of the Stripe call. Reserve first, then charge. If the charge fails, give the stock back.
The comment that lied
While fixing this I found a comment above the old decrement:
// Reserve inventory (subtract quantities) - will be restored if payment fails
There was no code anywhere in the repository that restored inventory. The only place the inventory field was ever changed was that decrement. Every abandoned checkout, every declined card, every customer who closed the tab after the order was created, permanently consumed the stock it had reserved.
That is arguably the worse bug of the two. The race needs two customers in the same half-second. The phantom restore fires every time anyone changes their mind.
Both are closed now. The reservation is released on every failure path, including the catch block at the bottom of the handler.
Testing a race
The tests model a collection whose updateOne is atomic, the way the real one is, and then run the reservation logic under interleavings that break the old code. Two buyers racing for one unit: exactly one wins, stock is zero. Ten buyers racing for three: exactly three win. A multi-item order that fails on the second item gives the first back.
One more test reproduces the old check-then-act pattern and asserts that it oversells and drives stock to -1. If that test ever starts passing, the concurrency model stopped being able to expose the bug, and every other test in the file is decoration. A test that guards the guards.
Still open, and written down
A customer who abandons checkout after the payment intent is created still holds their reservation, because nothing sweeps stale pending orders. That needs a decision about how long to hold stock for someone mid-checkout, and it is a design choice rather than a bug with one right answer. It is in the commit message so it does not get forgotten.
The general lesson
Any time your code reads a value, decides something, and then writes based on the decision, ask what happens if the value changes between the read and the write. If the answer is "something bad," push the condition into the write. Databases are very good at this. Application code is not.