Skip to content

Commit 402a9a5

Browse files
feat(billing): hygiene polish — stack trace, req.id, $slice ledger, TTL archived, cache utils (#3614)
* feat(billing): hygiene polish — stack trace, req.id, $slice ledger, TTL archived, cache utils Étape 5 — Polish (devkit-only): A — lastError + req.id correlation - incrementAttempts stores err.stack (not just message); JSON fallback for plain-object throws - withIdempotency accepts optional ctx.requestId; logs entry/skip/retry/dead-letter - Webhook controller reads req.id and threads it into all withIdempotency calls B — Performance hygiene - listLedger: new listLedgerPage() repo method uses $slice aggregation — only the page is transferred over the wire; perf integration test asserts < 50ms on 1000 entries - BillingUsage: TTL index on archivedAt (1-year, partialFilterExpression $type date) so archived weekly periods are auto-purged without touching active docs C — Tier 3 nits - Removed module-scope METER_RUN_BASE const (stale at load time); getMeterRunBase() function kept and re-exported from billing.meter.service.js - Exported clearPlansCache() from billing.plans.service.js for test isolation - fetchPlansFromStripe: explicit warn log when Stripe price ID mapped to multiple plans * fix(billing): harden tests + $ifNull guard on listLedgerPage ledger field - Add $ifNull(['$ledger', []]) guard in $size and $sortArray expressions so listLedgerPage is robust against legacy/partial docs with missing ledger - Add toHaveBeenCalledTimes(1) assertions before mock.calls[0] access in webhook idempotency + hardening tests for clearer failure diagnostics - Add dedicated controller test asserting req.id is propagated as requestId into withIdempotency third arg
1 parent f81aa78 commit 402a9a5

17 files changed

Lines changed: 527 additions & 74 deletions

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ const handleWebhook = async (req, res) => {
2222

2323
const sig = req.headers['stripe-signature'];
2424
const { webhookSecret } = config.stripe;
25+
// req.id is injected by the global requestId middleware (lib/middlewares/requestId.js).
26+
// Propagate it through to enable end-to-end traceability across webhook log lines.
27+
const requestId = req.id;
2528

2629
let event;
2730
try {
@@ -30,70 +33,83 @@ const handleWebhook = async (req, res) => {
3033
return responses.error(res, 400, 'Bad Request', 'Webhook signature verification failed')(err);
3134
}
3235

36+
logger.info('[billing.webhook] received', { type: event.type, id: event.id, requestId });
37+
3338
try {
3439
switch (event.type) {
3540
case 'checkout.session.completed':
3641
await BillingWebhookService.withIdempotency(event, (e) =>
3742
BillingWebhookService.handleCheckoutSessionCompleted(e),
43+
{ requestId },
3844
);
3945
break;
4046
case 'customer.subscription.created':
4147
await BillingWebhookService.withIdempotency(event, (e) =>
4248
BillingWebhookService.handleSubscriptionCreated(e.data.object, e),
49+
{ requestId },
4350
);
4451
break;
4552
case 'customer.subscription.updated':
4653
await BillingWebhookService.withIdempotency(event, (e) =>
4754
BillingWebhookService.handleSubscriptionUpdated(e.data.object, e),
55+
{ requestId },
4856
);
4957
break;
5058
case 'customer.subscription.deleted':
5159
await BillingWebhookService.withIdempotency(event, (e) =>
5260
BillingWebhookService.handleSubscriptionDeleted(e.data.object, e),
61+
{ requestId },
5362
);
5463
break;
5564
case 'invoice.payment_failed':
5665
await BillingWebhookService.withIdempotency(event, (e) =>
5766
BillingWebhookService.handleInvoicePaymentFailed(e.data.object, e),
67+
{ requestId },
5868
);
5969
break;
6070
case 'invoice.payment_succeeded':
6171
await BillingWebhookService.withIdempotency(event, (e) =>
6272
BillingWebhookService.handleInvoicePaymentSucceeded(e.data.object, e),
73+
{ requestId },
6374
);
6475
break;
6576
case 'charge.refunded':
6677
await BillingWebhookService.withIdempotency(event, (e) =>
6778
BillingWebhookService.handleChargeRefunded(e.data.object),
79+
{ requestId },
6880
);
6981
break;
7082
case 'customer.deleted':
7183
await BillingWebhookService.withIdempotency(event, (e) =>
7284
BillingWebhookService.handleCustomerDeleted(e.data.object, e),
85+
{ requestId },
7386
);
7487
break;
7588
case 'charge.dispute.created':
7689
await BillingWebhookService.withIdempotency(event, (e) =>
7790
BillingWebhookService.handleChargeDisputeCreated(e.data.object, e),
91+
{ requestId },
7892
);
7993
break;
8094
case 'charge.dispute.funds_withdrawn':
8195
await BillingWebhookService.withIdempotency(event, (e) =>
8296
BillingWebhookService.handleChargeDisputeFundsWithdrawn(e.data.object, e),
97+
{ requestId },
8398
);
8499
break;
85100
case 'charge.dispute.funds_reinstated':
86101
await BillingWebhookService.withIdempotency(event, (e) =>
87102
BillingWebhookService.handleChargeDisputeFundsReinstated(e.data.object, e),
103+
{ requestId },
88104
);
89105
break;
90106
default:
91-
logger.info('[billing.webhook] unhandled event type', { type: event.type, id: event.id });
107+
logger.info('[billing.webhook] unhandled event type', { type: event.type, id: event.id, requestId });
92108
break;
93109
}
94110
return res.status(200).json({ received: true });
95111
} catch (err) {
96-
logger.error('Stripe webhook handler error:', err);
112+
logger.error('[billing.webhook] handler error', { error: err?.message ?? String(err), stack: err?.stack, requestId });
97113
return responses.error(res, 500, 'Internal Server Error', 'Webhook handler failed')(err);
98114
}
99115
};

modules/billing/lib/billing.constants.js

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,11 @@ export const DEFAULT_EXTRAS_EXHAUSTED_EVENT = 'billing.extras_debit.exhausted';
2222
export const DEFAULT_GRACE_PERIOD_DAYS = 7;
2323
export const DEFAULT_DUNNING_THRESHOLD_DAYS = 14;
2424

25-
/**
26-
* Floor charge per run. `runBaseUnits` is kept as a backward-compatible alias
27-
* for downstream projects that already adopted the earlier config name.
28-
*/
29-
export const METER_RUN_BASE =
30-
config?.billing?.meter?.runBase
31-
?? config?.billing?.meter?.runBaseUnits
32-
?? DEFAULT_METER_RUN_BASE;
33-
3425
/**
3526
* @function getMeterRunBase
3627
* @description Resolve the configured base units charged when no costs are present.
28+
* Evaluated lazily on each call so test overrides and runtime config
29+
* changes are always reflected (avoids the stale module-scope const pattern).
3730
* @returns {number} Meter run base units.
3831
*/
3932
export const getMeterRunBase = () =>

modules/billing/models/billing.usage.model.mongoose.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,28 @@ UsageMongoose.index(
137137
*/
138138
UsageMongoose.index({ organizationId: 1, weekKey: 1 }, { unique: true, sparse: true });
139139

140+
/**
141+
* TTL index: automatically purge archived usage documents after 1 year.
142+
*
143+
* Conditional on archivedAt being set (non-null) — active (non-archived) documents
144+
* are excluded from this TTL via partialFilterExpression, so only past periods are
145+
* eligible for purge. The 1-year retention keeps the audit trail long enough for
146+
* billing reconciliation while preventing unbounded collection growth.
147+
*
148+
* Notes:
149+
* - The TTL daemon runs once per minute; actual deletion may lag by up to ~1min.
150+
* - Verify this index with: db.billingusages.getIndexes() — look for expireAfterSeconds.
151+
* - archivedAt is set by archiveOtherWeeks() in the usage repository when the
152+
* weekly reset sweep moves old weeks to the archive state.
153+
*/
154+
UsageMongoose.index(
155+
{ archivedAt: 1 },
156+
{
157+
expireAfterSeconds: 365 * 24 * 60 * 60,
158+
partialFilterExpression: { archivedAt: { $type: 'date' } },
159+
},
160+
);
161+
140162
/**
141163
* Returns the hex string representation of the document ObjectId.
142164
* @returns {string} Hex string of the ObjectId.

modules/billing/repositories/billing.extraBalance.repository.js

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,60 @@ const getBalance = async (orgId) => {
257257
return doc ? doc.cachedBalance : 0;
258258
};
259259

260+
/**
261+
* @function listLedgerPage
262+
* @description Return a paginated slice of the ledger array for an organization using
263+
* MongoDB aggregation `$slice` — only the requested page is transferred over the
264+
* wire, avoiding full-document fetches for large ledgers (1000+ entries).
265+
*
266+
* Entries are sorted descending by `at` (newest first) at the aggregation layer.
267+
* The aggregation pipeline is:
268+
* 1. $match — find the org's document
269+
* 2. $project — sort + slice the ledger, plus cachedBalance and _id=0
270+
*
271+
* Returns null when no document exists for the org yet (balance = 0).
272+
*
273+
* @param {string} orgId - The organization ObjectId (string).
274+
* @param {number} skip - Number of entries to skip (0-based).
275+
* @param {number} limit - Maximum number of entries to return.
276+
* @returns {Promise<{ledgerPage: Object[], total: number, cachedBalance: number}|null>}
277+
* null when the org has no ExtraBalance document.
278+
*/
279+
// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js repository, not Qwik
280+
const listLedgerPage = async (orgId, skip, limit) => {
281+
if (!isValidOrgId(orgId)) return null;
282+
if (!Number.isFinite(skip) || skip < 0) throw new TypeError('skip must be a non-negative number');
283+
if (!Number.isFinite(limit) || limit <= 0) throw new TypeError('limit must be a positive number');
284+
285+
const results = await BillingExtraBalance().aggregate([
286+
{ $match: { organization: new mongoose.Types.ObjectId(orgId) } },
287+
{
288+
$project: {
289+
_id: 0,
290+
cachedBalance: 1,
291+
total: { $size: { $ifNull: ['$ledger', []] } },
292+
// Sort descending by `at` then slice the requested page.
293+
// $ifNull guards against missing/null ledger field on legacy docs.
294+
ledgerPage: {
295+
$slice: [
296+
{
297+
$sortArray: {
298+
input: { $ifNull: ['$ledger', []] },
299+
sortBy: { at: -1 },
300+
},
301+
},
302+
skip,
303+
limit,
304+
],
305+
},
306+
},
307+
},
308+
]).exec();
309+
310+
if (!results || results.length === 0) return null;
311+
return results[0];
312+
};
313+
260314
/**
261315
* @function findOrgsWithExpiringTopups
262316
* @description Return the distinct organizationIds that have at least one topup ledger entry
@@ -308,5 +362,6 @@ export default {
308362
addExpirationEntries,
309363
refundPartial,
310364
getBalance,
365+
listLedgerPage,
311366
findOrgsWithExpiringTopups,
312367
};

modules/billing/repositories/billing.processedStripeEvent.repository.js

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,20 +104,37 @@ const deleteByEventId = async (eventId) => {
104104
* @function incrementAttempts
105105
* @description Atomically increment the attempts counter and record the last error details
106106
* on the processed event document. Used by withIdempotency to track retry depth.
107+
* Stores the full stack trace when available (err.stack), falling back to the
108+
* message string — provides full context for ops investigation of dead-letters.
107109
* @param {string} eventId - Stripe event ID.
108-
* @param {string} errorMessage - Error message from the last failed handler execution.
110+
* @param {string|Error} errorOrMessage - Error object (preferred) or message string from the
111+
* last failed handler execution. When an Error is passed, err.stack is persisted; otherwise
112+
* the string value is stored as-is.
109113
* @returns {Promise<Object|null>} Updated document or null if not found.
110114
*/
111115
// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js repository, not Qwik
112-
const incrementAttempts = async (eventId, errorMessage) => {
116+
const incrementAttempts = async (eventId, errorOrMessage) => {
113117
if (typeof eventId !== 'string' || eventId.trim() === '') {
114118
throw new Error('invalid argument: eventId must be a non-empty string');
115119
}
120+
// Persist stack trace when available — provides full context for ops investigation.
121+
// Falls back gracefully for non-Error throws (string, plain object, etc.).
122+
// JSON.stringify for plain objects avoids the uninformative "[object Object]" fallback.
123+
let lastError;
124+
if (errorOrMessage instanceof Error) {
125+
lastError = errorOrMessage.stack || errorOrMessage.message || String(errorOrMessage);
126+
} else if (errorOrMessage === null || errorOrMessage === undefined) {
127+
lastError = '';
128+
} else if (typeof errorOrMessage === 'object') {
129+
try { lastError = JSON.stringify(errorOrMessage); } catch { lastError = String(errorOrMessage); }
130+
} else {
131+
lastError = String(errorOrMessage);
132+
}
116133
return ProcessedStripeEvent().findOneAndUpdate(
117134
{ eventId },
118135
{
119136
$inc: { attempts: 1 },
120-
$set: { lastError: String(errorMessage ?? ''), lastErrorAt: new Date() },
137+
$set: { lastError, lastErrorAt: new Date() },
121138
},
122139
{ returnDocument: 'after' },
123140
).exec();

modules/billing/services/billing.extra.service.js

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,10 @@ const refundPartial = async (orgId, stripeSessionId, amountRefundedCents, packId
216216
* @description Return a paginated slice of the ledger for an organization.
217217
* Entries are returned in reverse chronological order (newest first).
218218
*
219+
* Uses MongoDB aggregation `$slice` via the repository so only the
220+
* requested page is transferred over the wire — avoids loading the full
221+
* ledger array into memory on large accounts (1000+ entries).
222+
*
219223
* @param {string} orgId - The organization ObjectId (string).
220224
* @param {Object} [options={}] - Pagination options.
221225
* @param {number} [options.page=1] - 1-based page number.
@@ -224,17 +228,18 @@ const refundPartial = async (orgId, stripeSessionId, amountRefundedCents, packId
224228
*/
225229
// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js service, not Qwik
226230
const listLedger = async (orgId, { page = 1, limit = 20 } = {}) => {
227-
const doc = await BillingExtraBalanceRepository.getOrCreate(orgId);
228-
if (!doc) return { entries: [], total: 0, balance: 0 };
229-
const ledger = doc.ledger ?? [];
230-
const total = ledger.length;
231-
232-
// Sort descending by at date
233-
const sorted = [...ledger].sort((a, b) => new Date(b.at) - new Date(a.at));
234-
const start = (page - 1) * limit;
235-
const entries = sorted.slice(start, start + limit);
236-
237-
return { entries, total, balance: doc.cachedBalance ?? 0 };
231+
const safePage = Math.max(1, Math.floor(Number(page)) || 1);
232+
const safeLimit = Math.max(1, Math.floor(Number(limit)) || 20);
233+
const skip = (safePage - 1) * safeLimit;
234+
235+
const result = await BillingExtraBalanceRepository.listLedgerPage(orgId, skip, safeLimit);
236+
if (!result) return { entries: [], total: 0, balance: 0 };
237+
238+
return {
239+
entries: result.ledgerPage ?? [],
240+
total: result.total ?? 0,
241+
balance: result.cachedBalance ?? 0,
242+
};
238243
};
239244

240245
export default {

modules/billing/services/billing.meter.service.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,8 @@ import {
99
getMeterRunBase,
1010
getDollarsToUnitRatio,
1111
getMaxUnitsPerOperation,
12-
METER_RUN_BASE,
1312
} from '../lib/billing.constants.js';
1413

15-
export { METER_RUN_BASE };
16-
1714
/**
1815
* @function unitsFromCosts
1916
* @description Convert a feature-keyed cost map (in USD) to meter units using
@@ -237,5 +234,5 @@ export default {
237234
unitsFromCosts,
238235
attribute,
239236
capBreakdown,
240-
METER_RUN_BASE,
237+
getMeterRunBase,
241238
};

modules/billing/services/billing.plans.service.js

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,14 @@ const fetchPlansFromStripe = async (stripe) => {
6464
pricesByProduct[price.product].push(price);
6565
}
6666

67+
// Build reverse-lookup: priceId → [planId, ...] to detect price ID reuse across plans.
68+
// A Stripe price ID mapped to multiple plans is a configuration error that causes silent
69+
// billing misrouting — warn explicitly with both plan IDs + the duplicate price ID.
70+
const priceIdToPlanIds = {};
71+
6772
const plans = products.map((product) => {
6873
const productPrices = pricesByProduct[product.id] || [];
74+
const planId = product.metadata?.planId || product.id;
6975

7076
let monthlyPrice = 0;
7177
let annualPrice = 0;
@@ -82,10 +88,15 @@ const fetchPlansFromStripe = async (stripe) => {
8288
annualPrice = amount / 100;
8389
stripePriceAnnual = price.id;
8490
}
91+
// Track price-to-plan mapping for duplicate detection
92+
if (price.id) {
93+
if (!priceIdToPlanIds[price.id]) priceIdToPlanIds[price.id] = [];
94+
priceIdToPlanIds[price.id].push(planId);
95+
}
8596
}
8697

8798
return {
88-
planId: product.metadata?.planId || product.id,
99+
planId,
89100
name: product.name,
90101
monthlyPrice,
91102
annualPrice,
@@ -94,6 +105,17 @@ const fetchPlansFromStripe = async (stripe) => {
94105
};
95106
});
96107

108+
// Emit explicit warn for any Stripe price ID mapped to more than one plan.
109+
// This is a Stripe account configuration error that causes silent billing misrouting.
110+
for (const [priceId, planIds] of Object.entries(priceIdToPlanIds)) {
111+
if (planIds.length > 1) {
112+
logger.warn('[billing.plans] duplicate Stripe price ID mapped to multiple plans — config error', {
113+
priceId,
114+
planIds,
115+
});
116+
}
117+
}
118+
97119
return plans.sort((a, b) => a.monthlyPrice - b.monthlyPrice);
98120
};
99121

@@ -164,6 +186,25 @@ const getPlans = async () => {
164186
return inFlightFetch;
165187
};
166188

189+
/**
190+
* @desc Reset the in-memory plan cache.
191+
*
192+
* Exposed for test isolation: each test that exercises different Stripe configurations
193+
* should call `clearPlansCache()` in `beforeEach`/`afterEach` to prevent stale module-scope
194+
* state from leaking between test files (ESM module cache is per-worker but shared across
195+
* tests in the same worker file when `jest.resetModules()` is not in use).
196+
*
197+
* NOT intended for production use — the cache is self-refreshing via stale-while-revalidate.
198+
*/
199+
const clearPlansCache = () => {
200+
cachedPlans = null;
201+
cacheTimestamp = 0;
202+
stalePlans = null;
203+
staleTimestamp = 0;
204+
inFlightFetch = null;
205+
};
206+
167207
export default {
168208
getPlans,
209+
clearPlansCache,
169210
};

0 commit comments

Comments
 (0)