Billing Pitfalls And Troubleshooting

Billing Pitfalls & Troubleshooting

This document covers common mistakes, subtle bugs, and edge cases in the billing system.

Race Conditions

TOCTOU: Check-Then-Act Without Lock

Pitfall: Check if payment is duplicate, then record it — without locking.

// ❌ WRONG
isDuplicate, _ := p.IsDuplicate(ctx, txID)  // Check: not duplicate
if isDuplicate {
    return &PaymentResult{Success: true}, nil
}
// ... context switch; another goroutine also checks and finds not duplicate ...
err := p.recordPayment(ctx, req)            // Act: record (but another thread also records!)
// → DOUBLE-CHARGE

Fix: Use distributed lock.

// ✓ CORRECT
lock, _ := locker.TryLock(ctx, lockKey)
defer lock.Release(ctx)

isDuplicate, _ := p.IsDuplicate(ctx, txID)  // Check + Act both inside lock
if isDuplicate {
    return &PaymentResult{Success: true}, nil
}
err := p.recordPayment(ctx, req)            // Only one thread executes this

See: Distributed Locks For Payments

---

Concurrent Activation

Pitfall: Multiple payment confirmations (webhook + polling) both activate the same subscription.

// Time 1: Webhook arrives
payment1 := &PaymentRequest{InvoiceID: inv123, Source: PaymentSourcePrimerWebhook}
processor.RecordPayment(ctx, payment1)  // Activates subscription
// → Chargebee: subscription.start_date = now, status = active

// Time 1.5: Polling worker also sees same invoice
payment2 := &PaymentRequest{InvoiceID: inv123, Source: PaymentSourcePrimerPolling}
processor.RecordPayment(ctx, payment2)  // Activates again?
// → Without lock: double-activation (idempotency fails)
// → Chargebee: start_date updated twice (not terrible, but unsafe)

Fix: Distributed lock serializes all payment attempts.

// payments/processor.go - RecordPayment always acquires lock first
lock, _ := locker.TryLock(ctx, locks.InvoiceLock(req.InvoiceID))
defer lock.Release(ctx)

// Inside lock: only one thread proceeds
isDuplicate, _ := p.IsDuplicate(ctx, req.TransactionID)
if isDuplicate {
    return &PaymentResult{Success: true, AlreadyProcessed: true}, nil
}
// Safe to record; no other thread can interfere

---

Concurrent Cleanup

Pitfall: Cleanup worker and user action both cancel the same subscription.

// Time 1: Cleanup detects expired checkout
expiredSub := findExpiredCheckout()
cancelSubscription(expiredSub.ID)  // Sets status = cancelled

// Time 1.5: User tries to reactivate (didn't know it was expired)
reactivateSubscription(expiredSub.ID)  // status = non_renewing → active
// → User gets access to expired subscription!

Fix: Validate subscription state before cancellation; use transactions if supported.

// ✓ Safe approach
lock, _ := locker.TryLock(ctx, locks.SubscriptionLock(subID))
defer lock.Release(ctx)

sub, _ := getSubscription(subID)
if sub.Status != "future" || sub.StartDate > now - 1*year {
    return // Not a future subscription, or not expired; skip
}
cancelSubscription(subID)  // Now safe

---

Timezone Gotchas

Chargebee Stores UTC; Comparisons Fail in Local Time

Pitfall: Query subscriptions created “today” without considering timezone.

// ❌ WRONG
today := time.Now()  // Local time
startOfDay := time.Date(today.Year(), today.Month(), today.Day(), 0, 0, 0, 0, time.Local)
endOfDay := startOfDay.AddDate(0, 0, 1)

// Query Chargebee (returns UTC times)
subs, _ := listSubscriptionsCreatedBetween(startOfDay, endOfDay)
// startOfDay is 2026-04-08 09:00:00 PDT = 2026-04-08 16:00:00 UTC
// endOfDay is 2026-04-09 09:00:00 PDT = 2026-04-09 16:00:00 UTC
// But subscriptions created at 2026-04-08 00:00:00 UTC don't match!

Fix: Always convert to UTC and be explicit.

// ✓ CORRECT
today := time.Now().UTC()
startOfDay := time.Date(today.Year(), today.Month(), today.Day(), 0, 0, 0, 0, time.UTC)
endOfDay := startOfDay.AddDate(0, 0, 1)

