fix(billing): drop over-deep Stripe expand at three sister sites of /sync#3872
Merged
Conversation
…sync The /sync hotfix (#3853) replaced its over-deep `data.items.data.price.product` expand with a separate `subscriptions.list` + per-product fetch. Three sister sites kept the broken pattern: - `dedup-on-subscription-created.ts`: 5-level expand on `subscriptions.list`. Catch swallowed the Stripe error and fell through to `no_duplicate`, silently disabling the cross-path duplicate-subscription guard from #3245. - `stripe-sub-reflected-in-org-row` invariant: 5-level expand on `subscriptions.list`. Threw on every run, so Lina/Adzymic-class detection never fired. - `/api/admin/backfill-revenue`: 6-level `subscriptions.data.items.data.price.product` on `customers.retrieve`. Whole loop crashed per customer. Drop expand at all three; classify by lookup_key with a per-product `products.retrieve` fallback (cached) for founding-era prices. Tests stay green: dedup tier-label fallback to `lookup_key` was already present, and the invariant tests pass `product` as an inline object that `isMembershipProductByMetadata` already accepts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…g-era test fidelity Code/security review on PR #3872 surfaced three real issues: - /backfill-revenue used limit:10 on subscriptions.list — could silently truncate the membership sub on customers with many lifetime subs. Bumped to 100, matching the dedup helper. - The invariant's orphan-customer violation reads sub.customer.email, but the new code path doesn't expand customer, so the field always serialized null and admins lost the triage hint. Added expand: ['data.customer'] (2 levels, well under the limit). - Founding-era invariant tests (Bidcliq, Equativ) passed `product` as an inline object, masking the real Stripe response shape. Rewrote to pass product as a string id, mock products.retrieve, and assert it was called — actually exercises the new fetch fallback path. Added a cache-coalescing test and a fetch-failure swallow test. Also adds direct unit tests for the new isMembershipSubWithProductFetch helper covering lookup_key fast path, metadata fallback, cache reuse, and the silent-false-on-fetch-error path that's easy to misuse. Test count: 145 passing (+34 from prior commit). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The /sync hotfix in #3853 replaced its over-deep
data.items.data.price.productexpand with a separatesubscriptions.list+ per-product fetch. Three sister sites still had the broken pattern and were silently failing on Stripe's 4-level expand limit:dedup-on-subscription-created.ts:117— 5-level expand onsubscriptions.list. Thetry/catchswallowed the Stripe error and fell through tono_duplicate, silently disabling the cross-path duplicate-subscription guard from Webhook dedup: revisit cancel-newer policy and add customer-facing apology #3245. Highest impact — dedup hasn't been running since the expand limit shipped.stripe-sub-reflected-in-org-rowinvariant — 5-level expand onsubscriptions.list. Threw on every run, so the Lina/Adzymic-class detection wasn't firing./api/admin/backfill-revenue(http.ts:5223) — 6-levelsubscriptions.data.items.data.price.productoncustomers.retrieve. Whole loop crashed per customer.The pattern at all three is: drop the over-deep expand, classify by
lookup_keywith a per-productproducts.retrievefallback (cached) for founding-era prices that lack theaao_membership_*convention. Same shape as #3853.Adds shared helper
isMembershipSubWithProductFetchinbilling/membership-prices(per-sub filter case, sibling to existingpickMembershipSubWithProductFetch).The Slack alert that prompted this (
admin-accounts-billingsource,data.items.data.price.productin the message) was probably from before #3853 deployed — the /sync route in current main can no longer trigger that error. But the three sister sites would have started surfacing it once exercised.Test plan
npx vitest run tests/unit/billing tests/unit/integrity— 111 passedtsc --noEmitcleanPOST /api/admin/accounts/:orgId/syncagainst an org with a real Stripe customer in dev (no regression vs. fix(billing): drop over-deep Stripe expand in /sync, fetch product per-sub #3853)stripe-sub-reflected-in-org-rowreturns results instead of throwing/api/admin/backfill-revenueagainst a small customer set in dev🤖 Generated with Claude Code