fix(billing): make usage reporting exactly-once (outbox) - #69
Open
keithfawcett wants to merge 2 commits into
Open
fix(billing): make usage reporting exactly-once (outbox)#69keithfawcett wants to merge 2 commits into
keithfawcett wants to merge 2 commits into
Conversation
The Stripe meter-event identifier was tied to a per-run rangeEnd (new Date()),
while the high-water mark only advances after Stripe accepts. A crash (or
timeout) after Stripe accepted but before the mark advanced meant the next
run aggregated an OVERLAPPING window with a NEW identifier — double-billing
the tenant for the same GMV.
Freeze {rangeStart, rangeEnd, amount, identifier} to a pending Config row
BEFORE the Stripe call; clear it only after the mark advances. On the next
run, a surviving pending row is re-sent VERBATIM (same identifier ⇒ Stripe's
Meter Events API dedupes) rather than re-aggregating. No schema change
(uses the per-tenant Config table); added deleteConfig helper.
Tests: a clean run freezes-then-clears; a simulated crash after the meter
call leaves the pending row, and the retry re-sends the SAME identifier +
amount (ignoring GMV accrued in between), proving exactly-once.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two review findings on the exactly-once outbox: - (concurrency) The meter-event identifier was keyed on a per-run rangeEnd, so two runs racing the same period (scheduler + manual /billing/report-usage, or two replicas) each froze a different rangeEnd/identifier and submitted the same GMV twice. Key the identifier on the period START (the high-water mark) instead — identical for concurrent same-period runs, so Stripe dedupes; distinct periods still get distinct keys. - (stale pending) A pending row left by a multi-week outage could never be resent (Stripe rejects meter events past its ~35-day timestamp window), wedging the tenant's reporting forever, and past Stripe's ≥24h dedup window a resend risks double-billing. If the frozen rangeEnd is >30d old, abandon it: advance the mark and ALERT for manual reconciliation (a rare, loud under-report beats a silent double-charge or a permanent wedge). Tests: identifier carries the period-start suffix (not rangeEnd); a 40-day-old pending row is abandoned (no Stripe call, mark advanced, pending cleared). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem (audit #4 — HIGH)
The Stripe meter-event
identifierwas tied to a per-runrangeEnd(new Date()), while the high-water mark (LastUsageReportedAt) only advances after Stripe accepts. So a crash/timeout after Stripe accepted but before the mark advanced left the next run to aggregate an overlapping window with a new identifier — double-billing the tenant for the same GMV. The old comment claimed dedupe, but it only covered a re-run within the same second.Fix (outbox, no schema change)
Freeze
{rangeStart, rangeEnd, amount, identifier}to a pendingConfigrow before the Stripe call; clear it only after the mark advances. On the next run, a surviving pending row is re-sent verbatim (sameidentifier⇒ Stripe's Meter Events API dedupes) instead of re-aggregating. Uses the per-tenantConfigtable (added adeleteConfighelper) — no migration.Crash windows:
Tests
usage-billing.test.ts: a clean run freezes-then-clears and advances the mark; a simulated crash (meter call throws after "accepting") leaves the pending row, and the retry re-sends the same identifier + amount even though extra GMV accrued in between — proving exactly-once. Typecheck clean.🤖 Generated with Claude Code