Skip to content

Commit 5f12d2a

Browse files
fix(billing): mega hardening v3 — atomicity + DRY + config (#3583)
* fix(billing): mega hardening v3 — atomicity + DRY + config knobs Bundles 3 logical groupings on overlapping surfaces to avoid rebase hell: ATOMICITY (🔴 + 4× 🟠 + 2× 🟡 + 🔵 + ⚪) - markFailedAttempt: single atomic op via aggregation pipeline (close TOCTOU on concurrent crons that double-emit billing.extras_debit.exhausted) - incrementMeterWithOutbox: try/catch outbox create, E11000 fetch existing - zero-cost attribute(): writes idempotency key (close double-charge replay) - forceRotateForPlanChange: planChangeResetTriggered set BEFORE try (close double-rotate via fallback resetWeek) - E11000 retry: legacy consumedHistoryIds filter symmetric (close mid-migration double-charge) - unitsFromCosts: hard-fail null ratioVersion + non-null costs in meterMode - BillingMeterOutbox: timestamps:true (drop manual createdAt) - billing.policy.js: drop stale ESLint disable DRY + lib EXTRACTION (3× 🟠 + 5× 🟡 + 3× 🔵) - webhook.controller: responses.error() canonical envelope (3 sites) - billing.controller: route via BillingExtraService.getOrgBalanceContext() - _ensureStripeCustomer private: dedup find-or-create between checkout flows - billing.processedStripeEvent.schema.js: NEW Zod schema parity - lib/billing.isoWeek.js: NEW currentWeekKey + isoWeekKey single source - findAllDueForReset deprecated: removed - lib/billing.cron-utils.js: NEW applyJitter helper, applied to all 3 crons - billing.plans.controller: unit tests added - lib/billing.errors.js: NEW isDuplicateKeyError single source - lib/billing.constants.js: NEW shared types/constants — kills dynamic imports CONFIG KNOBS SURFACING (NEW) - config.billing.{meter,outbox,crons,planChange,alerts,events}.* knobs - All magic numbers (METER_RUN_BASE, retry limits, jitter window, threshold percentages, fallback plan id, event names) now config-driven with safe defaults. README documents each with downstream override examples. * fix(billing): warn on unsupported threshold percentages + test coverage * fix(billing): named constant for ms-per-day + readme example uses supported threshold values
1 parent fcc7183 commit 5f12d2a

53 files changed

Lines changed: 1262 additions & 424 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

modules/billing/README.md

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,73 @@ Consumers wanting clean-break behavior on downgrade should pass `{ preserveUsage
9999

100100
## Extras debit reliability
101101

102-
`attribute()` returns optimistically after usage increment + outbox row insert. Extras debit happens out of band; if it fails, cron `retry-pending-extras-debit` reconciles within 5min. After 5 failed attempts, the outbox row is marked `failed` and event `billing.extras_debit.exhausted` is emitted for alerting.
102+
`attribute()` returns optimistically after usage increment + outbox row insert. Extras debit happens out of band; if it fails, cron `retry-pending-extras-debit` reconciles on the configured retry interval. After the configured failed-attempt limit, the outbox row is marked `failed` and the configured exhausted event is emitted for alerting.
103103

104104
Consumers should NOT retry on `applied: true` — the outbox handles eventual consistency.
105105

106+
## Meter hardening configuration
107+
108+
Defaults live in `modules/billing/config/billing.development.config.js` and can be overridden by downstream project config:
109+
110+
```js
111+
billing: {
112+
meter: {
113+
runBase: 1,
114+
maxUnitsPerOperation: 10000,
115+
fallbackPlanId: null,
116+
},
117+
outbox: {
118+
maxRetryAttempts: 5,
119+
retryIntervalSec: 300,
120+
},
121+
crons: {
122+
jitterMaxMs: 60_000,
123+
},
124+
planChange: {
125+
preserveUsageDefault: true,
126+
},
127+
alerts: {
128+
thresholdPercents: [80, 100], // only 80 and 100 are supported schema fields; other values warn and are skipped
129+
},
130+
events: {
131+
extrasExhausted: 'billing.extras_debit.exhausted',
132+
},
133+
}
134+
```
135+
136+
Example override:
137+
138+
```js
139+
// config/defaults/production.config.js
140+
export default {
141+
billing: {
142+
meter: {
143+
runBase: 2,
144+
maxUnitsPerOperation: 25000,
145+
fallbackPlanId: 'starter',
146+
},
147+
outbox: {
148+
maxRetryAttempts: 8,
149+
retryIntervalSec: 120,
150+
},
151+
crons: {
152+
jitterMaxMs: 30_000,
153+
},
154+
planChange: {
155+
preserveUsageDefault: false,
156+
},
157+
alerts: {
158+
thresholdPercents: [80, 100],
159+
},
160+
events: {
161+
extrasExhausted: 'billing.extras_debit.exhausted',
162+
},
163+
},
164+
};
165+
```
166+
167+
`meter.runBaseUnits` is still accepted as a backward-compatible alias for `meter.runBase`.
168+
106169
## Stripe — `automatic_tax` flag
107170

108171
```js

modules/billing/billing.init.js

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import config from '../../config/index.js';
66
import AnalyticsService from '../../lib/services/analytics.js';
77
import billingEvents from './lib/events.js';
88
import BillingPlanService from './services/billing.plan.service.js';
9+
import BillingUsageRepository from './repositories/billing.usage.repository.js';
910

1011
/**
1112
* Billing module initialisation.
@@ -29,7 +30,9 @@ export default async (app) => {
2930
billingEvents.on('plan.changed', ({ organizationId, newPlan }) => {
3031
try {
3132
AnalyticsService.groupIdentify('company', String(organizationId), { plan: newPlan });
32-
} catch (_) { /* analytics must not break billing flow */ }
33+
} catch (err) {
34+
console.warn('[billing] analytics groupIdentify failed (non-fatal):', err?.message ?? err);
35+
}
3336
});
3437