subs, _ := listSubscriptionsCreatedBetween(startOfDay, endOfDay)
// Now timezone-aware

Renewal Times Drift Across Timezones

Pitfall: User in PST renews on the “same date” as user in EST, but they’re actually different dates in UTC.

User A (PST):  2026-04-08 23:59:59 PST = 2026-04-09 06:59:59 UTC
User B (EST):  2026-04-08 00:00:01 EST = 2026-04-08 04:00:01 UTC

Both think they renewed on 2026-04-08, but in Chargebee (UTC) they're different dates.
Dunning worker runs at 2026-04-08 12:00 UTC:
  - User A's renewal is in the future (not yet due)
  - User B's renewal is overdue

Mitigation: Chargebee stores everything in UTC; don’t try to be timezone-smart. Use renewal date from Chargebee, not local calculations.

---

Free Checkouts (100% Discount)

Pitfall: Granting Access Without Chargebee Confirmation

Pitfall: Coupon gives 100% discount → amount = 0 → skip Primer → immediately grant access → user cancels on the other side → refund issued → user still has access.

// ❌ WRONG
if discountedAmount == 0 {
    grantAccess(userID)  // Grant access immediately
    // But what if Chargebee payment fails? User has access!
}

Fix: Record payment in Chargebee even for zero-amount invoices.

// ✓ CORRECT
// Create invoice in Chargebee with 0 amount
invoice, _ := createInvoiceForItems(customerID, items, coupon)

// Record payment (even though amount = 0)
processor.RecordPayment(ctx, &PaymentRequest{
    InvoiceID:  invoice.ID,
    Amount:     0,
    Source:     domain.PaymentSourceFreeCheckout,
})

// EventPoller will process the invoice and apply credits
// Only then is access granted

---

Pitfall: Saved Payment Method Not Verified for Free Checkouts

Pitfall: 100% discount checkout succeeds without saved payment method → user later expects dunning retry on failed renewal.

User applies 100% coupon, no saved card:
  ✓ Checkout succeeds (free)
  ✓ Subscription created, activated
  ✓ Access granted

Subscription renewal comes due:
  ✗ No saved card for dunning retry
  ✗ User unaware renewal failed (expected renewal to work)

Mitigation: For free checkouts, require saved payment method (Primer vaulting) OR explicitly document the limitation.

// Optional: Validate that user has saved card
if discountedAmount == 0 && !userHasSavedPaymentMethod(userID) {
    return errors.New("100% discount requires saved payment method for renewals")
}

---

Coupon Issues

Pitfall: Coupon Stacking

Pitfall: User applies multiple coupons; Chargebee allows it → discount is larger than expected.

// Coupon 1: 10% off
// Coupon 2: 10% off
// Both applied → 19% total discount (10% + 10% of remainder)
// Expected: 10% total

Mitigation:

1. Chargebee’s standard behavior: “Last coupon wins” (only one active at a time)
2. Our validation: validatecoupon.go checks if coupon is applicable
3. Never add coupons together; let Chargebee handle the limit

See: validatecoupon.go for coupon validation logic

---

Pitfall: Expired Coupon Applied After Subscription Creation

Pitfall: Coupon valid when subscription is created, expires before payment is recorded.

Time 1: Validate coupon → Coupon expires 2026-04-10
Time 2: Create pending subscription with coupon
Time 3: User completes payment (2026-04-11)
        Coupon is now expired!
        But Chargebee invoice already has coupon applied
        → What discount does user get?

Chargebee behavior: Coupon is applied at invoice creation time (subscription creation). If it expires before payment, the discount is still honored on that invoice. This is correct (honors what was valid at checkout time).

Mitigation: Document that coupons are validated at subscription creation, not payment recording. If user delays payment and coupon expires, the original discount applies anyway.

---

Pitfall: Coupon Credits Granted on Renewals

Pitfall: A coupon with credit metadata (for example WELCOME100, 100 bonus credits) stays attached to the subscription, so any credit path that hands the subscription id to the credit calculator grants the bonus again on every renewal.

// ❌ WRONG on a renewal invoice
creditCalculator.CalculateCredits(ctx, events.CalculateCreditsInput{
    ItemPriceID:    itemPriceID,
    SubscriptionID: subscriptionID, // calculator fetches the subscription coupon → +100 every cycle
})

