Skip to content

Commit cd3cde1

Browse files
fix(billing): restore plan after dunning recovery (#3627)
* fix(billing): restore plan after dunning recovery in invoice.payment_succeeded V8 audit C1: when markUnpaid downgrades a sub to plan=free and the user later pays the overdue invoice, re-fetch the live Stripe subscription and write the correct plan back alongside pastDueSince=null + status=active. Prevents paid customers being permanently stuck on free plan if customer.subscription.updated is dead-lettered. Falls back gracefully (warn log, no plan field) on Stripe re-fetch error. * fix(billing): guard getStripe() null + update JSDoc in invoice.payment_succeeded /critical-review medium: add explicit `if (!stripe) throw` so the fallback warn log is meaningful ("Stripe not configured") rather than a cryptic TypeError. /critical-review low: update JSDoc @description to document V8 C1 plan-restore behavior and the non-fatal Stripe re-fetch fallback contract.
1 parent 998e302 commit cd3cde1

2 files changed

Lines changed: 101 additions & 3 deletions

File tree

modules/billing/services/billing.webhook.service.js

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -538,9 +538,13 @@ const handleInvoicePaymentFailed = async (invoice, event) => {
538538
};
539539

540540
/**
541-
* @description Handle invoice.payment_succeeded event — clear degraded mode (pastDueSince).
541+
* @description Handle invoice.payment_succeeded event — clear degraded mode and restore plan.
542542
* When a past-due invoice is finally paid, remove the pastDueSince marker so
543543
* the subscription exits degraded mode on next request.
544+
* V8 C1: also re-fetches the live Stripe subscription to restore the correct plan,
545+
* guarding against the case where customer.subscription.updated is dead-lettered
546+
* after a dunning sweep downgraded the plan to 'free'. Stripe re-fetch failure is
547+
* non-fatal (warn log, plan not restored, pastDueSince+status update still fires).
544548
* Uses updateIfEventNewer to guard against out-of-order webhook delivery (V5 P1 #1).
545549
* @param {Object} invoice - Stripe invoice object
546550
* @param {Object} event - Full Stripe event (with event.created and event.id for ordering)
@@ -561,9 +565,29 @@ const handleInvoicePaymentSucceeded = async (invoice, event) => {
561565
// cheap (no runValidators on user-facing fields) and guards the invoice ordering window.
562566
//
563567
// For past-due subs: include pastDueSince + status so the sub exits degraded mode.
568+
// V8 C1: also restore the plan from Stripe so a dunning-downgraded sub is fully recovered
569+
// even when customer.subscription.updated is dead-lettered.
564570
const isPastDue = existing.pastDueSince !== null && existing.pastDueSince !== undefined;
565571
const fields = isPastDue ? { pastDueSince: null, status: 'active' } : {};
566572

573+
let resolvedPlan = null;
574+
if (isPastDue) {
575+
try {
576+
const stripe = getStripe();
577+
if (!stripe) throw new Error('Stripe not configured');
578+
const stripeSub = await stripe.subscriptions.retrieve(stripeSubscriptionId);
579+
resolvedPlan = resolvePlan(stripeSub);
580+
fields.plan = resolvedPlan;
581+
} catch (stripeErr) {
582+
logger.warn('[billing.webhook] invoice.payment_succeeded — Stripe re-fetch failed, plan not restored', {
583+
stripeSubscriptionId,
584+
error: stripeErr?.message ?? String(stripeErr),
585+
});
586+
}
587+
}
588+
589+
const organizationId = String(existing.organization?._id || existing.organization);
590+
567591
const updated = await SubscriptionRepository.updateIfEventNewer(
568592
String(existing._id),
569593
event.created,
@@ -573,6 +597,11 @@ const handleInvoicePaymentSucceeded = async (invoice, event) => {
573597
);
574598
if (!updated) {
575599
logger.info('[billing.webhook] skipped stale event', { eventId: event.id, type: event.type });
600+
return;
601+
}
602+
603+
if (isPastDue && resolvedPlan) {
604+
await syncOrganizationPlan(organizationId, resolvedPlan);
576605
}
577606
};
578607

modules/billing/tests/billing.webhook.subscription.unit.tests.js

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ describe('Billing webhook subscription unit tests:', () => {
1515
let mockOrganizationRepository;
1616
let mockResetService;
1717
let mockEvents;
18+
let mockStripe;
1819

1920
const orgId = '507f1f77bcf86cd799439011';
2021
const subId = '607f1f77bcf86cd799439022';
@@ -65,6 +66,14 @@ describe('Billing webhook subscription unit tests:', () => {
6566
default: mockResetService,
6667
}));
6768

69+
mockStripe = {
70+
subscriptions: { retrieve: jest.fn() },
71+
};
72+
73+
jest.unstable_mockModule('../lib/stripe.js', () => ({
74+
default: jest.fn(() => mockStripe),
75+
}));
76+
6877
jest.unstable_mockModule('../lib/events.js', () => ({
6978
default: mockEvents,
7079
}));
@@ -441,14 +450,17 @@ describe('Billing webhook subscription unit tests:', () => {
441450
status: 'past_due',
442451
};
443452
mockSubscriptionRepository.findByStripeSubscriptionId.mockResolvedValue(existing);
453+
mockStripe.subscriptions.retrieve.mockResolvedValue({
454+
items: { data: [{ price: { metadata: { planId: 'pro' } } }] },
455+
});
444456

445457
await BillingWebhookService.handleInvoicePaymentSucceeded({ subscription: 'sub_456' }, makeEvent());
446458

447459
expect(mockSubscriptionRepository.updateIfEventNewer).toHaveBeenCalledWith(
448460
subId,
449461
1700000400,
450462
'evt_succeeded',
451-
{ pastDueSince: null, status: 'active' },
463+
expect.objectContaining({ pastDueSince: null, status: 'active' }),
452464
'invoice',
453465
);
454466
});
@@ -485,14 +497,17 @@ describe('Billing webhook subscription unit tests:', () => {
485497
status: 'past_due',
486498
};
487499
mockSubscriptionRepository.findByStripeSubscriptionId.mockResolvedValue(existing);
500+
mockStripe.subscriptions.retrieve.mockResolvedValue({
501+
items: { data: [{ price: { metadata: { planId: 'pro' } } }] },
502+
});
488503

489504
await BillingWebhookService.handleInvoicePaymentSucceeded({ subscription: 'sub_456' }, makeEvent());
490505

491506
expect(mockSubscriptionRepository.updateIfEventNewer).toHaveBeenCalledWith(
492507
subId,
493508
1700000400,
494509
'evt_succeeded',
495-
{ pastDueSince: null, status: 'active' },
510+
expect.objectContaining({ pastDueSince: null, status: 'active' }),
496511
'invoice',
497512
);
498513
});
@@ -520,6 +535,9 @@ describe('Billing webhook subscription unit tests:', () => {
520535
};
521536
mockSubscriptionRepository.findByStripeSubscriptionId.mockResolvedValue(existing);
522537
mockSubscriptionRepository.updateIfEventNewer.mockResolvedValue(null);
538+
mockStripe.subscriptions.retrieve.mockResolvedValue({
539+
items: { data: [{ price: { metadata: { planId: 'pro' } } }] },
540+
});
523541

524542
let mockLogger;
525543
jest.unstable_mockModule('../../../lib/services/logger.js', () => {
@@ -531,6 +549,57 @@ describe('Billing webhook subscription unit tests:', () => {
531549

532550
expect(mockSubscriptionRepository.updateIfEventNewer).toHaveBeenCalled();
533551
});
552+
553+
// V8 audit C1 — dunning recovery plan restoration
554+
test('V8 C1 — dunning recovery: restores plan=pro after unpaid downgrade to free', async () => {
555+
const existing = {
556+
_id: subId,
557+
organization: orgId,
558+
pastDueSince: new Date('2026-04-01'),
559+
status: 'unpaid',
560+
plan: 'free',
561+
};
562+
mockSubscriptionRepository.findByStripeSubscriptionId.mockResolvedValue(existing);
563+
mockStripe.subscriptions.retrieve.mockResolvedValue({
564+
items: { data: [{ price: { metadata: { planId: 'pro' } } }] },
565+
});
566+
567+
await BillingWebhookService.handleInvoicePaymentSucceeded({ subscription: 'sub_456' }, makeEvent());
568+
569+
expect(mockSubscriptionRepository.updateIfEventNewer).toHaveBeenCalledWith(
570+
subId,
571+
1700000400,
572+
'evt_succeeded',
573+
expect.objectContaining({ plan: 'pro', status: 'active', pastDueSince: null }),
574+
'invoice',
575+
);
576+
expect(mockOrganizationRepository.setPlan).toHaveBeenCalledWith(orgId, 'pro');
577+
});
578+
579+
test('V8 C1 — Stripe re-fetch failure: falls back gracefully, does not restore plan', async () => {
580+
const existing = {
581+
_id: subId,
582+
organization: orgId,
583+
pastDueSince: new Date('2026-04-01'),
584+
status: 'unpaid',
585+
plan: 'free',
586+
};
587+
mockSubscriptionRepository.findByStripeSubscriptionId.mockResolvedValue(existing);
588+
mockStripe.subscriptions.retrieve.mockRejectedValue(new Error('Stripe unavailable'));
589+
590+
await expect(
591+
BillingWebhookService.handleInvoicePaymentSucceeded({ subscription: 'sub_456' }, makeEvent()),
592+
).resolves.not.toThrow();
593+
594+
// update still fires but without plan field
595+
expect(mockSubscriptionRepository.updateIfEventNewer).toHaveBeenCalledWith(
596+
subId,
597+
1700000400,
598+
'evt_succeeded',
599+
expect.not.objectContaining({ plan: expect.anything() }),
600+
'invoice',
601+
);
602+
});
534603
});
535604

536605
describe('handleInvoicePaymentFailed', () => {

0 commit comments

Comments
 (0)