Skip to content

fix(subscriptions): prepare for the Stripe webhook version cutover #21122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
david1alvarez wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(subscriptions): prepare for the Stripe webhook version cutover #21122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions libs/payments/webhooks/src/lib/stripe-webhooks.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ jest.mock('@sentry/node', () => {
return {
...actual,
captureException: jest.fn(),
setTag: jest.fn(),
};
});

Expand Down Expand Up @@ -193,6 +194,34 @@ describe('StripeWebhookService', () => {
);
});

it('should tag the event api version in Sentry', async () => {
const mockResponse = CustomerSubscriptionUpdatedResponseFactory();
jest
.spyOn(stripeEventManager, 'constructWebhookEventResponse')
.mockReturnValue(mockResponse);

await stripeWebhookService.handleWebhookEvent({}, 'signature');

expect(Sentry.setTag).toHaveBeenCalledWith(
'stripe_api_version',
mockResponse.event.api_version
);
});

it('should not tag the api version when the signature is rejected', async () => {
jest
.spyOn(stripeEventManager, 'constructWebhookEventResponse')
.mockImplementation(() => {
throw new Error('Webhook signature verification failed');
});

await expect(
stripeWebhookService.handleWebhookEvent({}, 'signature')
).rejects.toThrow('Webhook signature verification failed');

expect(Sentry.setTag).not.toHaveBeenCalled();
});

