feat(payments): reconcile stuck pending payments#4795
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const staleAction = isPendingPaymentStatus(observedStatus) | ||
| ? await sendStuckPaymentEmail(dodoPayment, candidate.planKey) | ||
| : undefined; | ||
| const outcome = await ctx.runMutation( | ||
| (internal as any).payments.billing.recordStuckPaymentReconciliationOutcome, | ||
| { | ||
| userId: candidate.userId, | ||
| dodoPaymentId: candidate.dodoPaymentId, | ||
| dodoSubscriptionId: readString(dodoPayment.subscription_id) ?? candidate.dodoSubscriptionId, | ||
| planKey: candidate.planKey, | ||
| amount: readNumber(dodoPayment.total_amount) ?? readNumber(dodoPayment.amount) ?? candidate.amount, | ||
| currency: readString(dodoPayment.currency) ?? candidate.currency, | ||
| pendingOccurredAt: candidate.pendingOccurredAt, | ||
| observedStatus, | ||
| staleAction, | ||
| rawPayload: payment, | ||
| }, | ||
| ); |
There was a problem hiding this comment.
sendStuckPaymentEmail is awaited at line 1229 before recordStuckPaymentReconciliationOutcome inserts the paymentReconciliationAttempts marker. If the subsequent mutation throws (transient DB error, Convex document-size limit hit, or process crash before completion), the email has already been delivered but no marker is recorded. On the next daily cron run, listStuckPendingPaymentCandidates finds no marker for that payment, selects it as a fresh candidate, and sends another email. The customer receives duplicates with no actionable difference between them.
To preserve the "notify at most once" guarantee, the marker should be committed first — ideally as a two-phase approach: claim with ops_notified, then attempt the email outside the mutation and schedule a follow-up mutation to upgrade the action to customer_notified if delivery succeeds.
| const candidates = await ctx.runQuery( | ||
| (internal as any).payments.billing.listStuckPendingPaymentCandidates, | ||
| args, | ||
| ) as StuckPaymentCandidate[]; |
There was a problem hiding this comment.
internal as any casts drop all call-site type checking
Both ctx.runQuery (line 1201) and ctx.runMutation (line 1232) use (internal as any) to reference internal functions. Convex generates strongly-typed internal references; casting to any silences the type checker so any future argument rename, addition, or removal in listStuckPendingPaymentCandidates or recordStuckPaymentReconciliationOutcome would compile successfully and fail at runtime. Using the generated internal.payments.billing.* references directly (without the cast) preserves the full type contract.
| const now = Date.now(); | ||
| const staleBefore = now - thresholdMs; | ||
| const lookbackStart = now - lookbackMs; | ||
| const paymentEvents = await ctx.db | ||
| .query("paymentEvents") | ||
| .withIndex("by_occurredAt", (q) => | ||
| q.gt("occurredAt", lookbackStart).lt("occurredAt", staleBefore), | ||
| ) | ||
| .take(MAX_STUCK_PAYMENT_RECONCILIATION_SCAN_ROWS); | ||
|
|
||
| const candidates: StuckPaymentCandidate[] = []; | ||
| const seenPaymentIds = new Set<string>(); | ||
| for (const ev of paymentEvents) { | ||
| if (candidates.length >= batchSize) break; | ||
| if (ev.type !== "charge") continue; | ||
| if (!isPendingPaymentStatus(ev.status)) continue; | ||
| if (seenPaymentIds.has(ev.dodoPaymentId)) continue; | ||
| seenPaymentIds.add(ev.dodoPaymentId); | ||
|
|
||
| const marker = await ctx.db | ||
| .query("paymentReconciliationAttempts") | ||
| .withIndex("by_dodoPaymentId", (q) => q.eq("dodoPaymentId", ev.dodoPaymentId)) | ||
| .first(); | ||
| if (marker) continue; | ||
|
|
||
| const history = await ctx.db | ||
| .query("paymentEvents") | ||
| .withIndex("by_dodoPaymentId", (q) => q.eq("dodoPaymentId", ev.dodoPaymentId)) | ||
| .collect(); | ||
| if (history.some((row) => row.type === "charge" && isTerminalPaymentStatus(row.status))) { | ||
| continue; | ||
| } | ||
|
|
||
| candidates.push({ | ||
| userId: ev.userId, | ||
| dodoPaymentId: ev.dodoPaymentId, | ||
| dodoSubscriptionId: ev.dodoSubscriptionId, | ||
| planKey: ev.planKey, | ||
| amount: ev.amount, | ||
| currency: ev.currency, | ||
| pendingStatus: ev.status as "processing" | "requires_customer_action", | ||
| pendingOccurredAt: ev.occurredAt, | ||
| }); | ||
| } | ||
|
|
||
| return candidates; | ||
| }, | ||
| }); | ||
|
|
||
| export const recordStuckPaymentReconciliationOutcome = internalMutation({ | ||
| args: { | ||
| userId: v.string(), | ||
| dodoPaymentId: v.string(), |
There was a problem hiding this comment.
listStuckPendingPaymentCandidates
For each unique dodoPaymentId encountered in the up-to-500-row scan, the loop issues two additional queries: one against paymentReconciliationAttempts (marker check) and one collect() against paymentEvents (terminal-event check). In the worst case this is ~2×500 = 1000 round-trips inside a single Convex query. Convex queries have a maximum of 16 384 document reads per transaction and a wall-clock budget; a populated paymentEvents table with many non-charge events in the lookback window could push this toward the limit before the full batch of candidates is filled. A compound index on ["type", "occurredAt"] would let the scan skip non-charge rows at the index layer instead of in application code.
Address the adversarial review of the stuck-pending payment cron: - F1: collapse unrecognised non-terminal Dodo IntentStatus (incl. null / requires_payment_method — the typical abandoned-3DS end-state) to still-pending, writing a marker that records the raw observed status in every fall-through. Previously these returned unknown_status with NO marker → daily re-poll for 14 days + batch-slot starvation. - F2: page ops for real. The still-pending ops branch now console.errors (Convex auto-Sentry, refund-alert precedent) instead of a never-surfaced console.warn. - F3: claim the reconciliation marker BEFORE sending the customer email (idempotency barrier — a post-send failure can't re-email next day), finalize after; split failure reporting by phase (poll / email / record). - F4: gate the "continue checkout" email on link freshness (<24h). Older candidates route to ops rather than emailing a dead Dodo checkout link. - F5: scan newest-first (.order desc) so freshly-stuck rows don't fall off the 500-row scan cap; log when the cap is hit. - F6: guard ctx.scheduler.runAfter with its own try/catch; add AbortSignal.timeout to the Resend POST; stop logging the raw Resend response body (can echo the recipient email). - F7: add reconciliation tests (unknown/null status marker, marker-before- email ordering, already_terminal race, Resend-throw-mid-batch isolation, scan-cap ordering). Claude-Session: https://claude.ai/code/session_018FBmQG921WC4s3qLmuEKoQ
…round 2)
Fold in the reliability + test findings from the ce-code-review pass:
- Bound the Dodo poll (`payments.retrieve`) with `{ timeout: 10s,
maxRetries: 1 }`. The SDK default is 60s x 2 retries (~180s); on a Dodo
outage one degraded poll could otherwise stall the whole sequential
batch — the exact failure this cron exists to survive. Mirrors the
Resend timeout already added.
- Run the reconciliation cron every 6h instead of daily. A payment
becomes a candidate at ~6h pending, so on a daily cadence its age at
first scan is uniformly 6h-30h and anything in (24h, 30h] misses the
24h customer-email freshness gate — ~25% of stuck payments silently
dropped to ops-only. At 6h cadence first-scan age stays <=~12h. Safe:
the action is idempotent and marker-gated.
- Dropped-webhook guard: when a reconciled SUCCEEDED payment carries a
subscription id but has no subscriptions row, console.error (Sentry).
A dropped payment webhook may travel with a dropped subscription.active
webhook — charged customer, no entitlement, and #4794's renewal
reconciler only scans active rows, so nobody else catches it.
- Correct the scan-cap comment/message: newest-first means over-cap OLD
rows are deferred indefinitely (until the backlog clears), not "to a
later run".
- Tests: action-level unrecognised-status (requires_payment_method) path,
two-run at-most-once-email idempotency, and the dropped-subscription
guard (pages / does not page).
Claude-Session: https://claude.ai/code/session_018FBmQG921WC4s3qLmuEKoQ
1 participant
Summary
Stuck Dodo payments now have a scheduled recovery path instead of aging forever after abandoned 3DS or dropped webhook flows. The cron finds stale unresolved pending charge events, polls Dodo for the authoritative status, appends missing terminal payment events when the webhook was lost, and marks still-pending payments once after attempting a customer continuation email.
The still-pending path falls back to an ops-visible marker and structured warning when email delivery is unavailable, so the job remains idempotent and does not re-nag on every run. Poll failures are also surfaced through a scheduled throwing mutation so Convex auto-Sentry captures them without aborting the whole batch.
Fixes #4470.
Validation