From ca415728d9d6e04dc5a24a7b54ee05b7708652fd Mon Sep 17 00:00:00 2001 From: Thibault Le Ouay Date: Wed, 9 Sep 2026 11:00:17 +0200 Subject: [PATCH] stripe: fix bug (#2672) * stripe: fix bug * stripe: fix bug * stripe: fix bug * stripe: fix bug --- .../data-table/billing/data-table.tsx | 50 +++++-- packages/api/src/router/stripe/index.ts | 125 +++++++++++++++--- packages/api/src/router/stripe/shared.ts | 118 +++++++++++++++++ packages/api/src/router/stripe/webhook.ts | 110 ++++++++------- 4 files changed, 328 insertions(+), 75 deletions(-) diff --git a/apps/dashboard/src/components/data-table/billing/data-table.tsx b/apps/dashboard/src/components/data-table/billing/data-table.tsx index 4e2e7fcd..5381f06c 100644 --- a/apps/dashboard/src/components/data-table/billing/data-table.tsx +++ b/apps/dashboard/src/components/data-table/billing/data-table.tsx @@ -24,9 +24,11 @@ import { } from "@openstatus/ui/components/ui/table"; import { Tabs, TabsList, TabsTrigger } from "@openstatus/ui/components/ui/tabs"; import { useCookieState } from "@openstatus/ui/hooks/use-cookie-state"; -import { useMutation, useQuery } from "@tanstack/react-query"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { isTRPCClientError } from "@trpc/client"; import { useRouter } from "next/navigation"; import { Fragment, useState, useTransition } from "react"; +import { toast } from "sonner"; import { config as featureGroups, plans } from "@/data/plans"; import { getStripe } from "@/lib/stripe"; @@ -48,6 +50,7 @@ export function DataTable({ restrictTo }: { restrictTo?: WorkspacePlan[] }) { const [currency] = useCookieState("x-currency", "USD"); const trpc = useTRPC(); const router = useRouter(); + const queryClient = useQueryClient(); const [isPending, startTransition] = useTransition(); const { data: workspace } = useQuery(trpc.workspace.get.queryOptions()); @@ -56,8 +59,19 @@ export function DataTable({ restrictTo }: { restrictTo?: WorkspacePlan[] }) { onSuccess: async (data) => { if (!data) return; + // An existing subscriber has the plan swapped on the subscription they + // already have, so there is no checkout to redirect to — only the + // refreshed workspace to pick up. + if (data.type === "updated") { + await queryClient.invalidateQueries({ + queryKey: trpc.workspace.get.queryKey(), + }); + toast.success("Your plan has been updated"); + return; + } + const stripe = await getStripe(); - stripe?.redirectToCheckout({ sessionId: data.id }); + stripe?.redirectToCheckout({ sessionId: data.session.id }); }, }), ); @@ -141,21 +155,29 @@ export function DataTable({ restrictTo }: { restrictTo?: WorkspacePlan[] }) { variant={id === "starter" ? "default" : "outline"} onClick={() => { startTransition(async () => { - if (id === "free") { - await customerPortalMutation.mutateAsync({ + try { + if (id === "free") { + await customerPortalMutation.mutateAsync({ + workspaceSlug: workspace.slug, + returnUrl: `${BASE_URL}/settings/billing`, + }); + return; + } + await checkoutSessionMutation.mutateAsync({ + currency: currency || "USD", + plan: id, + interval, workspaceSlug: workspace.slug, - returnUrl: `${BASE_URL}/settings/billing`, + successUrl: `${BASE_URL}/settings/billing?success=true`, + cancelUrl: `${BASE_URL}/settings/billing`, }); - return; + } catch (error) { + toast.error( + isTRPCClientError(error) + ? error.message + : "Failed to update your plan", + ); } - await checkoutSessionMutation.mutateAsync({ - currency: currency || "USD", - plan: id, - interval, - workspaceSlug: workspace.slug, - successUrl: `${BASE_URL}/settings/billing?success=true`, - cancelUrl: `${BASE_URL}/settings/billing`, - }); }); }} disabled={isPending || isCurrentPlan} diff --git a/packages/api/src/router/stripe/index.ts b/packages/api/src/router/stripe/index.ts index 40e7a582..3d5b9222 100644 --- a/packages/api/src/router/stripe/index.ts +++ b/packages/api/src/router/stripe/index.ts @@ -17,12 +17,16 @@ import { updateAddonInLimits, } from "@openstatus/db/src/schema/plan/utils"; import { countWorkspaceUsage } from "@openstatus/services"; +import { updateWorkspacePlan } from "@openstatus/services/workspace"; import { TRPCError } from "@trpc/server"; -import type { Stripe } from "stripe"; import { z } from "zod"; import { createTRPCRouter, protectedProcedure } from "../../trpc"; -import { stripe } from "./shared"; +import { + buildFromSubscriptionOrThrow, + getCurrentSubscription, + stripe, +} from "./shared"; import { getPlanFromPriceId, getPriceIdForFeature, @@ -163,6 +167,98 @@ export const stripeRouter = createTRPCRouter({ } const priceId = getPriceIdForPlan(opts.input.plan, opts.input.interval); + if (!priceId) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "Invalid plan", + }); + } + + // A customer who already pays gets the plan swapped on the subscription + // they have. Sending them back through checkout would open a *second* + // subscription: both would bill until some unrelated webhook happened to + // retire one, the addon line items would stay behind on the old one, and + // the workspace would be rebuilt from a plan-only subscription — silently + // dropping every addon they bought. + const { current } = await getCurrentSubscription(stripeId); + + if (current) { + const planItem = current.items.data.find((item) => + getPlanFromPriceId(item.price.id), + ); + + if (!planItem) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: + "Your subscription is on a legacy price and cannot be changed here. Contact us and we will move it for you.", + }); + } + + // Stripe rejects mixed billing intervals on one subscription and every + // addon price is monthly, so a yearly plan cannot hold the addon items. + const hasAddons = current.items.data.some( + (item) => item.id !== planItem.id, + ); + + if (opts.input.interval === "yearly" && hasAddons) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: + "Add-ons are billed monthly. Remove them before switching to a yearly plan, or contact us.", + }); + } + + // Classify before mutating Stripe. An item on a price neither table + // knows throws, and throwing *after* the update would leave the + // customer re-priced and billed while the workspace kept the old plan + // — a split the webhook cannot repair either, since it throws on the + // same item. + buildFromSubscriptionOrThrow(current); + + // Only the plan item is listed, so Stripe leaves every other item + // untouched and the addons survive the plan change. Clearing + // `cancel_at_period_end` resumes a subscription the customer had + // scheduled to cancel — choosing a paid plan says they mean to keep + // paying. + const updated = await stripe.subscriptions.update(current.id, { + items: [{ id: planItem.id, price: priceId }], + proration_behavior: "create_prorations", + cancel_at_period_end: false, + }); + + const built = buildFromSubscriptionOrThrow(updated); + + if (!built) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "Invalid plan", + }); + } + + // Sync here rather than waiting on `customer.subscription.updated`, so + // the workspace the dashboard refetches is already correct. The webhook + // rebuilds the same state from the same subscription, so applying it + // again is a no-op. + await updateWorkspacePlan({ + ctx: { + workspace: selectWorkspaceSchema.parse(result), + actor: { type: "user", userId: opts.ctx.user.id }, + db: opts.ctx.db, + }, + input: { + plan: built.plan, + subscriptionId: updated.id, + endsAt: new Date(updated.current_period_end * 1000), + paidUntil: new Date(updated.current_period_end * 1000), + limits: built.limits, + reason: "plan_changed", + }, + }); + + return { type: "updated" as const }; + } + const session = await stripe.checkout.sessions.create({ payment_method_types: ["card"], currency: opts.input.currency, @@ -188,7 +284,7 @@ export const stripeRouter = createTRPCRouter({ opts.input.cancelUrl || `${url}/app/${result.slug}/settings/billing`, }); - return session; + return { type: "checkout" as const, session }; }), addAddon: protectedProcedure @@ -233,15 +329,15 @@ export const stripeRouter = createTRPCRouter({ }); } - const sub = (await stripe.customers.retrieve(stripeId, { - expand: ["subscriptions"], - })) as Stripe.Customer; + // Same "which subscription is current" rule as the plan change and the + // webhooks. `customers.retrieve(expand: ["subscriptions"])` also returns + // the `incomplete` records an abandoned checkout leaves behind, and + // taking the first of those would attach the addon to a subscription + // that never bills — granting the limit for free until a later webhook + // rebuilt it away. + const { current } = await getCurrentSubscription(stripeId); - if (!sub) { - return; - } - - if (!sub.subscriptions?.data[0]?.id) { + if (!current) { return; } @@ -254,10 +350,9 @@ export const stripeRouter = createTRPCRouter({ }); } - const subscriptionId = sub.subscriptions.data[0].id; - const items = await stripe.subscriptionItems.list({ - subscription: subscriptionId, + subscription: current.id, + limit: 100, }); // Stripe rejects mixed billing intervals on one subscription and every @@ -328,7 +423,7 @@ export const stripeRouter = createTRPCRouter({ } else { await stripe.subscriptionItems.create({ price: priceId, - subscription: subscriptionId, + subscription: current.id, quantity, }); } diff --git a/packages/api/src/router/stripe/shared.ts b/packages/api/src/router/stripe/shared.ts index c451967d..87827819 100644 --- a/packages/api/src/router/stripe/shared.ts +++ b/packages/api/src/router/stripe/shared.ts @@ -1,6 +1,8 @@ +import { TRPCError } from "@trpc/server"; import Stripe from "stripe"; import { env } from "../../env"; +import { buildLimitsFromSubscription } from "./utils"; export const stripe = new Stripe(env.STRIPE_SECRET_KEY ?? "", { apiVersion: "2023-08-16", @@ -10,6 +12,122 @@ export const stripe = new Stripe(env.STRIPE_SECRET_KEY ?? "", { }, }); +// An unsupported price is a permanent misconfiguration; surface it as a 400 so +// Stripe stops retrying instead of hammering the endpoint on a 5xx. +export function buildFromSubscriptionOrThrow( + subscription: Stripe.Subscription, +) { + try { + return buildLimitsFromSubscription(subscription); + } catch (e) { + console.error(e); + throw new TRPCError({ + code: "BAD_REQUEST", + message: e instanceof Error ? e.message : "Invalid subscription", + }); + } +} + +// Statuses that mean the customer still has a subscription. `incomplete` and +// `incomplete_expired` are abandoned checkouts, `unpaid` and `paused` no longer +// entitle anything, and `canceled` is gone. +const LIVE_STATUSES: Stripe.Subscription.Status[] = [ + "active", + "trialing", + "past_due", +]; + +/** + * Every subscription that still entitles the customer to something. + * + * Auto-paged on purpose: every abandoned checkout leaves an `incomplete` (then + * `incomplete_expired`) subscription behind and Stripe returns those in an + * unfiltered list, so enough of them push a customer's real subscription past + * the first page. Reading one page would report "nothing left" for a paying + * customer — which is what `customer.subscription.deleted` turns into a + * destructive downgrade. + */ +export async function listLiveSubscriptions(customerId: string) { + const subscriptions = await stripe.subscriptions + .list({ customer: customerId, limit: 100 }) + .autoPagingToArray({ limit: 1000 }); + + return subscriptions.filter((sub) => LIVE_STATUSES.includes(sub.status)); +} + +/** + * Total order over a customer's subscriptions, newest first. `created` is only + * second-granular, so two subscriptions can tie; falling back to the id keeps + * every caller — the "which one is current" pick, the staleness guard and the + * cancellation — resolving a tie the same way instead of each choosing its own + * winner and contradicting the others. + */ +export function isNewerSubscription( + a: Stripe.Subscription, + b: Stripe.Subscription, +) { + if (a.created !== b.created) return a.created > b.created; + return a.id > b.id; +} + +/** + * The customer's live subscriptions and the one the workspace should follow: + * the newest of them. Stripe returns `list` newest-first today but does not + * contract it, so the pick is explicit. + * + * Live rather than strictly `active`: a newer subscription that is `trialing` + * or `past_due` still entitles the customer, and omitting it would let an + * older active one look current and revert the workspace onto its plan. + * + * Always read through this rather than off a webhook payload: Stripe + * guarantees neither delivery order nor exactly-once delivery, and it + * serialises the payload with the API version pinned on the *endpoint* rather + * than the one this client pins. + */ +export async function getCurrentSubscription(customerId: string) { + const live = await listLiveSubscriptions(customerId); + + const current = live.reduce( + (newest, sub) => + newest === undefined || isNewerSubscription(sub, newest) ? sub : newest, + undefined, + ); + + return { live, current }; +} + +/** + * A customer carries exactly one subscription. `current` is the one to keep; + * every subscription that predates it is a leftover that would otherwise keep + * billing. Best-effort — a Stripe failure here must not fail the caller, which + * for a webhook would mean Stripe replaying the whole plan sync. + */ +export async function cancelSupersededSubscriptions( + subscriptions: Stripe.Subscription[], + current: Stripe.Subscription, +) { + // Never retire anything in favour of a subscription that is not itself + // active. A checkout still settling (`incomplete` — async payment method, + // deferred 3DS) would otherwise leave the customer with nothing active, and + // the `customer.subscription.deleted` fired by our own cancellation reads + // that as "they cancelled": `downgradeWorkspaceToFree` then hard-deletes + // pages, deactivates monitors and strips members on the way to free. + if (current.status !== "active") return; + + for (const sub of subscriptions) { + if (sub.id === current.id) continue; + // Only ever cancel *downwards*. A replayed or out-of-order delivery names + // a subscription that may already have been superseded, and retiring the + // newer one on its behalf is the failure this whole path exists to avoid. + if (isNewerSubscription(sub, current)) continue; + try { + await stripe.subscriptions.cancel(sub.id); + } catch (e) { + console.error(`Failed to cancel superseded subscription ${sub.id}:`, e); + } + } +} + export async function cancelSubscription(customer?: string) { if (!customer) return; diff --git a/packages/api/src/router/stripe/webhook.ts b/packages/api/src/router/stripe/webhook.ts index 4367be2e..67031cec 100644 --- a/packages/api/src/router/stripe/webhook.ts +++ b/packages/api/src/router/stripe/webhook.ts @@ -14,22 +14,14 @@ import { z } from "zod"; import { removeDomainFromVercelIfUnused } from "../../lib/vercel"; import { createTRPCRouter, publicProcedure } from "../../trpc"; -import { stripe } from "./shared"; -import { buildLimitsFromSubscription } from "./utils"; - -// An unsupported price is a permanent misconfiguration; surface it as a 400 so -// Stripe stops retrying instead of hammering the endpoint on a 5xx. -function buildFromSubscriptionOrThrow(subscription: Stripe.Subscription) { - try { - return buildLimitsFromSubscription(subscription); - } catch (e) { - console.error(e); - throw new TRPCError({ - code: "BAD_REQUEST", - message: e instanceof Error ? e.message : "Invalid subscription", - }); - } -} +import { + buildFromSubscriptionOrThrow, + cancelSupersededSubscriptions, + getCurrentSubscription, + isNewerSubscription, + listLiveSubscriptions, + stripe, +} from "./shared"; const webhookProcedure = publicProcedure.input( z.object({ @@ -48,17 +40,30 @@ const webhookProcedure = publicProcedure.input( export const webhookRouter = createTRPCRouter({ customerSubscriptionUpdated: webhookProcedure.mutation(async (opts) => { - const subscription = opts.input.event.data.object as Stripe.Subscription; + const eventSubscription = opts.input.event.data + .object as Stripe.Subscription; - if (subscription.status !== "active") { + const customerId = + typeof eventSubscription.customer === "string" + ? eventSubscription.customer + : eventSubscription.customer.id; + + // Deliberately built from Stripe's live state rather than from + // `event.data.object`: Stripe guarantees neither delivery order nor + // exactly-once delivery, so a late or duplicated event would otherwise + // replay an outdated item set — re-enabling an addon the customer just + // removed, or dropping one they just bought. Re-reading makes every + // delivery converge on the same result. It also keeps + // `current_period_end` trustworthy, which the raw payload is not: it is + // serialised with the API version pinned on the Stripe *endpoint*, and + // newer versions moved that field onto the subscription items. + const { live, current } = await getCurrentSubscription(customerId); + + // Nothing live left — `customer.subscription.deleted` owns the downgrade. + if (!current) { return; } - const customerId = - typeof subscription.customer === "string" - ? subscription.customer - : subscription.customer.id; - const ws = await getWorkspaceByStripeId({ input: { stripeId: customerId }, db: opts.ctx.db, @@ -72,14 +77,24 @@ export const webhookRouter = createTRPCRouter({ const oldPlan = ws.plan; - const built = buildFromSubscriptionOrThrow(subscription); + const built = buildFromSubscriptionOrThrow(current); // Subscription has no recognized plan item (e.g. a standalone addon sub); // nothing to sync here, unlike sessionCompleted which always has a plan. + // Bail before cancelling anything: if the newest subscription is not one + // we can classify, the plan may well be carried by an older one, and + // retiring that would leave the workspace paying for nothing. if (!built) { return; } + // The workspace follows the newest active subscription; anything older is + // a leftover from a plan change that went through checkout. Cancelling by + // age rather than by "whichever subscription this event named" is what + // stops a stale event from retiring the subscription the customer is + // actually on. + await cancelSupersededSubscriptions(live, current); + // No `reason` metadata: `customer.subscription.updated` fires on trivial // changes too, so let the audit no-op-skip drop rows where nothing // tracked changed. The `stripe-subscription-updated` actor id still @@ -92,27 +107,13 @@ export const webhookRouter = createTRPCRouter({ }, input: { plan: built.plan, - subscriptionId: subscription.id, - endsAt: new Date(subscription.current_period_end * 1000), - paidUntil: new Date(subscription.current_period_end * 1000), + subscriptionId: current.id, + endsAt: new Date(current.current_period_end * 1000), + paidUntil: new Date(current.current_period_end * 1000), limits: built.limits, }, }); - const allActive = await stripe.subscriptions.list({ - customer: customerId, - status: "active", - }); - - for (const sub of allActive.data) { - if (sub.id === subscription.id) continue; - try { - await stripe.subscriptions.cancel(sub.id); - } catch (e) { - console.error(`Failed to cancel duplicate subscription ${sub.id}:`, e); - } - } - const newPlan = built.plan; if (newPlan !== oldPlan) { const customer = await stripe.customers.retrieve(customerId); @@ -170,6 +171,16 @@ export const webhookRouter = createTRPCRouter({ }); } + // A replayed or late `checkout.session.completed` can name a subscription + // that a newer one has already superseded. Writing it would move the + // workspace back to the older plan while the newer subscription keeps + // billing, so leave the workspace to that subscription's own events. + const { live, current } = await getCurrentSubscription(customerId); + + if (current && isNewerSubscription(current, subscription)) { + return; + } + const built = buildFromSubscriptionOrThrow(subscription); if (!built) { console.error("Invalid plan"); @@ -179,6 +190,11 @@ export const webhookRouter = createTRPCRouter({ }); } + // Checkout always opens a new subscription, so anything else still active + // predates it and would keep billing. Retire it here instead of waiting + // for an unrelated `customer.subscription.updated` to come along. + await cancelSupersededSubscriptions(live, subscription); + await updateWorkspacePlan({ ctx: { workspace: ws, @@ -220,12 +236,14 @@ export const webhookRouter = createTRPCRouter({ ? subscription.customer : subscription.customer.id; - const activeSubscriptions = await stripe.subscriptions.list({ - customer: customerId, - status: "active", - }); + // Only the customer's *last* subscription going away is a downgrade. This + // event also fires for a subscription we retired ourselves as superseded, + // and for one Stripe cancelled after dunning while another still stands — + // in both cases the customer is still subscribed and the cascade below + // would be destructive. + const live = await listLiveSubscriptions(customerId); - if (activeSubscriptions.data.length > 0) { + if (live.length > 0) { return; } -- 2.51.2