// ✓ CORRECT: renewals grant plan credits only
creditCalculator.CalculateCredits(ctx, events.CalculateCreditsInput{
    ItemPriceID: itemPriceID,
})

Rule: coupon credits are a first-subscription bonus, for every coupon (decided 2026-09-03, FOXY-1162). The renewal path in apps/sirloin/internal/app/services/billing/dunningretry.go and the event poller in apps/sirloin/internal/app/services/billing/events/poller.go pass SubscriptionID only for a first invoice. This includes scheduler-raised first invoices. The retry worker passes InvoiceID for top-ups, SubscriptionID for first invoices, and neither for renewals. Any new credit path that can see a renewal invoice must follow the same rule.

Durable retries: ApplyCreditsData stores subscription_id and first_invoice so a transient credit failure preserves the first-invoice coupon bonus. CreateFailedCreditOperation also sets the row’s subscription_id column for manual-intervention alerts. Rows without these JSON fields retain renewal behavior. The regression test TestRetryApplyCredits_FirstInvoiceKeepsCouponBonus in apps/sirloin/internal/app/worker/retryfailedoperations_test.go checks persisted first-invoice credits and duplicate delivery.

Already-paid recovery: applyCreditsForSettledPayment in apps/sirloin/internal/app/worker/pollprimerpayments.go and applyCreditsForSubscription in apps/sirloin/internal/app/services/billing/submitpaidinvoice.go retrieve the invoice before calculating credits. They pass the subscription coupon source only when Chargebee marks that invoice as first_invoice. A failed invoice lookup leaves credit application pending for recovery, so it cannot silently classify a first invoice as a renewal.

Mixed invoice lines: When a renewal retry lacks an item price, resolveInvoiceCreditDetails in apps/sirloin/internal/app/worker/retryfailedoperations.go selects a plan_item_price line. Additional charge lines must not replace the billed plan because they appear earlier in the invoice. If no plan line is available, the existing subscription-item fallback applies.

---

Currency Issues

Pitfall: Currency Mismatches

Pitfall: User in EUR, selects USD pricing, currency gets confused.

// User is in EU (default EUR)
// User clicks "USD plan" button
// Frontend sends item_price_id = "plan-Starter-USD-Monthly"
// But user's billing address is in EUR country

// Chargebee creates subscription in USD
// But user's account says EUR
// Revenue report is confused (double-count?)

Mitigation:

1. Frontend should respect user’s timezone/currency preference
2. ListProducts() returns prices in all currencies; frontend selects
3. Server should validate that selected currency matches user’s account

See: products/list.go for product listing with currency handling

---

Pitfall: Cross-Currency Coupons

Pitfall: Coupon is valid in USD only, user applies it to EUR subscription.

// Coupon: "10% off USD plans only"
// User: Tries to apply to EUR subscription
// Chargebee: Silently ignores coupon (no error)
// User: Unaware coupon didn't apply

Mitigation: Validate coupon against specific item_price_id (currency-specific).

// validatecoupon.go should check:
if !coupon.ApplicableTo(itemPriceID) {
    return errors.New("coupon not applicable to this plan")
}

---

Renewal Credits & Proration

Pitfall: Renewal Credits Not Subtracted From Upgrade

Pitfall: User has 3 months left on annual plan, upgrades to premium. Should get prorated credit for unused time. If not subtracted, over-charges.

Annual Starter: $348/year, 3 months remaining = $87 credit
Premium upgrade: $1188/year, prorated to 3 months = $297
User should pay: $297 - $87 = $210

If credit not subtracted: User pays $297 + $87 = ❌ WRONG

Mitigation: Chargebee handles this automatically if invoice_immediately = true during subscription change. The created invoice includes both the credit and the prorated charge.

See: payments/processor.go:ActivateSubscription() - sets InvoiceImmediately: true

---

Idempotency Key Pitfalls

Pitfall: Transaction ID Not Globally Unique

Pitfall: Same transaction ID used for different payments.

// Primer transaction: "12345"
// User applies to invoice_A
// Later, user applies same "12345" to invoice_B (copy/paste error?)
// Both invoices marked as paid with same transaction ID
// Idempotency check: "Transaction 12345 already processed" → skips invoice_B
// → invoice_B not recorded; user gets no credits

Mitigation: Transaction ID must be globally unique across all payments. Use:

• Primer transaction ID (already unique)
• If manual: generate UUID and require

