DEV Community

John Builds
John Builds

Posted on

The same failed payment destroyed one customer's data and left the next one's alone

We had a bug that only fired sometimes, and "sometimes" turned out to be a coin flip on webhook arrival order.

Here's the setup. Our billing code had two constants. One listed the statuses that still grant access. One listed the statuses that trigger destructive cleanup: reverting scheduled work to drafts, revoking API keys, turning features off.

The status a payment processor sets on a first failed invoice was in both lists. Same string, opposite meanings, four lines apart in the same file.

That alone is a bug. What kept it alive was the ordering.

The cleanup path was guarded by "was this subscription active before this webhook?" Two webhooks arrive for one failed payment, and the processor does not promise which lands first:

  • Subscription-updated first: prior status still reads active, guard passes, cleanup runs, the customer's scheduled times are nulled out.
  • Payment-failed first: that handler sets the status directly, so by the time subscription-updated lands the guard already reads false, and cleanup never runs.

Same event, same code, two outcomes decided by network timing. It never reproduced consistently enough for anyone to chase it.

Then I found the test that was protecting it. A spec looped over the "inactive" statuses asserting each one enqueued the cleanup job. Green suite. Fixing the code turned that spec red, because the spec had encoded the wrong behavior as the requirement.

Three things I took from it:

A constant used for access control and a constant used for destructive cleanup must never share a member. If they have to, the shared member needs a comment explaining why. Grep every consumer before you add a status to a list that does irreversible work. Ours had exactly one consumer, so the check cost nothing.

Any handler whose behavior depends on the record's prior state is ordering-dependent. Delivery is guaranteed. Order is not. Derive the guard from the payload, or make the handler idempotent.

Cleanup that nulls a column is not undone by re-activating. Gating access is reversible. Destroying data is not, so it belongs at a terminal state only.

A grace period is not a terminal state. We were treating "the first payment failed and we're going to retry for a few days" as "this person is gone."

Top comments (0)