Skip to content

Commit 3c27a01

Browse files
fix(billing): address reviewer nits — narrow comment + improve log wording
- Narrow shouldRetry JSDoc comment to only mention invalid_request_error (not auth errors) — matches what the predicate actually checks (Copilot thread PRRT_kwDOBss37M6EM5HL) - Change failure log from "failed after retries" to "failed (retries exhausted or skipped)" to reflect short-circuit path (CodeRabbit thread PRRT_kwDOBss37M6EM5c1) - Update billing.refund-correlation tests to match new log wording
1 parent 46efee0 commit 3c27a01

2 files changed

Lines changed: 6 additions & 6 deletions

File tree

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -326,16 +326,16 @@ const handleCheckoutPaymentCompleted = async (session) => {
326326
{
327327
attempts: 3,
328328
baseMs: 200,
329-
// Skip retries on deterministic Stripe client errors (bad params / auth) — they
330-
// never succeed on retry and only delay the dead-letter path. Same classification
331-
// as billing.admin.controller.js.
329+
// Skip retries on Stripe invalid-request errors (invalid_request_error /
330+
// StripeInvalidRequestError) — these are deterministic client errors that never
331+
// succeed on retry and only delay the dead-letter path.
332332
shouldRetry: (err) =>
333333
err?.type !== 'StripeInvalidRequestError' && err?.type !== 'invalid_request_error',
334334
},
335335
);
336336
} catch (err) {
337337
logger.error(
338-
'[billing.webhook] PI metadata backfill failed after retries — refund correlation at risk',
338+
'[billing.webhook] PI metadata backfill failed (retries exhausted or skipped) — refund correlation at risk',
339339
{ paymentIntentId, stripeSessionId, error: err?.message ?? String(err), stack: err?.stack },
340340
);
341341
try {

modules/billing/tests/billing.refund-correlation.unit.tests.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ describe('checkout.session.completed — metadata backfill retry:', () => {
158158

159159
// logger.error should be called for the final failure.
160160
expect(mockLogger.error).toHaveBeenCalledWith(
161-
expect.stringContaining('backfill failed after retries'),
161+
expect.stringContaining('backfill failed (retries exhausted or skipped)'),
162162
expect.objectContaining({ paymentIntentId }),
163163
);
164164

@@ -185,7 +185,7 @@ describe('checkout.session.completed — metadata backfill retry:', () => {
185185
// Both error paths should be logged.
186186
expect(mockLogger.error).toHaveBeenCalledTimes(2);
187187
expect(mockLogger.error).toHaveBeenCalledWith(
188-
expect.stringContaining('backfill failed after retries'),
188+
expect.stringContaining('backfill failed (retries exhausted or skipped)'),
189189
expect.objectContaining({ paymentIntentId }),
190190
);
191191
expect(mockLogger.error).toHaveBeenCalledWith(

0 commit comments

Comments
 (0)