// PaymentRequest validation
func (r *PaymentRequest) Validate() error {
    if r.TransactionID == "" {
        return errors.New("transaction_id required")
    }
    // Assume it's already globally unique (from Primer)
    return nil
}

---

Pitfall: Idempotency Key Lost on Retry

Pitfall: Webhook arrives, we record payment with transaction ID. Webhook retried by Primer, but our request handler doesn’t preserve the original transaction ID.

// Primer Webhook 1: transaction_id = "tx_abc123"
// We record payment, store transaction_id in DB

// Primer Webhook 1 Retry (Primer auto-retry): transaction_id = "tx_abc123"
// If we generate NEW transaction ID on each call: "tx_xyz789"
// → Duplicate payment! (Different transaction ID, same underlying Primer tx)

Mitigation: Always use Primer’s transaction ID, never generate a new one.

// submitpaidinvoice.go or primer webhook handler
func handlePrimerWebhook(w http.ResponseWriter, r *http.Request) {
    var event PrimerEvent
    json.Unmarshal(r.Body, &event)

    // Use Primer's ID directly; don't generate new one
    result, _ := processor.RecordPayment(ctx, &PaymentRequest{
        TransactionID: event.TransactionID,  // ← From Primer, globally unique
        InvoiceID:    event.OrderID,
        Amount:       event.Amount,
        Source:       domain.PaymentSourcePrimerWebhook,
    })
}

---

Pitfall: SDK Global State

Pitfall: Chargebee SDK has global state (API key) that gets confused if multiple environments run in the same process.

// Process starts with API key for production
chargebee.SetAPIKey(prodKey)

// Test suite runs, temporarily sets test API key
chargebee.SetAPIKey(testKey)

// Main code runs again... which API key is active?
// → API calls may go to wrong environment

Mitigation:

1. Don’t use SDK global state; wrap in a client struct (already done: chargebee/client.go)
2. Each handler gets its own client instance
3. Tests use mock clients, not real SDK

See: chargebee/client.go - wraps SDK in a Client struct

---

Database Transaction Pitfalls

Pitfall: Chargebee Change + DB Transaction Mismatch

Pitfall: Chargebee update succeeds, but DB transaction fails. Now they’re out of sync.

// Step 1: Update Chargebee
chargebee.UpdateSubscription(subID, params)  // Status updated

// Step 2: Start DB transaction
tx := db.BeginTx(ctx)
tx.UpdateLocalSubscription(subID, ...)
tx.Commit()  // ❌ FAILS (network error)

// Result: Chargebee has new state, DB has old state
// Next query: DB shows old state; user sees inconsistency

Mitigation: Use saga pattern. Update Chargebee first, then DB. If DB fails, saga is incomplete but safe to retry.

// ✓ Correct order
// Step 1: Update Chargebee (external)
chargebee.UpdateSubscription(subID, params)

// Step 2: Record in DB (internal)
// If this fails, Chargebee has committed but DB hasn't
// Next retry will see the Chargebee update and catch up

db.UpdateLocalSubscription(subID, ...)

See: Saga Pattern For Distributed Payments

---

New Type Boundaries

Pitfall: Money.Multiply() Isn’t Currency-Aware

Pitfall: Multiply USD amount by 12 (yearly multiplier), forget that different currencies have different precision.

// USD: $100.00 = 10000 cents
// JPY: ¥10000 = 10000 (no cents; 1 yen is smallest unit)

// Code assumes cents everywhere:
amount := 10000
amount = amount * 12  // = 120000

// For USD: Correct (= $1200.00)
// For JPY: ❌ WRONG ($1200 instead of ¥120000)

Mitigation: Chargebee handles this. All amounts are in the smallest currency unit. We just multiply; don’t overthink.

domain/entities.go

type Money struct {
    Amount   int64
    Currency string
}

func (m *Money) Multiply(factor int) *Money {
    // Chargebee handles precision; we just multiply
    return &Money{
        Amount:   m.Amount * int64(factor),
        Currency: m.Currency,
    }
}

---

Pitfall: Credit Amount Overflow

Pitfall: Large credit amounts overflow int64.

// User's credit: 9223372036854775807 (max int64)
// Add 1 more credit → wraps to -9223372036854775808
// User suddenly has negative credits; exploit!

Mitigation: This is unlikely in practice (would require 9 exabytes of credits), but validate on large transfers.