it('should report exception to Sentry and throw on exception', async () => {
jest
.spyOn(stripeWebhookService as any, 'dispatchEventToHandler')
Expand Down
4 changes: 4 additions & 0 deletions libs/payments/webhooks/src/lib/stripe-webhooks.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ export class StripeWebhookService {
payload,
signature
);
Sentry.setTag(
'stripe_api_version',
webhookEventResponse.event.api_version
);
Comment on lines +33 to +36

const eventAlreadyProcessed = await this.stripeEventManager.isProcessed(
webhookEventResponse.event.id
Expand Down
10 changes: 5 additions & 5 deletions packages/fxa-admin-server/src/subscriptions/subscriptions.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,17 +178,17 @@ describe('Subscription Service', () => {
{
current_period_end: currentPeriodEnd / 1e3,
current_period_start: currentPeriodStart / 1e3,
plan: {
id: planId,
product: productId,
product_name: productName,
},
},
],
},
ended_at: endedAt,
id: subscriptionId,
latest_invoice: latestInvoice,
plan: {
id: planId,
product: productId,
product_name: productName,
},
status,
},
],
Expand Down
7 changes: 3 additions & 4 deletions packages/fxa-admin-server/src/subscriptions/subscriptions.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import { Injectable } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import { singlePlan } from 'fxa-shared/subscriptions/stripe';
import { AbbrevPlan } from 'fxa-shared/subscriptions/types';
import Stripe from 'stripe';
import { MozSubscription } from '../rest/model/moz-subscription.model';
Expand Down Expand Up @@ -112,10 +113,8 @@ export class SubscriptionsService {

for (const subscription of customer?.subscriptions?.data || []) {
// Inspired by code in auth-server payments ;]
const plan = plans.find(
// @ts-ignore
(p) => p.plan_id === subscription.plan.id
);
const subscriptionPlan = singlePlan(subscription);
const plan = plans.find((p) => p.plan_id === subscriptionPlan?.id);

let invoice = subscription.latest_invoice;
if (typeof invoice === 'string') {
Expand Down
120 changes: 118 additions & 2 deletions packages/fxa-auth-server/lib/payments/stripe.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3466,7 +3466,7 @@ describe('StripeHelper', () => {
.mockResolvedValue({ id: productId, name: productName });
pastDueInvoice.payments.data[0].payment.charge = failedChargeCopy;
pastDueSub.latest_invoice = pastDueInvoice;
pastDueSub.plan.product = product1.id;
pastDueSub.items.data[0].plan.product = product1.id;
const input = { data: [pastDueSub] };
const actual = await stripeHelper.subscriptionsToResponse(input);
expect(actual).toEqual(expectedPastDue);
Expand Down Expand Up @@ -3544,7 +3544,7 @@ describe('StripeHelper', () => {
const latestInvoiceItemsEnded =
stripeInvoiceToLatestInvoiceItemsDTO(paidInvoice);
const sub = deepCopy(cancelledSubscription);
sub.plan.product = product1.id;
sub.items.data[0].plan.product = product1.id;
const input = { data: [sub] };
jest
.spyOn(stripeHelper.stripe.invoices, 'retrieve')
Expand Down Expand Up @@ -5644,6 +5644,122 @@ describe('StripeHelper', () => {
);
});

it('reconstructs the old plan from a basil items.data[].price diff', async () => {
const event = deepCopy(eventCustomerSubscriptionUpdated);
const oldPrice = {
id: 'OLD_00000000000000',
recurring: { interval: 'year', interval_count: 1 },
unit_amount: 1000000,
};
event.data.object.cancel_at_period_end = false;
delete event.data.previous_attributes.plan;
event.data.previous_attributes.cancel_at_period_end = false;
event.data.previous_attributes.latest_invoice =
'mock_latest_invoice_id';
event.data.previous_attributes.items = { data: [{ price: oldPrice }] };
const result =
await stripeHelper.extractSubscriptionUpdateEventDetailsForEmail(
event
);
expect(result).toBe(mockUpgradeDowngradeDetails);
assertOnlyExpectedHelperCalledWith(
'extractSubscriptionUpdateUpgradeDowngradeDetailsForEmail',
event.data.object,
expectedBaseUpdateDetails,
mockInvoice,
undefined,
{
...event.data.object.items.data[0].plan,
id: oldPrice.id,
interval: 'year',
interval_count: 1,
}
);
});

it('reconstructs the old plan from a basil items.data[].plan diff', async () => {
const event = deepCopy(eventCustomerSubscriptionUpdated);
const oldItemPlan = {
id: 'OLD_00000000000000',
interval: 'year',
interval_count: 1,
amount: 1000000,
};
event.data.object.cancel_at_period_end = false;
delete event.data.previous_attributes.plan;
event.data.previous_attributes.cancel_at_period_end = false;
event.data.previous_attributes.latest_invoice =
'mock_latest_invoice_id';
event.data.previous_attributes.items = {
data: [{ plan: oldItemPlan }],
};
const result =
await stripeHelper.extractSubscriptionUpdateEventDetailsForEmail(
event
);
expect(result).toBe(mockUpgradeDowngradeDetails);
assertOnlyExpectedHelperCalledWith(
'extractSubscriptionUpdateUpgradeDowngradeDetailsForEmail',
event.data.object,
expectedBaseUpdateDetails,
mockInvoice,
undefined,
{ ...event.data.object.items.data[0].plan, ...oldItemPlan }
);
});

it('keeps the old cadence when a basil price diff omits recurring', async () => {
const event = deepCopy(eventCustomerSubscriptionUpdated);
event.data.object.cancel_at_period_end = false;
delete event.data.previous_attributes.plan;
event.data.previous_attributes.cancel_at_period_end = false;
event.data.previous_attributes.latest_invoice =
'mock_latest_invoice_id';
event.data.previous_attributes.items = {
data: [{ price: { id: 'OLD_00000000000000', unit_amount: 1000000 } }],
};
await stripeHelper.extractSubscriptionUpdateEventDetailsForEmail(event);
const [, , , , planOld] = (
stripeHelper as any
).extractSubscriptionUpdateUpgradeDowngradeDetailsForEmail.mock.calls[0];
expect(planOld.interval).toBe(
event.data.object.items.data[0].plan.interval
);
expect(planOld.interval_count).toBe(
event.data.object.items.data[0].plan.interval_count
);
});

// Stripe's basil changelog says previous_attributes now reports the
// subscription item's billing-period change, so this arrives on every
// renewal. The full prior item is asserted here rather than a partial
// diff, because the real payload shape is not verified.
it('does not send an upgrade when a renewal item diff repeats the current price', async () => {
const event = deepCopy(eventCustomerSubscriptionUpdated);
const currentPlan = event.data.object.items.data[0].plan;
event.data.object.cancel_at_period_end = false;
delete event.data.previous_attributes.plan;
event.data.previous_attributes.latest_invoice =
'mock_latest_invoice_id';
event.data.previous_attributes.items = {
data: [
{
current_period_end: 1326853478,
plan: currentPlan,
price: { id: currentPlan.id, unit_amount: currentPlan.amount },
},
],
};
const result =
await stripeHelper.extractSubscriptionUpdateEventDetailsForEmail(
event
);
expect(result).toEqual(expectedBaseUpdateDetails);
expect(
stripeHelper.extractSubscriptionUpdateUpgradeDowngradeDetailsForEmail
).not.toHaveBeenCalled();
});

it('calls the expected helper method for upgrade or downgrade if previously cancelled', async () => {
const event = deepCopy(eventCustomerSubscriptionUpdated);
event.data.object.cancel_at_period_end = false;
Expand Down
63 changes: 50 additions & 13 deletions packages/fxa-auth-server/lib/payments/stripe.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,10 @@ export const SUBSCRIPTION_PROMOTION_CODE_METADATA_KEY = 'appliedPromotionCode';
* basil shape (parent.subscription_details.subscription), falling back to the
* legacy top-level `subscription` field for invoices still in the acacia shape
* (events emitted by an acacia-era account, or docs cached in the Firestore
* mirror before the basil cutover). Remove once no acacia-era invoices remain.
* mirror before the basil cutover).
*
* PAY-3900: remove with the rest of the dual-format support once the Firestore
* migration has run.
*/
export function getInvoiceSubscription(
invoice: Stripe.Invoice
Expand Down Expand Up @@ -1802,9 +1805,16 @@ export class StripeHelper extends StripeHelperBase {
const rawCoupon = discount?.source?.coupon;
const coupon = typeof rawCoupon === 'object' ? rawCoupon : undefined;

// This type inconsistency runs quite deep, but plan does exist on the subscription here
// for all current use-cases.
const plan = (sub as any).plan as Stripe.Plan;
const plan = singlePlan(sub);
if (!plan) {
throw error.internalValidationError(
'subscriptionsToResponse',
sub,
new Error(
`Multiple items for a subscription not supported: ${sub.id}`
)
);
}

const product = products.find((p) => p.product_id === plan.product);
if (!product)
Expand Down Expand Up @@ -2416,15 +2426,42 @@ export class StripeHelper extends StripeHelperBase {
// Stripe only sends fields that have changed in their previous_attributes field
// Additionally, previous_attributes is a generic field that has no proper typings
// and is used in a flexible manner.
const previousAttributes = eventData.previous_attributes as any;
const planOldDiff = (previousAttributes as any)
.plan as Partial<Stripe.Plan> | null;
const planOld: Stripe.Plan | null = planOldDiff
? {
...planNew,
...planOldDiff,
}
: null;
const previousAttributes: Partial<Stripe.Subscription> & {
plan?: Partial<Stripe.Plan>;
} = eventData.previous_attributes ?? {};
// Acacia sent the previous price as the top-level `plan`; basil removed
// that field and reports item changes under `items.data[]` instead —
// including the billing-period bump on every renewal, so only a moved
// price id is an actual upgrade.
//
Comment on lines +2432 to +2436

@julianpoy julianpoy Sep 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Mind removing these past-present looking comments? I think these types of comments should live in tickets rather than in code

@david1alvarez david1alvarez Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally agreed, though in this particular case it may be worth keeping despite its verbosity. We're going to be revisiting this to remove the dual-shape-handling code, and having this context at that point may be useful.

Want me to keep it with a // TODO: remove comment alongside code after Firestore-Stripe Sync script run, or to just remove it?

// PAY-3900: remove with the rest of the dual-format support once the
// Firestore migration has run.
const previousItem:
| { plan?: Partial<Stripe.Plan>; price?: Partial<Stripe.Price> }
| undefined = previousAttributes.items?.data?.[0];
const previousItemPrice = previousItem?.price;
const itemDiff: Partial<Stripe.Plan> | undefined =
previousItem?.plan ??
(previousItemPrice
? {
id: previousItemPrice.id,
interval: previousItemPrice.recurring?.interval,
interval_count: previousItemPrice.recurring?.interval_count,
}
: undefined);
const planOldDiff: Partial<Stripe.Plan> | undefined =
previousAttributes.plan ??
(itemDiff && itemDiff.id !== planNew.id ? itemDiff : undefined);

let planOld: Stripe.Plan | null = null;
if (planOldDiff) {
planOld = {
...planNew,
...planOldDiff,
interval: planOldDiff.interval ?? planNew.interval,
interval_count: planOldDiff.interval_count ?? planNew.interval_count,
Comment on lines +2461 to +2462
};
}

let invoiceOldCurrency: string | undefined;
let invoiceTotalOldInCents: number | undefined;
Expand Down
1 change: 1 addition & 0 deletions packages/fxa-auth-server/lib/routes/subscriptions/stripe-webhook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ export class StripeWebhookHandler extends StripeHandler {
request.payload,
request.headers['stripe-signature']
);
Sentry.setTag('stripe_api_version', event.api_version);
const firestoreHandled = await this.processEventToFirestore(event);
await this.dispatchEventToHandler(request, event, firestoreHandled);
} catch (error) {
Expand Down
36 changes: 36 additions & 0 deletions packages/fxa-auth-server/lib/routes/subscriptions/stripe-webhooks.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,42 @@ describe('StripeWebhookHandler', () => {
.mockImplementation((fn: any) => fn(scopeSpy));
});

describe('stripe_api_version tag', () => {
it('tags the version off the verified event', async () => {
const setTagSpy = jest
.spyOn(Sentry, 'setTag')
.mockImplementation(() => {});
const createdEvent = deepCopy(eventCustomerUpdated);
StripeWebhookHandlerInstance.stripeHelper.constructWebhookEvent.mockReturnValue(
createdEvent
);

await StripeWebhookHandlerInstance.handleWebhookEvent(request);

expect(setTagSpy).toHaveBeenCalledWith(
'stripe_api_version',
createdEvent.api_version
);
});

it('does not tag when the signature is rejected', async () => {
const setTagSpy = jest
.spyOn(Sentry, 'setTag')
.mockImplementation(() => {});
StripeWebhookHandlerInstance.stripeHelper.constructWebhookEvent.mockImplementation(
() => {
throw new Error('Webhook signature verification failed');
}
);

await expect(
StripeWebhookHandlerInstance.handleWebhookEvent(request)
).rejects.toThrow('Webhook signature verification failed');

expect(setTagSpy).not.toHaveBeenCalled();
});
});

const assertNamedHandlerCalled = (
expectedHandlerName: string | null = null
) => {
Expand Down
Loading