Skip to content

fix: /billing/create-subscription is idempotent for 'created' status#10

Merged
mastermanas805 merged 1 commit intomasterfrom
fix/idempotent-create-subscription
Apr 25, 2026
Merged

fix: /billing/create-subscription is idempotent for 'created' status#10
mastermanas805 merged 1 commit intomasterfrom
fix/idempotent-create-subscription

Conversation

@mastermanas805
Copy link
Copy Markdown
Member

Reproduced in prod today

User clicked Subscribe twice. First click created `sub_ShfPaP1hvNJiTO` (no payment). Second click created `sub_ShfZkBk3TBCU5U`. User paid the second one. The webhook for sub_2 eventually arrived and updated their account, but the time between "paid on Razorpay" and "plan_tier=paid on InstaNode" was unnecessarily long because the user's account row pointed at sub_1 and the billing reconciler — which keys off `users.razorpay_subscription_id` — was uselessly polling sub_1.

The user record is now correct (webhook arrived, sub_id swapped, tier promoted) but the double-sub creation itself is the bug. Fixing it prevents future recurrence.

Fix

Same idempotency pattern as #8 (`/db/new` and `/webhook/new`), keyed on subscription state instead of name:

  • Schema: new column `users.razorpay_subscription_short_url TEXT`. Persisted at sub-create time alongside `razorpay_subscription_id`. Cleared when the sub transitions to `active` (charged).

  • handleCreateSubscription: after the existing 409 on `active`/`authenticated` (`subscriptionStatusBlocksNew`), the new branch covers `created` — return the stored `short_url` instead of creating a parallel sub.

  • handleSubscriptionCharged: clears `razorpay_subscription_short_url` when promoting to `active`.

Edge cases

  • Lookup fails → fall through to create-new (worst case = pre-fix behaviour, no regression).
  • `status='created'` but no `short_url` stored (legacy row pre-migration) → fall through to create-new. Same.
  • User cancels and resubscribes → `subscriptionStatusBlocksNew` returns false for cancelled, idempotency check requires `status='created'`, so a fresh create runs. Correct.

Verification

`go build`, `go vet`, `gofmt`, `go test` all pass.

Post-deploy verify

```bash

First call: creates sub

curl -X POST https://api.instanode.dev/billing/create-subscription \
-H "Authorization: Bearer $JWT" -H 'Content-Type: application/json' \
-d '{"plan":"monthly","currency":"USD"}' | jq .subscription_id

Second call (without paying first): SAME subscription_id + short_url

curl -X POST https://api.instanode.dev/billing/create-subscription \
-H "Authorization: Bearer $JWT" -H 'Content-Type: application/json' \
-d '{"plan":"monthly","currency":"USD"}' | jq .subscription_id
```

Reproduced in prod (mastermanas805 account today): user clicked
Subscribe, payment didn't complete, clicked again → API created a
second Razorpay subscription. User paid the second one. Webhook
for sub_2 fired and tried to update users.razorpay_subscription_id
based on notes.user_id, which DID work — but until that webhook
arrived, the user's account row still pointed at sub_1 and the
billing reconciler (which keys off users.razorpay_subscription_id)
was uselessly polling sub_1.

Two changes:

1. Schema: new column users.razorpay_subscription_short_url. Stores
   the Razorpay-provided short_url at sub creation time so we can
   return it on subsequent /create calls without re-fetching.

2. handleCreateSubscription: after the existing block on
   active/authenticated subs (subscriptionStatusBlocksNew), check
   for status='created' (Razorpay sub provisioned, mandate not yet
   signed). If present and we have a stored short_url, return that
   short_url instead of creating a new sub. The user can complete
   payment at the same hosted Razorpay page; no parallel sub.

3. handleSubscriptionCharged: clears the stored short_url when the
   sub transitions to 'active' — once paid, the short_url is no
   longer relevant and shouldn't be returned to a future Subscribe
   click (which would now hit the active/authenticated 409 block
   anyway, but cleaner state hygiene).

Falls through to create-new on lookup error or missing short_url
(legacy rows pre-migration), so worst case is the same as
pre-fix behaviour.
@mastermanas805 mastermanas805 merged commit 8afffeb into master Apr 25, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant