fix(subscription): make billing account teardown retryable - #1844
fix(subscription): make billing account teardown retryable#1844whoAbhishekSah wants to merge 1 commit into
Conversation
DeleteByCustomer failed and could not be run again when the provider side was already gone: - a subscription already canceled on the provider now just syncs the local state instead of failing the cancel call - a subscription missing on the provider counts as canceled - offline accounts skip the provider entirely; only local records go - drop the SyncWithProvider call, which failed outright when the provider customer was already deleted Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
rohilsurana
left a comment
rohilsurana
left a comment
There was a problem hiding this comment.
The teardown logic checks out: the early return syncs local state when the provider already canceled, and dropping SyncWithProvider is safe because deleting the Stripe customer later cancels anything still live on the provider. One edge case inline.
Also, no tests cover the new Cancel branch, which is the one piece of real logic in this stack. The concrete Stripe client makes unit tests hard here, so a short note in the PR on how this was verified against Stripe test mode would help.
| return sub, err | ||
| } | ||
|
|
||
| if stripeSubscription != nil && stripeSubscription.Status == stripe.SubscriptionStatusCanceled { |
There was a problem hiding this comment.
createOrGetSchedule also returns a nil schedule for incomplete and incomplete_expired statuses. Those fall through to Subscriptions.Cancel below. Stripe rejects a cancel of an incomplete_expired subscription, and that error is not ErrSubscriptionOnProviderNotFound, so DeleteByCustomer would stop there. It needs local state to say active while the provider says expired, so it is rare, but treating those statuses like canceled here would close the gap.
2 participants
Part of #1835. Stacked on the kyc delete PR.
DeleteByCustomerfailed and could not be run again when the provider side was already gone. That mattered for org delete: after a halfway failure, the Stripe customer was deleted but local rows remained, and every retry died on the first provider call.Changes
SyncWithProvidercall, which failed outright when the provider customer was already deleted.🤖 Generated with Claude Code