3538
try {
@@ -49,6 +52,13 @@ export default async (app) => {
4952
// Runs after ensureSeeded so the plan catalog is up to date.
5053
// Never crashes boot — wrapped in try/catch.
5154
if (config?.billing?.meterMode) {
55+
const legacyUsageCount = await BillingUsageRepository.countLegacyConsumedHistoryIds();
56+
if (legacyUsageCount > 0) {
57+
throw new Error(
58+
`[billing] legacy consumedHistoryIds field still present on ${legacyUsageCount} usage document(s); run migration 20260502100000-rename-consumed-history-ids-to-attribution-keys before enabling meterMode`,
59+
);
60+
}
61+
5262
try {
5363
const Subscription = mongoose.model('Subscription');
5464
const knownPlans = new Set(config.billing.plans ?? []);
@@ -58,8 +68,8 @@ export default async (app) => {
5868
console.warn(`[billing] Subscription.plan value "${plan}" not in planDefinitions — orphaned plan, may resolve quota=0`);
5969
}
6070
}
61-
} catch (_err) {
62-
// Validator failure must NOT crash boot (e.g. model not yet registered at early init)
71+
} catch (err) {
72+
console.warn('[billing] Subscription.plan boot validator failed (non-fatal):', err?.message ?? err);
6373
}
6474
}
6575
};

