Skip to content

Commit e76019b

Browse files
fix(billing): v8.1 post-merge hardening
F1: wrap syncOrganizationPlan in try/catch in handleInvoicePaymentSucceeded — DB blip no longer dead-letters handler; emits billing.organization.sync_failed F2: prepend '~' to synthetic event IDs in bumpEventMarkers — same-second lex tiebreaker now sorts synthetic bumps after Stripe evt_* IDs F3: validatePlan warns on unrecognized non-empty planId before returning null F4: add 'unpaid' to RECONCILE_STATUSES so reconciler covers unpaid subs F5: add V8-C2b — canceled subscription in meter mode must be fail-closed
1 parent cd3cde1 commit e76019b

6 files changed

Lines changed: 49 additions & 7 deletions

modules/billing/lib/billing.markerBump.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,6 @@ export const bumpEventMarkers = (family, source) => {
1313
const fieldPrefix = family === 'invoice' ? 'lastInvoiceEvent' : 'lastSubscriptionEvent';
1414
return {
1515
[`${fieldPrefix}CreatedAt`]: Math.floor(ms / 1000),
16-
[`${fieldPrefix}Id`]: `${source}-${ms}`,
16+
[`${fieldPrefix}Id`]: `~${source}-${ms}`,
1717
};
1818
};

modules/billing/services/billing.reconcile.service.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const RECONCILE_PAGE_SIZE = 100;
1515
/**
1616
* Statuses reconciled against Stripe.
1717
*/
18-
const RECONCILE_STATUSES = ['active', 'past_due', 'trialing'];
18+
const RECONCILE_STATUSES = ['active', 'past_due', 'trialing', 'unpaid'];
1919

2020
/**
2121
* Valid plan names from config.

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

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,11 @@ const validPlans = new Set(config.billing?.plans || ['free', 'starter', 'pro', '
4242
* @param {string} plan - The plan name to validate.
4343
* @returns {string|null} The plan name if valid, null otherwise.
4444
*/
45-
const validatePlan = (plan) => (validPlans.has(plan) ? plan : null);
45+
const validatePlan = (plan) => {
46+
if (validPlans.has(plan)) return plan;
47+
if (plan) logger.warn('[billing.webhook] validatePlan: unrecognized planId', { raw: plan, validPlans: [...validPlans] });
48+
return null;
49+
};
4650

4751
/**
4852
* Plan rank lookup — higher index means higher-tier plan.
@@ -601,7 +605,21 @@ const handleInvoicePaymentSucceeded = async (invoice, event) => {
601605
}
602606

603607
if (isPastDue && resolvedPlan) {
604-
await syncOrganizationPlan(organizationId, resolvedPlan);
608+
try {
609+
await syncOrganizationPlan(organizationId, resolvedPlan);
610+
} catch (syncErr) {
611+
logger.error('[billing.webhook] syncOrganizationPlan failed (non-fatal)', {
612+
organizationId,
613+
error: syncErr?.message ?? String(syncErr),
614+
});
615+
try {
616+
billingEvents.emit('billing.organization.sync_failed', { organizationId, source: 'dunning_recovery' });
617+
} catch (evtErr) {
618+
logger.error('[billing.webhook] billing.organization.sync_failed listener error (non-fatal)', {
619+
error: evtErr?.message ?? String(evtErr),
620+
});
621+
}
622+
}
605623
}
606624
};
607625

modules/billing/tests/billing.admin.service.unit.tests.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ describe('BillingAdminService unit tests:', () => {
449449

450450
expect(lastSubscriptionEventCreatedAt).toBeGreaterThanOrEqual(before);
451451
expect(lastSubscriptionEventCreatedAt).toBeLessThanOrEqual(after);
452-
expect(lastSubscriptionEventId).toMatch(/^admin-cancel-\d+$/);
452+
expect(lastSubscriptionEventId).toMatch(/^~admin-cancel-\d+$/);
453453
});
454454
});
455455

modules/billing/tests/billing.quota.unit.tests.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -621,5 +621,29 @@ describe('requireQuota middleware:', () => {
621621
// Fail-closed branch must short-circuit before the meter check
622622
expect(mockBillingUsageService.getMeter).not.toHaveBeenCalled();
623623
});
624+
625+
test('V8-C2b: canceled subscription in meter mode → 402 METER_EXHAUSTED with free quota (not paid)', async () => {
626+
// Org has status='canceled' (subscription ended). Paid plan must NOT bleed through.
627+
// Fail-closed branch routes to free plan, same as incomplete.
628+
mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'pro', status: 'canceled' });
629+
mockBillingUsageService.getMeter.mockResolvedValue(null);
630+
mockBillingExtraBalanceRepository.getBalance.mockResolvedValue(0);
631+
mockBillingPlanService.getActivePlan.mockReturnValue({ meterQuota: 0, version: 'v1' });
632+
633+
await requireQuota('scraps', 'create')(req, res, next);
634+
635+
expect(next).not.toHaveBeenCalled();
636+
expect(res.status).toHaveBeenCalledWith(402);
637+
const payload = res.json.mock.calls[0][0];
638+
const errData = JSON.parse(payload.error);
639+
expect(errData.type).toBe('METER_EXHAUSTED');
640+
// Free plan quota (0), not the pro paid quota
641+
expect(errData.meterQuota).toBe(0);
642+
// getActivePlan called with the free/default plan id, not 'pro'
643+
expect(mockBillingPlanService.getActivePlan).toHaveBeenCalledWith('free');
644+
expect(mockBillingPlanService.getActivePlan).not.toHaveBeenCalledWith('pro');
645+
// Fail-closed branch must short-circuit before the meter check
646+
expect(mockBillingUsageService.getMeter).not.toHaveBeenCalled();
647+
});
624648
});
625649
});

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ describe('BillingSubscriptionRepository unit tests:', () => {
409409

410410
expect(lastSubscriptionEventCreatedAt).toBeGreaterThanOrEqual(before);
411411
expect(lastSubscriptionEventCreatedAt).toBeLessThanOrEqual(after);
412-
expect(lastSubscriptionEventId).toMatch(/^admin-bump-\d+$/);
412+
expect(lastSubscriptionEventId).toMatch(/^~admin-bump-\d+$/);
413413
});
414414
});
415415

@@ -433,7 +433,7 @@ describe('BillingSubscriptionRepository unit tests:', () => {
433433

434434
expect(lastSubscriptionEventCreatedAt).toBeGreaterThanOrEqual(before);
435435
expect(lastSubscriptionEventCreatedAt).toBeLessThanOrEqual(after);
436-
expect(lastSubscriptionEventId).toMatch(/^dunning-\d+$/);
436+
expect(lastSubscriptionEventId).toMatch(/^~dunning-\d+$/);
437437
});
438438
});
439439
});

0 commit comments

Comments
 (0)