func (c *CreditBundle) Add(other *CreditBundle) *CreditBundle {
    images := c.Images + other.Images
    if images > MaxCredits {
        log.Warn().Msg("credit overflow prevented")
        images = MaxCredits
    }
    // ...
}

---

API & Chargebee Quirks

Pitfall: Chargebee Returns nil for “Not Found”

Pitfall: Chargebee API returns nil object for 404 (not found), not an error.

result, err := chargebee.RetrieveSubscription(subID)
// If subscription doesn't exist:
// result.Subscription = nil
// err = nil
// ❌ Code assumes `err != nil` means not found

if err != nil {
    return errors.New("subscription not found")  // Never executed!
}

// Code continues with result.Subscription = nil
// → Panic on nil dereference

Fix: Always check for nil object.

result, err := chargebee.RetrieveSubscription(subID)
if err != nil {
    return fmt.Errorf("failed to retrieve: %w", err)
}
if result.Subscription == nil {  // ← Always check
    return domain.ErrSubscriptionNotFound
}

// Safe to use result.Subscription

---

Pitfall: Chargebee Timestamps Are Seconds, Not Milliseconds

Pitfall: Chargebee returns Unix timestamps in seconds; code assumes milliseconds.

// Chargebee: 1712592000 (April 8, 2026 noon UTC)
// Code: time.Unix(0, 1712592000)  // April 1, 1970 + 1.7s
// ❌ Off by 50+ years!

// Correct: time.Unix(1712592000, 0)

Mitigation: Always use time.Unix(secondsSinceEpoch, 0) when parsing Chargebee timestamps.

---

Pitfall: Chargebee Enum Values Are Strings, Not Constants

Pitfall: Comparing Chargebee status strings without using enums.

// ❌ WRONG (easy to typo)
if sub.Status == "actve" {  // Typo: "actve" instead of "active"
    // This block never executes
}

// ✓ CORRECT
if sub.Status == subscriptionEnum.StatusActive {
    // Using SDK enum constant
}

// ✓ ALSO CORRECT (in domain layer)
if domain.SubscriptionStatus(sub.Status) == domain.SubscriptionStatusActive {
    // Using domain enum
}

---

Testing & Monitoring Pitfalls

Pitfall: Tests Pass Locally, Fail in CI (Timezone)

Pitfall: Test assumes local timezone, but CI runs in UTC.

// ❌ WRONG
func TestRenewalDate(t *testing.T) {
    today := time.Now()
    expectedRenewal := today.AddDate(0, 1, 0)  // 1 month from now
    // Passes locally (local tz); fails in CI (UTC tz)
}

// ✓ CORRECT
func TestRenewalDate(t *testing.T) {
    today := time.Now().UTC()
    expectedRenewal := today.AddDate(0, 1, 0)
    // Works everywhere
}

---

Pitfall: Mock Doesn’t Match Real API

Pitfall: Mock Chargebee client doesn’t enforce constraints that real API does.

// Mock allows:
mock.CreateSubscription(customerID="", itemPriceID="")  // ✓ Mock doesn't care

// Real API rejects:
chargebee.CreateSubscription(customerID="", itemPriceID="")  // ❌ Error: required field

// Test passes with mock, fails with real API

Mitigation: Test with real Chargebee (sandbox) for integration tests. Mocks only for unit tests.

---

Pitfall: Monitoring Doesn’t Catch Silent Failures

Pitfall: Event polling succeeds (returns 0 events) but Chargebee is actually down.

// Poller runs every 15 seconds
result, err := poller.Poll(ctx, opts)
if err != nil {
    log.Error("poll failed")  // Logged
    // But if no error (e.g., Chargebee returns 500 silently as empty result set)
    // Poller thinks: "No new events" (correct)
    // But actually: "API down, all events missed" (not detected)
}

Mitigation: Add healthcheck endpoint; monitor API latency; alert if poll returns empty for >5 minutes.

---

Pitfall: Credit Calculator Built Without a Chargebee Client

Pitfall: events.NewCreditCalculator accepts a nil Chargebee client. The calculator reads item price, item, and coupon metadata through the in-process cache first, so a nil-client calculator works while the cache is warm and fails on the first miss with chargebee client is required for this operation.

// ❌ WRONG: works until the 6-hour item TTL expires or the pod restarts
calc, _ := events.NewCreditCalculator(repo, cacheManager, nil)