modules/billing/config/billing.development.config.js

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,14 @@ const config = {
6464
* Meter unit parameters — downstream projects must override with their
6565
* actual unit economics before enabling meterMode in production.
6666
*
67-
* runBaseUnits: flat units charged per run (before per-feature ratios).
67+
* runBase: flat units charged per run when no cost data is available.
68+
* runBaseUnits: deprecated alias kept for downstream backward compatibility.
6869
* maxUnitsPerOperation: safety cap per single operation run.
6970
*/
7071
meter: {
72+
runBase: 1,
7173
runBaseUnits: 1,
74+
fallbackPlanId: null,
7275
/**
7376
* Canonical version string emitted by billing.meter.service attribute() when writing
7477
* history.planVersion. Downstream projects MUST override this value and keep it
@@ -98,6 +101,22 @@ const config = {
98101
dollarsToUnitRatio: 1000,
99102
maxUnitsPerOperation: 10000,
100103
},
104+
outbox: {
105+
maxRetryAttempts: 5,
106+
retryIntervalSec: 300,
107+
},
108+
crons: {
109+
jitterMaxMs: 60_000,
110+
},
111+
planChange: {
112+
preserveUsageDefault: true,
113+
},
114+
alerts: {
115+
thresholdPercents: [80, 100],
116+
},
117+
events: {
118+
extrasExhausted: 'billing.extras_debit.exhausted',
119+
},
101120
/**
102121
* Extra meter packs — downstream projects override with actual packs.
103122
* Example: [{ packId: 'pack_500k', meterUnits: 500000, stripePriceId: 'price_xxx' }]

modules/billing/controllers/billing.controller.js

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,6 @@ import responses from '../../../lib/helpers/responses.js';
77
import BillingService from '../services/billing.service.js';
88
import BillingUsageService from '../services/billing.usage.service.js';
99
import BillingExtraService from '../services/billing.extra.service.js';
10-
import BillingExtraBalanceRepository from '../repositories/billing.extraBalance.repository.js';
11-
12-
// NOTE: BillingExtraBalance uses field name 'organization', BillingUsage uses 'organizationId' — both are Schema.ObjectId refs to Organization.
13-
// Only the field name differs (historical reasons) — keep queries consistent with each model's own convention.
1410

1511
/**
1612
* @desc Endpoint to create a Stripe Checkout session
@@ -81,7 +77,7 @@ const getUsage = async (req, res) => {
8177
if (config.billing?.meterMode) {
8278
// Meter mode — return compute fields
8379
const meter = await BillingUsageService.getMeter(req.organization._id.toString());
84-
const extrasRemaining = await BillingExtraBalanceRepository.getBalance(req.organization._id.toString());
80+
const extrasRemaining = await BillingExtraService.getOrgBalanceContext(req.organization._id.toString());
8581
const packsAvailable = config.billing?.packs ?? [];
8682

8783
return responses.success(res, 'billing usage')({
@@ -153,7 +149,7 @@ const extrasCheckout = async (req, res) => {
153149
// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js controller, not Qwik
154150
const extrasBalance = async (req, res) => {
155151
try {
156-
const balance = await BillingExtraBalanceRepository.getBalance(req.organization._id.toString());
152+
const balance = await BillingExtraService.getOrgBalanceContext(req.organization._id.toString());
157153
const packsAvailable = config.billing?.packs ?? [];
158154
responses.success(res, 'extras balance')({ balance, packsAvailable });
159155
} catch (err) {

modules/billing/controllers/billing.webhook.controller.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44
import config from '../../../config/index.js';
55
import logger from '../../../lib/services/logger.js';
6+
import responses from '../../../lib/helpers/responses.js';
67
import getStripe from '../lib/stripe.js';
78
import BillingWebhookService from '../services/billing.webhook.service.js';
89

@@ -15,7 +16,9 @@ import BillingWebhookService from '../services/billing.webhook.service.js';
1516
// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js controller, not Qwik
1617
const handleWebhook = async (req, res) => {
1718
const stripe = getStripe();
18-
if (!stripe) return res.status(400).json({ error: 'Stripe is not configured' });
19+
if (!stripe) {
20+
return responses.error(res, 400, 'Bad Request', 'Stripe is not configured')(new Error('Stripe is not configured'));
21+
}
1922

2023
const sig = req.headers['stripe-signature'];
2124
const { webhookSecret } = config.stripe;
@@ -24,7 +27,7 @@ const handleWebhook = async (req, res) => {
2427
try {
2528
event = stripe.webhooks.constructEvent(req.body, sig, webhookSecret);
2629
} catch (err) {
27-
return res.status(400).json({ error: 'Webhook signature verification failed' });
30+
return responses.error(res, 400, 'Bad Request', 'Webhook signature verification failed')(err);
2831
}
2932

3033
try {
@@ -65,7 +68,7 @@ const handleWebhook = async (req, res) => {
6568
return res.status(200).json({ received: true });
6669
} catch (err) {
6770
logger.error('Stripe webhook handler error:', err);
68-
return res.status(500).json({ error: 'Webhook handler failed' });
71+
return responses.error(res, 500, 'Internal Server Error', 'Webhook handler failed')(err);
6972
}
7073
};
7174

modules/billing/crons/README.md

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,19 +59,21 @@ Repeat the manifest for `billing.extrasExpiration.js` and `billing.dunningSweep.
5959

6060
Devkit-shipped crons run on identical UTC schedules across all consumer deployments. To avoid thundering-herd against a shared DB or external API:
6161

62-
### Recommended pattern — startup jitter
62+
### Built-in startup jitter
6363

64-
These scripts are invoked once per CronJob execution and exit immediately after. Add a random delay at the top of your entrypoint to spread load across deployments:
64+
`billing.extrasExpiration.js`, `billing.dunningSweep.js`, and `retry-pending-extras-debit.cron.js` call `applyJitter(config.billing.crons.jitterMaxMs ?? 60000)` before doing work. Override the window per project:
6565

6666
```js
67-
// Wrap in an async IIFE — cron entrypoints are CommonJS, so top-level await is not available.
68-
// Jitter is re-randomized on each CronJob invocation — this is intentional for K8s CronJobs.
69-
// For a stable per-pod offset, derive from process.env.HOSTNAME instead (see note below).
70-
(async () => {
71-
const jitterMs = Math.floor(Math.random() * 60_000); // 0–60s window
72-
await new Promise(r => setTimeout(r, jitterMs));
73-
await BillingResetService.resetAllDue();
74-
})();
67+
export default {
68+
billing: {
69+
crons: {
70+
jitterMaxMs: 30_000,
71+
},
72+
outbox: {
73+
retryIntervalSec: 120,
74+
},
75+
},
76+
};
7577
```
7678

7779
> **Stable per-pod jitter (optional):** If you want the same pod to always fire at the same offset within the window, derive jitter from the pod hostname instead of `Math.random()`. Use a distinct variable name to avoid shadowing if both snippets appear in the same file:

modules/billing/crons/billing.dunningSweep.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,16 @@
1717

1818
process.env.NODE_ENV = process.env.NODE_ENV || 'development';
1919

20-
const [{ default: config }, { default: mongooseService }] = await Promise.all([
20+
const [
21+
{ default: config },
22+
{ default: mongooseService },
23+
{ applyJitter },
24+
{ getCronJitterMaxMs },
25+
] = await Promise.all([
2126
import('../../../config/index.js'),
2227
import('../../../lib/services/mongoose.js'),
28+
import('../lib/billing.cron-utils.js'),
29+
import('../lib/billing.constants.js'),
2330
]);
2431

2532
if (!config?.billing?.meterMode) {
@@ -28,6 +35,7 @@ if (!config?.billing?.meterMode) {
2835
}
2936

3037
try {
38+
await applyJitter(getCronJitterMaxMs());
3139
await mongooseService.connect();
3240

3341
const [{ default: BillingSubscriptionRepository }, { default: OrganizationRepository }] = await Promise.all([

modules/billing/crons/billing.extrasExpiration.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,16 @@
1414

1515
process.env.NODE_ENV = process.env.NODE_ENV || 'development';
1616

17-
const [{ default: config }, { default: mongooseService }] = await Promise.all([
17+
const [
18+
{ default: config },
19+
{ default: mongooseService },
20+
{ applyJitter },
21+
{ getCronJitterMaxMs },
22+
] = await Promise.all([
1823
import('../../../config/index.js'),
1924
import('../../../lib/services/mongoose.js'),
25+
import('../lib/billing.cron-utils.js'),
26+
import('../lib/billing.constants.js'),
2027
]);
2128

2229
if (!config?.billing?.meterMode) {
@@ -25,6 +32,7 @@ if (!config?.billing?.meterMode) {
2532
}
2633

2734
try {
35+
await applyJitter(getCronJitterMaxMs());
2836
await mongooseService.connect();
2937

3038
const [{ default: BillingExtraService }, { default: BillingExtraBalanceRepository }] =

modules/billing/crons/retry-pending-extras-debit.cron.js

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,26 +10,31 @@
1010

1111
process.env.NODE_ENV = process.env.NODE_ENV || 'development';
1212

13-
const [{ default: config }, { default: mongooseService }] = await Promise.all([
13+
const [
14+
{ default: config },
15+
{ default: mongooseService },
16+
{ applyJitter },
17+
{ getCronJitterMaxMs, getOutboxRetryIntervalMs },
18+
] = await Promise.all([
1419
import('../../../config/index.js'),
1520
import('../../../lib/services/mongoose.js'),
21+
import('../lib/billing.cron-utils.js'),
22+
import('../lib/billing.constants.js'),
1623
]);
1724

1825
if (!config?.billing?.meterMode) {
1926
console.log('[billing.retryPendingExtrasDebit] meterMode disabled — skipping.');
2027
process.exit(0);
2128
}
2229

23-
const { randomInt } = await import('node:crypto');
24-
const jitterMs = randomInt(0, 60_000);
25-
await new Promise((resolve) => setTimeout(resolve, jitterMs));
30+
await applyJitter(getCronJitterMaxMs());
2631

2732
try {
2833
await mongooseService.loadModels();
2934
await mongooseService.connect();
3035

3136
const { default: BillingMeterOutboxService } = await import('../services/billing.meter.outbox.service.js');
32-
const result = await BillingMeterOutboxService.retryPendingExtrasDebits(5 * 60 * 1000, 100);
37+
const result = await BillingMeterOutboxService.retryPendingExtrasDebits(getOutboxRetryIntervalMs(), 100);
3338

3439
console.log(
3540
`[billing.retryPendingExtrasDebit] done — scanned: ${result.scanned}, committed: ${result.committed}, failedAttempts: ${result.failedAttempts}, exhausted: ${result.exhausted}`,

0 commit comments

Comments
 (0)