From 1eb2309d102ace03ac43560456eff5ac76c4cc23 Mon Sep 17 00:00:00 2001 From: Manas Srivastava Date: Sat, 25 Apr 2026 14:43:24 +0530 Subject: [PATCH] fix: /billing/create-subscription is idempotent for "created" status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- internal/server/billing_subscriptions.go | 72 +++++++++++++++++++----- internal/server/schema.sql | 7 +++ 2 files changed, 64 insertions(+), 15 deletions(-) diff --git a/internal/server/billing_subscriptions.go b/internal/server/billing_subscriptions.go index 9ee7b63..6215e27 100644 --- a/internal/server/billing_subscriptions.go +++ b/internal/server/billing_subscriptions.go @@ -2,6 +2,7 @@ package server import ( "context" + "database/sql" "encoding/json" "log/slog" "net/http" @@ -57,6 +58,45 @@ func (s *server) handleCreateSubscription(w http.ResponseWriter, r *http.Request return } + // Idempotency: if the user already has a subscription stuck at status + // 'created' (Razorpay provisioned but customer hasn't signed the + // mandate / completed first charge), return that subscription's + // short_url instead of creating a second one. Without this, every + // double-click on Subscribe spawns a parallel sub on Razorpay's side + // and the webhook for whichever one actually gets paid races to + // resolve back to our user record. We saw this in prod — a user paid + // sub_B but their account row pointed at sub_A, so the charge + // webhook found no matching sub_id and never promoted them. + if user.SubscriptionStatus != nil && *user.SubscriptionStatus == "created" { + idemCtx, idemCancel := context.WithTimeout(r.Context(), 5*time.Second) + defer idemCancel() + var existingSubID, existingShortURL sql.NullString + err := s.db.QueryRowContext(idemCtx, + `SELECT razorpay_subscription_id, razorpay_subscription_short_url + FROM users + WHERE id = $1`, + user.ID, + ).Scan(&existingSubID, &existingShortURL) + if err == nil && existingSubID.Valid && existingShortURL.Valid && existingShortURL.String != "" { + slog.InfoContext(r.Context(), "create-subscription: returning existing pending sub", + "user_id", user.ID, "sub_id", existingSubID.String) + writeJSON(w, http.StatusOK, CreateSubscriptionResponse{ + SubscriptionID: existingSubID.String, + ShortURL: existingShortURL.String, + KeyID: s.cfg.Razorpay.KeyID, + PlanLabel: "Existing pending subscription — complete payment at the same short_url", + }) + return + } + // Fall through if lookup failed or short_url isn't stored (legacy + // row pre-migration). Worst case we create a duplicate, same as + // pre-fix behaviour. + if err != nil { + slog.WarnContext(r.Context(), "create-subscription idempotency lookup failed; falling through", + "error", err, "user_id", user.ID) + } + } + // Currency lock-in: if the user already has a paid plan_currency (from a // prior subscription cycle, even one that's since been cancelled), a new // subscribe must stay in the same currency. Mixing is rejected — not to @@ -137,13 +177,14 @@ func (s *server) handleCreateSubscription(w http.ResponseWriter, r *http.Request // defence in depth behind the explicit check above. if _, err := s.db.ExecContext(ctx, `UPDATE users - SET razorpay_subscription_id = $1, - subscription_status = 'created', - plan_period = $2, - plan_currency = COALESCE(plan_currency, $3), - cancel_email_sent_at = NULL - WHERE id = $4`, - subID, req.Plan, currency, user.ID, + SET razorpay_subscription_id = $1, + razorpay_subscription_short_url = $2, + subscription_status = 'created', + plan_period = $3, + plan_currency = COALESCE(plan_currency, $4), + cancel_email_sent_at = NULL + WHERE id = $5`, + subID, shortURL, req.Plan, currency, user.ID, ); err != nil { slog.ErrorContext(r.Context(), "persist subscription_id failed", "error", err, "user_id", user.ID, "sub_id", subID) } @@ -191,14 +232,15 @@ func (s *server) handleSubscriptionCharged(ctx context.Context, subEntity, payme if _, err := s.db.ExecContext(ctx, `UPDATE users - SET plan_tier = 'paid', - plan_period = $1, - plan_paid_at = NOW(), - razorpay_subscription_id = $2, - subscription_status = 'active', - current_period_end = $3, - plan_currency = COALESCE(plan_currency, $4) - WHERE id = $5`, + SET plan_tier = 'paid', + plan_period = $1, + plan_paid_at = NOW(), + razorpay_subscription_id = $2, + razorpay_subscription_short_url = NULL, + subscription_status = 'active', + current_period_end = $3, + plan_currency = COALESCE(plan_currency, $4) + WHERE id = $5`, period, subID, periodEnd, planCurrency, userID, ); err != nil { slog.Error("subscription.charged: user update failed", "error", err, "user_id", userID, "sub_id", subID) diff --git a/internal/server/schema.sql b/internal/server/schema.sql index 32f2d32..cb1713d 100644 --- a/internal/server/schema.sql +++ b/internal/server/schema.sql @@ -25,6 +25,13 @@ ALTER TABLE users ADD COLUMN IF NOT EXISTS plan_paid_at TIMESTAMPTZ; ALTER TABLE users ADD COLUMN IF NOT EXISTS razorpay_subscription_id TEXT; ALTER TABLE users ADD COLUMN IF NOT EXISTS subscription_status TEXT; ALTER TABLE users ADD COLUMN IF NOT EXISTS current_period_end TIMESTAMPTZ; +-- Stored short_url makes /billing/create-subscription idempotent for users +-- with a still-payable subscription. If the user has subscription_status +-- 'created' (Razorpay sub provisioned but mandate not yet signed), a second +-- /create call returns the existing short_url instead of spinning up a +-- duplicate sub on Razorpay's side. Cleared once the sub transitions to +-- active (payment captured) or terminal (cancelled/halted/completed). +ALTER TABLE users ADD COLUMN IF NOT EXISTS razorpay_subscription_short_url TEXT; -- Email idempotency markers. The webhook handlers, the one-time-order handler, -- and the billing reconciler all race to send confirmation / cancellation