// ✓ CORRECT
calc, _ := events.NewCreditCalculator(repo, cacheManager, chargebeeClient)

Why it hides: nothing warms the cache, and the miss surfaces as a warning plus a failed-operation row that another path usually repairs. The renewal path ran this way from May to September 2026 (FOXY-1162): about 40 renewals per day fell through to the event poller, and coupon lookups were skipped on every renewal.

Mitigation: always pass the handler’s client. If a caller genuinely has none, it should not calculate credits.

---

Authorization Attempt Quota Pitfalls

CIT/MIT Buffer Asymmetry

Pitfall: Changing CIT limits without updating MIT limits (or vice versa) breaks the reservation invariant.

CIT enforces limit - reservePaymentAttemptMITBuffer in each window the next renewal can land in and always in the 30-day window (renewalSlotHeld in renewalreservation.go); MIT enforces the full limit. The buffer is applied at the checkout path only, so the registry stores each processor’s real limit and the two paths can no longer drift apart.

Where the numbers live: apps/sirloin/internal/app/services/billing/domain/processorlimits.go, one entry per processor. A processor with no Velocity entry imposes no rolling quota, which is how NMI is expressed — nothing in the enforcement path names a processor. Unattributed traffic (no processor resolved) has its own entry holding the tightest known quota; when measured in August 2026 it was about 9% of declines — 1,134 of 12,147 Primer decline payments over the trailing 30 days carried no processor name. reservePaymentAttemptMITBuffer in apps/sirloin/internal/app/services/billing/reservepaymentattempt.go still holds one slot back for renewals.

VelocityGuard vs QuotaGuard Scope

Pitfall: Confusing which guard runs where.

Guard	What it checks	Where it runs
VelocityGuard (EnforceHighRisk...)	Count windows on authorization_attempt, amount windows on payment_successful	CIT checkout + manual MIT retry only
QuotaGuard (inline MIT gate)	Count windows on authorization_attempt only	All MIT retries (manual + scheduled)
ReservePaymentAttempt	Atomic count + insert authorization_attempt	CIT checkout only (pre-checkout)

Removing or reordering these changes enforcement behavior. VelocityGuard runs only for manual retries. QuotaGuard runs for all MIT retries (manual and scheduled).

PSP Routing Fails Closed

Pitfall: resolveRenewalPSPRouting returns an error (not a default) when Chargebee is unreachable or subscription data is missing. This aborts the dunning retry. It looks like a bug but is intentional — routing to the wrong PSP (e.g., SFW Stripe for an NSFW subscription) is worse than a delayed retry.

Production-Only Enforcement

Pitfall: IsProd() gates all quota enforcement. Staging and development skip it entirely. If you need to test quota behavior locally, you must temporarily remove the IsProd() guard.

Quota-Blocked Dunning Attempts

Pitfall: If a quota-blocked MIT retry does NOT record a dunning_attempt, the DunningAttemptExists check returns false on the next poll cycle, causing the same blocked retry to fire every 2 min (dev) or 15 min (prod) until the dunning window expires. Blocked attempts must set attemptResult so the defer records the row.

Polling Cursor Must Not Advance Past Unresolved Payments

Pitfall: In TaskPollPrimerPayments, returning nil from processSettledPayment counts as success and lets the cursor advance past the payment. Outside the 5-minute overlap window the payment is never re-listed — “retry next tick” silently never comes. This is how orphan-recovery backoff waits used to truncate the 10-attempt budget to 2-3 on busy traffic (waits returned nil), and how a transient detail-fetch failure could permanently disable recovery for a payment.

Mitigation: Skip-but-retry-later states must surface the errOrphanRecoveryBackoff sentinel (or any error), and persistPollingCursor/clampCursorToUnresolved pins the cursor behind the oldest unresolved payment. Return nil only for genuinely terminal outcomes (processed, exhausted-and-alerted).

Logging: Backoff holds log at Debug (orphan recovery in backoff - holding cursor); real failures log at Error per tick.

---

NSFW Payment Method Pitfalls

Pitfall: Wallet Token Sent to Card-Only PSP

Pitfall: User’s primary payment method is Apple Pay or Google Pay. Scheduled NSFW renewal sends the wallet token to EMP (card-only). EMP hard-declines. Subscription enters dunning unnecessarily.

Mitigation: resolveNSFWRenewalPaymentToken in dunningretry.go detects this triple condition (scheduled + NSFW + wallet primary) and substitutes a saved card if one exists. If no card is saved, the wallet token is sent anyway (EMP rejects, dunning proceeds normally).

Logging: Look for nsfw_renewal_card_substitution (card found and swapped) or nsfw_renewal_no_card_found (no card available, proceeding with wallet).

---

Pitfall: NSFW Checkout Doesn’t Set Primary

Pitfall: User completes NSFW checkout with a new card. Primer vaults it but doesn’t set it as default. Next renewal uses the old primary (possibly a wallet), which EMP rejects.

Mitigation: maybeSetNSFWCheckoutPrimary in submitpaidinvoice.go auto-sets the checkout card as default after successful NSFW payment. Non-fatal — if Primer’s SetDefault API fails, the old primary remains.

Logging: Look for nsfw_checkout_set_primary (success) or nsfw_checkout_set_primary_failed (failure).

---

Pitfall: NMI Rejects Cross-PSP Vaulted Cards

Pitfall: Card was vaulted through Stripe (SFW checkout). User switches to NSFW tier. NMI cannot use a Stripe-vaulted token.

Mitigation: Frontend hides vaulted methods on SFW→NSFW tier switch (showVaultedMethods={false}), forcing fresh card entry through the NSFW checkout flow which vaults via the correct PSP.

---

Pitfall: Dropping or Widening the CVV Gate on Stored Cards

Pitfall: Someone retires the CVV prompt on stored cards for everyone “because EMP enabled tokenization”, or keys it on the tier label again so NMI and Cybersource customers see it too.

Why it is wrong: Tokenization changed nothing on eMerchantPay’s terminals: the *-3D-Initial terminals still carry Requires CVV, the *-CVVOFF-Subsequent ones do not, and Primer’s connector picks the terminal by whether 3DS ran. When FOXY-1294 retired the prompt, most EEA saved-card payments on eMerchantPay were refused with code 320 and settled only on the Cybersource fallback, with no conversion gain to show for it (numbers on FOXY-1319). Widening the prompt beyond eMerchantPay adds a field for NMI and Cybersource customers whose processors never asked for it.

Mitigation: Keep requireVaultedCardCvv reading the routed psp from the session metadata (domain.PSPEMP asks, nothing else does). Brisket hides the field by design during a card’s retry cooldown, on an Amex card blocked on the NSFW lane and on an SFW-to-full-access switch, which shows no saved cards; otherwise a missing field for an eMerchantPay-locked customer points at the session’s psp first. Two accepted gaps still send cards to eMerchantPay without the prompt: an unlocked full-access customer, and a Cybersource lock on the Coastal merchant account (FOXY-1179). If eMerchantPay declines stored-card payments, check the terminal in the EMP portal (Payment Transactions → Terminal) and whether the request carried a cvv element before touching the client session: a 3D terminal refusal without a CVV is this gate, a CVVOFF terminal decline is a normal EMP investigation. Watch the drift log line processor lock: settled processor differs from locked psp (workflow drift) and billing.primer_payments by locked against settled processor; the drift line counts only subscription-linked settles and misses top-ups.

---

Processor Assignment Pitfalls

Pitfall: Backfilling Unassigned NSFW Users

Pitfall: Setting nsfw_processor for NSFW users who have an empty value. Since FOXY-596 (settle-then-lock), NSFW users are intentionally unassigned until their first payment settles — Primer’s high-risk workflow routes the first payment and sirloin locks whatever processor + MID actually took the money. Backfilling a guessed processor reintroduces the exact mislock (a vaulted token pinned to a processor it can’t be charged on) the design removed. (The old decideProcessorForNewUser geo/hash split no longer exists.)

Mitigation: Never backfill nsfw_processor. Only MID-backfill for legacy NMI users is safe: SET nsfw_processor_mid = '<MID>' WHERE nsfw_processor = 'nmi' AND (nsfw_processor_mid IS NULL OR nsfw_processor_mid = ''). Never bulk-backfill a Cybersource or EMP MID: since FOXY-1364 an empty MID on those locks marks an existing user and keeps them on the workflows’ default branches.

Pitfall: Assuming Renewals Never Write the Lock

