From 05b08839ee9e71ba6ab98eddb739de03016f2885 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 21 May 2026 21:57:54 +0200 Subject: [PATCH 1/6] feat(billing): retryWithBackoff helper + BillingFailedBackfill dead-letter model --- modules/billing/lib/billing.retry.js | 26 +++++ .../billing.failedBackfill.model.mongoose.js | 95 +++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 modules/billing/lib/billing.retry.js create mode 100644 modules/billing/models/billing.failedBackfill.model.mongoose.js diff --git a/modules/billing/lib/billing.retry.js b/modules/billing/lib/billing.retry.js new file mode 100644 index 000000000..fda5955bf --- /dev/null +++ b/modules/billing/lib/billing.retry.js @@ -0,0 +1,26 @@ +/** + * @function retryWithBackoff + * @description Retry an async operation with exponential backoff. + * Returns the result of the first successful call, or throws the last error + * after all attempts are exhausted. + * + * @param {() => Promise} fn - Async function to attempt. + * @param {object} [opts] + * @param {number} [opts.attempts=3] - Maximum number of attempts (including the first call). + * @param {number} [opts.baseMs=200] - Base delay in ms; doubles on each retry (200 → 400 → 800). + * @returns {Promise} + */ +export async function retryWithBackoff(fn, { attempts = 3, baseMs = 200 } = {}) { + let lastErr; + for (let i = 0; i < attempts; i++) { + try { + return await fn(); + } catch (err) { + lastErr = err; + if (i < attempts - 1) { + await new Promise((resolve) => setTimeout(resolve, baseMs * 2 ** i)); + } + } + } + throw lastErr; +} diff --git a/modules/billing/models/billing.failedBackfill.model.mongoose.js b/modules/billing/models/billing.failedBackfill.model.mongoose.js new file mode 100644 index 000000000..14805732e --- /dev/null +++ b/modules/billing/models/billing.failedBackfill.model.mongoose.js @@ -0,0 +1,95 @@ +/** + * Module dependencies + */ +import mongoose from 'mongoose'; + +const Schema = mongoose.Schema; + +/** + * BillingFailedBackfill Data Model Mongoose + * + * Dead-letter store for PaymentIntent metadata backfill failures. + * Records are written when the refund-correlation backfill (stripe.paymentIntents.update + * in handleCheckoutPaymentCompleted) fails after all retry attempts. + * + * Kept permanently so operators can manually reconcile unresolved entries. + * Never auto-expired — resolvedAt is set by the operator after manual fix. + */ +const BillingFailedBackfillMongoose = new Schema( + { + paymentIntentId: { + type: String, + required: true, + index: true, + }, + stripeSessionId: { + type: String, + required: true, + }, + /** + * Serialised error message from the last failed attempt. + */ + error: { + type: String, + default: null, + }, + /** + * Timestamp of the first failure (when the record was created). + */ + failedAt: { + type: Date, + required: true, + default: () => new Date(), + }, + /** + * Timestamp set by the operator after the PI metadata has been manually patched + * and the refund correlation risk resolved. + */ + resolvedAt: { + type: Date, + default: null, + }, + /** + * Operator tag explaining how the record was resolved. + * E.g. 'admin', 'cron'. + */ + resolvedBy: { + type: String, + default: null, + }, + }, + { + collection: 'billing_failed_backfills', + timestamps: false, + }, +); + +/** + * Partial index to support the runbook query "find unresolved entries". + * Uses a sparse index on resolvedAt so the null check stays efficient as the + * collection grows (only unresolved documents are indexed). + */ +BillingFailedBackfillMongoose.index( + { resolvedAt: 1 }, + { sparse: true }, +); + +/** + * Returns the hex string representation of the document ObjectId. + * @returns {string} Hex string of the ObjectId. + */ +function addID() { + return this._id.toHexString(); +} + +/** + * Model configuration + */ +BillingFailedBackfillMongoose.virtual('id').get(addID); +BillingFailedBackfillMongoose.set('toJSON', { + virtuals: true, +}); + +export const BillingFailedBackfill = + mongoose.models.BillingFailedBackfill ?? + mongoose.model('BillingFailedBackfill', BillingFailedBackfillMongoose); From 3bc671340e917eaf59f9f5a2854273b49124ed28 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 21 May 2026 21:58:04 +0200 Subject: [PATCH 2/6] fix(billing): retry refund correlation backfill 3x + dead-letter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PaymentIntent metadata backfill on checkout.session.completed now retries 3 times with exponential backoff (200ms, 400ms, 800ms) before escalating. On exhaustion, a persistent BillingFailedBackfill dead-letter record is written so operators can manually patch missing PI metadata and preserve refund correlation. Strategy chosen: Option B — new BillingFailedBackfill model (no prior mechanism for this specific failure path). The existing ProcessedStripeEvent dead-letter covers Stripe event delivery failures; this addresses a different failure class (Stripe API write failure on a side-effect within a successfully-delivered event handler). BillingFailedBackfill model auto-registers via assets.js glob: modules/*/models/*.mongoose.js — no billing.init.js change required. Closes audit P1 (2026-05-21) — silent warn-log path on refund correlation. --- .../services/billing.webhook.service.js | 52 +++-- .../billing.refund-correlation.unit.tests.js | 200 ++++++++++++++++++ 2 files changed, 238 insertions(+), 14 deletions(-) create mode 100644 modules/billing/tests/billing.refund-correlation.unit.tests.js diff --git a/modules/billing/services/billing.webhook.service.js b/modules/billing/services/billing.webhook.service.js index 8d1ba64b3..cab4c132b 100644 --- a/modules/billing/services/billing.webhook.service.js +++ b/modules/billing/services/billing.webhook.service.js @@ -13,6 +13,15 @@ import BillingExtraService from './billing.extra.service.js'; import BillingResetService from './billing.reset.service.js'; import billingEvents from '../lib/events.js'; import { SENTINEL_PENDING } from '../lib/billing.constants.js'; +import { retryWithBackoff } from '../lib/billing.retry.js'; + +/** + * Lazily resolves the BillingFailedBackfill Mongoose model. + * Deferred to keep unit tests importable before model registration. + * @returns {import('mongoose').Model} The registered BillingFailedBackfill model. + */ +// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js service, not Qwik +const BillingFailedBackfill = () => mongoose.model('BillingFailedBackfill'); /** * Treats a stripeSessionId as "unresolved" when absent, empty, or still the @@ -311,21 +320,36 @@ const handleCheckoutPaymentCompleted = async (session) => { const stripe = getStripe(); if (stripe) { try { - await stripe.paymentIntents.update(paymentIntentId, { - metadata: { - organizationId, - packId, - kind: 'extras', - stripeSessionId, // real cs_* ID (replaces SENTINEL_PENDING) - }, - }); + await retryWithBackoff( + () => + stripe.paymentIntents.update(paymentIntentId, { + metadata: { + organizationId, + packId, + kind: 'extras', + stripeSessionId, // real cs_* ID (replaces SENTINEL_PENDING) + }, + }), + { attempts: 3, baseMs: 200 }, + ); } catch (err) { - // Log but don't fail — refund correlation may use the backfill resolver path - logger.warn('[billing.webhook] PaymentIntent metadata update failed', { - paymentIntentId, - error: err?.message ?? String(err), - stack: err?.stack, - }); + logger.error( + '[billing.webhook] PI metadata backfill failed after retries — refund correlation at risk', + { paymentIntentId, sessionId: stripeSessionId, error: err?.message ?? String(err) }, + ); + try { + await BillingFailedBackfill().create({ + paymentIntentId, + stripeSessionId, + error: err?.message ?? String(err), + failedAt: new Date(), + }); + } catch (dlqErr) { + logger.error( + '[billing.webhook] dead-letter write failed — manual reconciliation required', + { paymentIntentId, sessionId: stripeSessionId, error: dlqErr?.message ?? String(dlqErr) }, + ); + } } } } diff --git a/modules/billing/tests/billing.refund-correlation.unit.tests.js b/modules/billing/tests/billing.refund-correlation.unit.tests.js new file mode 100644 index 000000000..39de13c3e --- /dev/null +++ b/modules/billing/tests/billing.refund-correlation.unit.tests.js @@ -0,0 +1,200 @@ +/** + * Module dependencies. + */ +import { jest, describe, test, beforeEach, afterEach, expect } from '@jest/globals'; + +/** + * Unit tests — checkout.session.completed PaymentIntent metadata backfill retry + * + * Covers: + * - retryWithBackoff: 3 attempts on Stripe API failure before escalating + * - Dead-letter: BillingFailedBackfill.create called after 3 consecutive failures + * - Dead-letter write failure: logger.error emitted, handler does not throw + */ + +describe('checkout.session.completed — metadata backfill retry:', () => { + let BillingWebhookService; + let mockStripeInstance; + let mockExtraService; + let mockLogger; + let mockBillingFailedBackfill; + + const orgId = '507f1f77bcf86cd799439011'; + const stripeSessionId = 'cs_test_session_abc'; + const paymentIntentId = 'pi_test_retry_001'; + + const makeSession = () => ({ + id: stripeSessionId, + payment_status: 'paid', + payment_intent: paymentIntentId, + metadata: { organizationId: orgId, packId: 'pack_500k', kind: 'extras' }, + }); + + beforeEach(async () => { + jest.resetModules(); + // Use fake timers so backoff delays don't slow the suite. + jest.useFakeTimers(); + + mockLogger = { info: jest.fn(), error: jest.fn(), warn: jest.fn(), debug: jest.fn() }; + + mockExtraService = { + creditPack: jest.fn().mockResolvedValue({ doc: {}, applied: true }), + refundPartial: jest.fn(), + }; + + mockStripeInstance = { + paymentIntents: { + update: jest.fn().mockResolvedValue({}), + retrieve: jest.fn().mockResolvedValue({ id: paymentIntentId, metadata: { stripeSessionId } }), + }, + }; + + // BillingFailedBackfill is resolved lazily via mongoose.model('BillingFailedBackfill'). + // Provide a mock implementation on the model registry. + mockBillingFailedBackfill = { + create: jest.fn().mockResolvedValue({}), + }; + + jest.unstable_mockModule('../lib/stripe.js', () => ({ + default: jest.fn(() => mockStripeInstance), + })); + + jest.unstable_mockModule('../services/billing.extra.service.js', () => ({ + default: mockExtraService, + })); + + jest.unstable_mockModule('../repositories/billing.subscription.repository.js', () => ({ + default: { + findByOrganization: jest.fn(), + findByStripeCustomerId: jest.fn(), + findByStripeSubscriptionId: jest.fn(), + create: jest.fn(), + update: jest.fn(), + updateIfEventNewer: jest.fn().mockResolvedValue(null), + }, + })); + + jest.unstable_mockModule('../repositories/billing.processedStripeEvent.repository.js', () => ({ + default: { + tryRecord: jest.fn().mockResolvedValue({ recorded: true }), + incrementAttempts: jest.fn(), + markDeadLetter: jest.fn(), + deleteByEventId: jest.fn(), + }, + })); + + jest.unstable_mockModule('../../organizations/repositories/organizations.repository.js', () => ({ + default: { setPlan: jest.fn().mockResolvedValue({}) }, + })); + + jest.unstable_mockModule('../services/billing.reset.service.js', () => ({ + default: { resetWeek: jest.fn(), forceRotateForPlanChange: jest.fn() }, + })); + + jest.unstable_mockModule('../lib/events.js', () => ({ + default: { emit: jest.fn() }, + })); + + jest.unstable_mockModule('../../../lib/services/logger.js', () => ({ + default: mockLogger, + })); + + jest.unstable_mockModule('../../../config/index.js', () => ({ + default: { billing: { plans: ['free', 'starter', 'pro', 'enterprise'] } }, + })); + + // mongoose.model() is called lazily: 'BillingFailedBackfill' must resolve to the mock. + jest.unstable_mockModule('mongoose', () => ({ + default: { + Types: { ObjectId: { isValid: (id) => /^[a-f\d]{24}$/i.test(id) } }, + model: (name) => { + if (name === 'BillingFailedBackfill') return mockBillingFailedBackfill; + return {}; + }, + models: {}, + }, + })); + + const mod = await import('../services/billing.webhook.service.js'); + BillingWebhookService = mod.default; + }); + + afterEach(() => { + jest.useRealTimers(); + jest.restoreAllMocks(); + }); + + test('retries up to 3 times on Stripe API failure before escalating', async () => { + // Fail twice, succeed on the third attempt. + mockStripeInstance.paymentIntents.update + .mockRejectedValueOnce(new Error('Stripe 500')) + .mockRejectedValueOnce(new Error('Stripe 500')) + .mockResolvedValueOnce({}); + + const promise = BillingWebhookService.handleCheckoutPaymentCompleted(makeSession()); + + // Advance fake timers past the two backoff delays (200ms + 400ms). + await jest.runAllTimersAsync(); + + await promise; + + // 3 total calls (2 retries + 1 success) + expect(mockStripeInstance.paymentIntents.update).toHaveBeenCalledTimes(3); + // No dead-letter record created because the third attempt succeeded. + expect(mockBillingFailedBackfill.create).not.toHaveBeenCalled(); + // No error log — outcome was successful. + expect(mockLogger.error).not.toHaveBeenCalled(); + }); + + test('escalates to dead-letter after 3 failed retries', async () => { + const stripeErr = new Error('Stripe persistently down'); + mockStripeInstance.paymentIntents.update.mockRejectedValue(stripeErr); + + const promise = BillingWebhookService.handleCheckoutPaymentCompleted(makeSession()); + + // Advance past all backoff delays. + await jest.runAllTimersAsync(); + + await promise; + + // All 3 attempts exhausted. + expect(mockStripeInstance.paymentIntents.update).toHaveBeenCalledTimes(3); + + // logger.error should be called for the final failure. + expect(mockLogger.error).toHaveBeenCalledWith( + expect.stringContaining('backfill failed after retries'), + expect.objectContaining({ paymentIntentId }), + ); + + // Dead-letter record created with the right shape. + expect(mockBillingFailedBackfill.create).toHaveBeenCalledWith( + expect.objectContaining({ + paymentIntentId, + stripeSessionId, + error: stripeErr.message, + }), + ); + }); + + test('handler does not throw when dead-letter write itself fails', async () => { + mockStripeInstance.paymentIntents.update.mockRejectedValue(new Error('Stripe down')); + mockBillingFailedBackfill.create.mockRejectedValue(new Error('Mongo down')); + + const promise = BillingWebhookService.handleCheckoutPaymentCompleted(makeSession()); + await jest.runAllTimersAsync(); + + // Must not throw even if both the backfill and dead-letter write fail. + await expect(promise).resolves.toBeUndefined(); + + // Both error paths should be logged. + expect(mockLogger.error).toHaveBeenCalledTimes(2); + expect(mockLogger.error).toHaveBeenCalledWith( + expect.stringContaining('backfill failed after retries'), + expect.objectContaining({ paymentIntentId }), + ); + expect(mockLogger.error).toHaveBeenCalledWith( + expect.stringContaining('dead-letter write failed'), + expect.objectContaining({ paymentIntentId }), + ); + }); +}); From 390ddef0f55af9df7f24244a7028aad97fd97ff7 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 21 May 2026 21:58:08 +0200 Subject: [PATCH 3/6] docs(billing): runbook for refund correlation backfill failure --- modules/billing/RUNBOOKS.md | 44 +++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/modules/billing/RUNBOOKS.md b/modules/billing/RUNBOOKS.md index 157ef0568..90e61f395 100644 --- a/modules/billing/RUNBOOKS.md +++ b/modules/billing/RUNBOOKS.md @@ -175,3 +175,47 @@ Operational runbooks for the billing module. Each runbook references real endpoi 4. Monitor `billing.plans.stale` event frequency — if the stale cache is 24h+, alert the on-call to decide whether to take the plans endpoint down entirely or serve a static fallback. 5. Once Stripe recovers: `POST /api/admin/billing/sync/:orgId` on any org that attempted a subscription change during the outage. 6. Check dead-letter queue for events that exhausted retries during the outage window: `GET /api/admin/billing/dead-letters`. + +--- + +## 6 — Refund Correlation Backfill Failure + +**Symptom:** ERROR log `PI metadata backfill failed after retries — refund correlation at risk`. +A later refund webhook may additionally log `refund unresolved — no stripeSessionId on charge metadata`. + +**Cause:** `stripe.paymentIntents.update` failed for all 3 retry attempts during +`checkout.session.completed` handling. Likely cause: transient Stripe API outage. + +**Triage:** + +1. Query unresolved entries: + + ```bash + db.billing_failed_backfills.find({ resolvedAt: null }) + ``` + + Each document contains `paymentIntentId`, `stripeSessionId`, `error`, and `failedAt`. + +2. For each entry, manually patch the PI via Stripe CLI or Dashboard: + + ```bash + stripe payment_intents update pi_xxx \ + --metadata stripeSessionId=cs_xxx + ``` + + Verify in Stripe Dashboard → Payments → PaymentIntent → Metadata. + +3. Mark the record resolved to close the loop: + + ```bash + db.billing_failed_backfills.updateOne( + { _id: ObjectId('...') }, + { $set: { resolvedAt: new Date(), resolvedBy: 'admin' } } + ) + ``` + +4. Confirm refund correlation: if a refund was already processed while the PI metadata was missing, + check for `billing.refund.unresolved` alerts and follow **Runbook #2** (Dead-Letter Investigation) + to replay the refund event after the PI metadata is patched. + +**Escalate if:** frequency exceeds 1 per week → promote to cron-based auto-retry. From c39b9c3fa0e4dac07909b10b3caf1b0ec1b7bd88 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 21 May 2026 22:10:03 +0200 Subject: [PATCH 4/6] fix(billing): address code-quality findings on refund correlation retry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - IMPORTANT: JSDoc delay sequence corrected (200 → 400, not 200 → 400 → 800 for default attempts=3, baseMs=200 — only 2 delays fire) - IMPORTANT: BillingFailedBackfill access moved to billing.failedBackfill.repository.js (Option B) — lazy mongoose.model() removed from service layer; test mocks updated to stub the repository boundary instead of mongoose.model() - IMPORTANT: sparse index → partial index on resolvedAt (sparse no-op because field has default: null, all docs include it — partial targets only unresolved docs) - MINOR: RUNBOOK section 6 symptom includes [billing.webhook] prefix --- modules/billing/RUNBOOKS.md | 2 +- modules/billing/lib/billing.retry.js | 14 +++++---- .../billing.failedBackfill.model.mongoose.js | 11 ++++--- .../billing.failedBackfill.repository.js | 30 +++++++++++++++++++ .../services/billing.webhook.service.js | 11 ++----- .../billing.refund-correlation.unit.tests.js | 23 +++++--------- 6 files changed, 54 insertions(+), 37 deletions(-) create mode 100644 modules/billing/repositories/billing.failedBackfill.repository.js diff --git a/modules/billing/RUNBOOKS.md b/modules/billing/RUNBOOKS.md index 90e61f395..f4acc2c0f 100644 --- a/modules/billing/RUNBOOKS.md +++ b/modules/billing/RUNBOOKS.md @@ -180,7 +180,7 @@ Operational runbooks for the billing module. Each runbook references real endpoi ## 6 — Refund Correlation Backfill Failure -**Symptom:** ERROR log `PI metadata backfill failed after retries — refund correlation at risk`. +**Symptom:** ERROR log `[billing.webhook] PI metadata backfill failed after retries — refund correlation at risk`. A later refund webhook may additionally log `refund unresolved — no stripeSessionId on charge metadata`. **Cause:** `stripe.paymentIntents.update` failed for all 3 retry attempts during diff --git a/modules/billing/lib/billing.retry.js b/modules/billing/lib/billing.retry.js index fda5955bf..152b7a11c 100644 --- a/modules/billing/lib/billing.retry.js +++ b/modules/billing/lib/billing.retry.js @@ -1,13 +1,17 @@ /** - * @function retryWithBackoff - * @description Retry an async operation with exponential backoff. - * Returns the result of the first successful call, or throws the last error - * after all attempts are exhausted. + * Retry an async operation with exponential backoff. + * + * For default opts (attempts=3, baseMs=200), delays are 200ms then 400ms + * (no delay after the final attempt). General formula: baseMs * 2^i for + * each non-final attempt i. + * + * Returns the result of the first successful call, or throws the last + * error after all attempts are exhausted. * * @param {() => Promise} fn - Async function to attempt. * @param {object} [opts] * @param {number} [opts.attempts=3] - Maximum number of attempts (including the first call). - * @param {number} [opts.baseMs=200] - Base delay in ms; doubles on each retry (200 → 400 → 800). + * @param {number} [opts.baseMs=200] - Base delay in ms for the first retry. * @returns {Promise} */ export async function retryWithBackoff(fn, { attempts = 3, baseMs = 200 } = {}) { diff --git a/modules/billing/models/billing.failedBackfill.model.mongoose.js b/modules/billing/models/billing.failedBackfill.model.mongoose.js index 14805732e..2d94a887d 100644 --- a/modules/billing/models/billing.failedBackfill.model.mongoose.js +++ b/modules/billing/models/billing.failedBackfill.model.mongoose.js @@ -64,14 +64,13 @@ const BillingFailedBackfillMongoose = new Schema( }, ); -/** - * Partial index to support the runbook query "find unresolved entries". - * Uses a sparse index on resolvedAt so the null check stays efficient as the - * collection grows (only unresolved documents are indexed). - */ +// Partial index — only unresolved documents are indexed, so this stays small +// even after the collection accumulates many resolved entries. +// (Sparse would be a no-op here: resolvedAt has default: null, so every document +// has the field present — sparse skips only docs where the field is absent.) BillingFailedBackfillMongoose.index( { resolvedAt: 1 }, - { sparse: true }, + { partialFilterExpression: { resolvedAt: null } }, ); /** diff --git a/modules/billing/repositories/billing.failedBackfill.repository.js b/modules/billing/repositories/billing.failedBackfill.repository.js new file mode 100644 index 000000000..9144324e6 --- /dev/null +++ b/modules/billing/repositories/billing.failedBackfill.repository.js @@ -0,0 +1,30 @@ +/** + * Module dependencies + */ +import mongoose from 'mongoose'; + +/** + * @function BillingFailedBackfill + * @description Lazily resolves the BillingFailedBackfill Mongoose model. + * Deferred to keep unit tests importable before model registration. + * @returns {import('mongoose').Model} The registered BillingFailedBackfill model. + */ +// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js repository, not Qwik +const BillingFailedBackfill = () => mongoose.model('BillingFailedBackfill'); + +/** + * @function record + * @description Write a dead-letter entry for a PaymentIntent metadata backfill failure. + * Called by billing.webhook.service after all retry attempts are exhausted. + * @param {object} opts + * @param {string} opts.paymentIntentId - Stripe PaymentIntent id (pi_*). + * @param {string} opts.stripeSessionId - Stripe checkout session id (cs_*). + * @param {string|null} [opts.error] - Serialised error message from the last failed attempt. + * @param {Date} [opts.failedAt] - Timestamp of the failure (defaults to now). + * @returns {Promise} + */ +// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js repository, not Qwik +const record = ({ paymentIntentId, stripeSessionId, error = null, failedAt = new Date() }) => + BillingFailedBackfill().create({ paymentIntentId, stripeSessionId, error, failedAt }); + +export default { record }; diff --git a/modules/billing/services/billing.webhook.service.js b/modules/billing/services/billing.webhook.service.js index cab4c132b..a17b73f3f 100644 --- a/modules/billing/services/billing.webhook.service.js +++ b/modules/billing/services/billing.webhook.service.js @@ -9,20 +9,13 @@ import logger from '../../../lib/services/logger.js'; import SubscriptionRepository from '../repositories/billing.subscription.repository.js'; import ProcessedStripeEventRepository from '../repositories/billing.processedStripeEvent.repository.js'; import OrganizationRepository from '../../organizations/repositories/organizations.repository.js'; +import BillingFailedBackfillRepository from '../repositories/billing.failedBackfill.repository.js'; import BillingExtraService from './billing.extra.service.js'; import BillingResetService from './billing.reset.service.js'; import billingEvents from '../lib/events.js'; import { SENTINEL_PENDING } from '../lib/billing.constants.js'; import { retryWithBackoff } from '../lib/billing.retry.js'; -/** - * Lazily resolves the BillingFailedBackfill Mongoose model. - * Deferred to keep unit tests importable before model registration. - * @returns {import('mongoose').Model} The registered BillingFailedBackfill model. - */ -// biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js service, not Qwik -const BillingFailedBackfill = () => mongoose.model('BillingFailedBackfill'); - /** * Treats a stripeSessionId as "unresolved" when absent, empty, or still the * creation-time sentinel placeholder written before Stripe assigned a real cs_* id. @@ -338,7 +331,7 @@ const handleCheckoutPaymentCompleted = async (session) => { { paymentIntentId, sessionId: stripeSessionId, error: err?.message ?? String(err) }, ); try { - await BillingFailedBackfill().create({ + await BillingFailedBackfillRepository.record({ paymentIntentId, stripeSessionId, error: err?.message ?? String(err), diff --git a/modules/billing/tests/billing.refund-correlation.unit.tests.js b/modules/billing/tests/billing.refund-correlation.unit.tests.js index 39de13c3e..b12940626 100644 --- a/modules/billing/tests/billing.refund-correlation.unit.tests.js +++ b/modules/billing/tests/billing.refund-correlation.unit.tests.js @@ -49,10 +49,9 @@ describe('checkout.session.completed — metadata backfill retry:', () => { }, }; - // BillingFailedBackfill is resolved lazily via mongoose.model('BillingFailedBackfill'). - // Provide a mock implementation on the model registry. + // BillingFailedBackfillRepository.record() is the repository boundary used by the service. mockBillingFailedBackfill = { - create: jest.fn().mockResolvedValue({}), + record: jest.fn().mockResolvedValue({}), }; jest.unstable_mockModule('../lib/stripe.js', () => ({ @@ -103,16 +102,8 @@ describe('checkout.session.completed — metadata backfill retry:', () => { default: { billing: { plans: ['free', 'starter', 'pro', 'enterprise'] } }, })); - // mongoose.model() is called lazily: 'BillingFailedBackfill' must resolve to the mock. - jest.unstable_mockModule('mongoose', () => ({ - default: { - Types: { ObjectId: { isValid: (id) => /^[a-f\d]{24}$/i.test(id) } }, - model: (name) => { - if (name === 'BillingFailedBackfill') return mockBillingFailedBackfill; - return {}; - }, - models: {}, - }, + jest.unstable_mockModule('../repositories/billing.failedBackfill.repository.js', () => ({ + default: mockBillingFailedBackfill, })); const mod = await import('../services/billing.webhook.service.js'); @@ -141,7 +132,7 @@ describe('checkout.session.completed — metadata backfill retry:', () => { // 3 total calls (2 retries + 1 success) expect(mockStripeInstance.paymentIntents.update).toHaveBeenCalledTimes(3); // No dead-letter record created because the third attempt succeeded. - expect(mockBillingFailedBackfill.create).not.toHaveBeenCalled(); + expect(mockBillingFailedBackfill.record).not.toHaveBeenCalled(); // No error log — outcome was successful. expect(mockLogger.error).not.toHaveBeenCalled(); }); @@ -167,7 +158,7 @@ describe('checkout.session.completed — metadata backfill retry:', () => { ); // Dead-letter record created with the right shape. - expect(mockBillingFailedBackfill.create).toHaveBeenCalledWith( + expect(mockBillingFailedBackfill.record).toHaveBeenCalledWith( expect.objectContaining({ paymentIntentId, stripeSessionId, @@ -178,7 +169,7 @@ describe('checkout.session.completed — metadata backfill retry:', () => { test('handler does not throw when dead-letter write itself fails', async () => { mockStripeInstance.paymentIntents.update.mockRejectedValue(new Error('Stripe down')); - mockBillingFailedBackfill.create.mockRejectedValue(new Error('Mongo down')); + mockBillingFailedBackfill.record.mockRejectedValue(new Error('Mongo down')); const promise = BillingWebhookService.handleCheckoutPaymentCompleted(makeSession()); await jest.runAllTimersAsync(); From dbb9c25d01318f110ee10f0331fbfa29c36fcefe Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Fri, 22 May 2026 08:53:34 +0200 Subject: [PATCH 5/6] fix(billing): validate retryWithBackoff options + document makeSession MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - guard attempts (positive integer) + baseMs (non-negative finite) with TypeError — prevents entering loop with attempts<=0 (would throw undefined) - JSDoc header on makeSession test helper Addresses CodeRabbit re-review on PR #3690. --- modules/billing/lib/billing.retry.js | 6 ++++++ .../billing.refund-correlation.unit.tests.js | 15 +++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/modules/billing/lib/billing.retry.js b/modules/billing/lib/billing.retry.js index 152b7a11c..a238157bd 100644 --- a/modules/billing/lib/billing.retry.js +++ b/modules/billing/lib/billing.retry.js @@ -15,6 +15,12 @@ * @returns {Promise} */ export async function retryWithBackoff(fn, { attempts = 3, baseMs = 200 } = {}) { + if (!Number.isInteger(attempts) || attempts < 1) { + throw new TypeError(`retryWithBackoff: attempts must be a positive integer, received ${attempts}`); + } + if (!Number.isFinite(baseMs) || baseMs < 0) { + throw new TypeError(`retryWithBackoff: baseMs must be a non-negative finite number, received ${baseMs}`); + } let lastErr; for (let i = 0; i < attempts; i++) { try { diff --git a/modules/billing/tests/billing.refund-correlation.unit.tests.js b/modules/billing/tests/billing.refund-correlation.unit.tests.js index b12940626..227bb4c59 100644 --- a/modules/billing/tests/billing.refund-correlation.unit.tests.js +++ b/modules/billing/tests/billing.refund-correlation.unit.tests.js @@ -2,6 +2,7 @@ * Module dependencies. */ import { jest, describe, test, beforeEach, afterEach, expect } from '@jest/globals'; +import { retryWithBackoff } from '../lib/billing.retry.js'; /** * Unit tests — checkout.session.completed PaymentIntent metadata backfill retry @@ -23,6 +24,10 @@ describe('checkout.session.completed — metadata backfill retry:', () => { const stripeSessionId = 'cs_test_session_abc'; const paymentIntentId = 'pi_test_retry_001'; + /** + * Build a minimal Stripe checkout.session fixture for backfill tests. + * @returns {{ id: string, payment_status: string, payment_intent: string, metadata: { organizationId: string, packId: string, kind: string } }} + */ const makeSession = () => ({ id: stripeSessionId, payment_status: 'paid', @@ -189,3 +194,13 @@ describe('checkout.session.completed — metadata backfill retry:', () => { ); }); }); + +describe('retryWithBackoff guards:', () => { + test('throws TypeError when attempts < 1', async () => { + await expect(retryWithBackoff(async () => 'x', { attempts: 0 })).rejects.toThrow(TypeError); + }); + + test('throws TypeError when baseMs is negative', async () => { + await expect(retryWithBackoff(async () => 'x', { baseMs: -1 })).rejects.toThrow(TypeError); + }); +}); From 0c5ac16890e74f0519cb01ef86aa793db73cf7a3 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Fri, 22 May 2026 09:27:58 +0200 Subject: [PATCH 6/6] fix(billing): add err.stack + consistent stripeSessionId key to backfill logs Fixes two logger.error calls (lines 332 and 344 of billing.webhook.service.js): - rename sessionId key to stripeSessionId for consistency with the rest of the file - add stack: err?.stack to both meta objects, matching surrounding logger.error style Also updates billing.refund-correlation unit test header comment to reference BillingFailedBackfillRepository.record instead of the stale BillingFailedBackfill.create. --- modules/billing/services/billing.webhook.service.js | 4 ++-- .../billing/tests/billing.refund-correlation.unit.tests.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/billing/services/billing.webhook.service.js b/modules/billing/services/billing.webhook.service.js index a17b73f3f..987edc874 100644 --- a/modules/billing/services/billing.webhook.service.js +++ b/modules/billing/services/billing.webhook.service.js @@ -328,7 +328,7 @@ const handleCheckoutPaymentCompleted = async (session) => { } catch (err) { logger.error( '[billing.webhook] PI metadata backfill failed after retries — refund correlation at risk', - { paymentIntentId, sessionId: stripeSessionId, error: err?.message ?? String(err) }, + { paymentIntentId, stripeSessionId, error: err?.message ?? String(err), stack: err?.stack }, ); try { await BillingFailedBackfillRepository.record({ @@ -340,7 +340,7 @@ const handleCheckoutPaymentCompleted = async (session) => { } catch (dlqErr) { logger.error( '[billing.webhook] dead-letter write failed — manual reconciliation required', - { paymentIntentId, sessionId: stripeSessionId, error: dlqErr?.message ?? String(dlqErr) }, + { paymentIntentId, stripeSessionId, error: dlqErr?.message ?? String(dlqErr), stack: dlqErr?.stack }, ); } } diff --git a/modules/billing/tests/billing.refund-correlation.unit.tests.js b/modules/billing/tests/billing.refund-correlation.unit.tests.js index 227bb4c59..2593e2aad 100644 --- a/modules/billing/tests/billing.refund-correlation.unit.tests.js +++ b/modules/billing/tests/billing.refund-correlation.unit.tests.js @@ -9,7 +9,7 @@ import { retryWithBackoff } from '../lib/billing.retry.js'; * * Covers: * - retryWithBackoff: 3 attempts on Stripe API failure before escalating - * - Dead-letter: BillingFailedBackfill.create called after 3 consecutive failures + * - Dead-letter: BillingFailedBackfillRepository.record called after 3 consecutive failures * - Dead-letter write failure: logger.error emitted, handler does not throw */