Pitfall: Treating the renewal path as read-only when reasoning about races. overrideProcessorFromCredits still only reads, but the renewal path as a whole is not read-only anymore: an unlocked renewal on either lane calls RepairProcessorLockFromHistory, which persists a lock recovered from the user’s settled Primer history, and the settle-then-lock hook writes the lock on any successful subscription-linked settle (checkout, renewal, or poller).

Mitigation: Every application lock write goes through its lane’s first-wins guard — AssignNSFWProcessorWithMid on NSFW, AssignSFWProcessor on SFW — so concurrent application writers are atomic no-ops for the losers. Manual SQL bypasses the guard, which is why it is the override path described below. Never guess a default at renewal — route unassigned users processor-absent and let the settle lock.

Pitfall: Write-Once Guard Depends on NULL/Empty Check

Pitfall: AssignSFWProcessor and AssignNSFWProcessorWithMid use WHERE sfw_processor IS NULL OR sfw_processor = ''. If a processor is set to a non-empty value by any path, it can never be overwritten through normal code.

Mitigation: This is intentional — processor lock is permanent. Manual SQL is the only override path, and should only be used for incident remediation.

Pitfall: Processor Per-Transaction Limit (NMI $1499 / EMP $2500)

Pitfall: The full-access (NSFW/Pro) processors have hard per-charge ceilings — NMI $1499 and EMP $2500, its max ticket as raised on 2026-08-21. EMP’s ceiling is no longer the same number as its daily settled-amount cap ($3000): the processor separated the two, so a charge can clear the ticket ceiling and still exceed what may settle in a day, and the daily cap is enforced cumulatively in the velocity path rather than here. A single charge above the routed processor’s ceiling fails at the processor level if not caught earlier. Under pricing-v3 this is the annual Pro tiers: faxppr4/5 yearly ($4,008 / $8,028) exceed both caps, faxppr3 yearly ($2,004) now clears EMP but not NMI, and legacy NSFW Ultimate yearly ($1,799) exceeds NMI but not EMP.

Mitigation: CreatePrimerCheckout applies the ceiling after processor resolution on both the new-checkout path and the scheduled-downgrade-into-annual path, through two different entries — grep for either alone and the other looks missing. The new-checkout path calls checkoutExceedsProcessorLimit in apps/sirloin/internal/app/services/billing/createprimercheckout.go, which derives the lane from the plan and delegates; that is the point of the wrapper, since the checkout site must not spell the lane by hand. The scheduled-downgrade path calls exceedsProcessorTransactionLimit directly, where the lane is NSFW by construction. The lane argument only decides the unassigned (processor == "") case, which both lanes can now reach: an NSFW discovery lands on NMI or EMP, so the smaller of their two ceilings applies; an SFW discovery lands on Cybersource or Stripe, neither of which caps a single transaction, so it is not capped at all. If the charge exceeds the routed processor’s cap (domain.MaxChargeCentsForProcessor, NMI $1499 / EMP $2500), checkout is rejected with codes.InvalidArgument / "plan price exceeds processor limit", the pending Chargebee subscription is rolled back, and the frontend shows a toast (annual is also disabled client-side on those tiers). No fraud events are recorded for blocked attempts.

If adding new high-priced plans: Verify the annual price stays under the routed processor’s ceiling (domain.MaxChargeCentsForProcessor in apps/sirloin/internal/app/services/billing/domain/processorlimits.go) or the plan is unpurchasable for that processor’s users. EMP’s rolling weekly ($6,000) and monthly ($10,000) caps are enforced separately in apps/sirloin/internal/app/services/billing/successfulpaymentvelocity.go (high-risk path), not by this per-charge guard.

---

Summary: Testing Checklist

When modifying billing code, verify:

• No race conditions: Concurrent calls are serialized by locks
• Idempotency: Same request processed twice → same result
• Timezone-aware: All time comparisons in UTC
• Chargebee nil-safe: Check for nil objects, not just errors
• Currency-safe: Currency precision handled correctly
• Coupon-safe: Coupons don’t stack unexpectedly
• Free checkout tested: 100% discount works end-to-end
• Renewal tested: Dunning retry succeeds/fails as expected
• Quota guards: CIT reserves MIT buffer, MIT uses full limit
• Monitoring: Metrics and logs sufficient to diagnose
• Backward compatible: Old subscriptions still work
• Processor assignment: Write-once guard holds on every writer; renewals do write (repair and the settle hook) even though the lock read itself is read-only; backfill before NMI flip