Skip to content

Commit b537f9d

Browse files
fix(billing): classify non-transient Stripe errors via rawType + auth/permission
The retry short-circuit predicate checked err.type === 'invalid_request_error', but stripe-node sets err.type to the class name (StripeInvalidRequestError) and exposes the wire type on err.rawType — so that string was dead for SDK errors. It also missed StripeAuthenticationError (401) and StripePermissionError (403), which are equally deterministic and wasted the full retry budget. Extract isNonTransientStripeError() in billing.stripe-errors.js: match the SDK class names on .type plus invalid_request_error on .type/.rawType, and wire it into the PI-backfill retry. billing.admin.controller.js left as-is (different HTTP-422 mapping semantics; can adopt the helper later). Closes #3699
1 parent b7ea2e3 commit b537f9d

3 files changed

Lines changed: 80 additions & 5 deletions

File tree

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* Classify Stripe errors for retry / short-circuit decisions.
3+
*
4+
* stripe-node sets `err.type` to the error CLASS NAME (Error.js: `this.type = type || this.constructor.name`,
5+
* and every subclass passes its own name via `super(raw, 'StripeXError')`), while the raw API error type
6+
* string (e.g. `'invalid_request_error'`) lives on `err.rawType`. Unwrapped/raw API error objects instead
7+
* carry the wire type directly on `.type`. Both shapes are handled below.
8+
*/
9+
10+
const NON_TRANSIENT_STRIPE_ERROR_CLASSES = new Set([
11+
'StripeInvalidRequestError', // 400/404 — bad params, deterministic
12+
'StripeAuthenticationError', // 401 — bad/missing API key, deterministic
13+
'StripePermissionError', // 403 — key lacks permission for the resource, deterministic
14+
]);
15+
16+
/**
17+
* True when a Stripe error is deterministic and will never succeed on retry
18+
* (invalid request, authentication, or permission failures). Transient errors
19+
* (api_error/500, connection, rate_limit/429) return false so they keep retrying.
20+
*
21+
* @param {unknown} err
22+
* @returns {boolean}
23+
*/
24+
export function isNonTransientStripeError(err) {
25+
if (!err || typeof err !== 'object') return false;
26+
// SDK-wrapped errors expose the class name on `.type`.
27+
if (NON_TRANSIENT_STRIPE_ERROR_CLASSES.has(err.type)) return true;
28+
// SDK mirrors the wire type on `.rawType`; unwrapped API error objects carry it on `.type`.
29+
return err.rawType === 'invalid_request_error' || err.type === 'invalid_request_error';
30+
}

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import BillingResetService from './billing.reset.service.js';
1515
import billingEvents from '../lib/events.js';
1616
import { SENTINEL_PENDING } from '../lib/billing.constants.js';
1717
import { retryWithBackoff } from '../lib/billing.retry.js';
18+
import { isNonTransientStripeError } from '../lib/billing.stripe-errors.js';
1819

1920
/**
2021
* Treats a stripeSessionId as "unresolved" when absent, empty, or still the
@@ -326,11 +327,9 @@ const handleCheckoutPaymentCompleted = async (session) => {
326327
{
327328
attempts: 3,
328329
baseMs: 200,
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.
332-
shouldRetry: (err) =>
333-
err?.type !== 'StripeInvalidRequestError' && err?.type !== 'invalid_request_error',
330+
// Skip retries on deterministic Stripe errors (invalid request / auth / permission) —
331+
// they never succeed on retry and only delay the dead-letter path. See billing.stripe-errors.js.
332+
shouldRetry: (err) => !isNonTransientStripeError(err),
334333
},
335334
);
336335
} catch (err) {
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/**
2+
* Module dependencies.
3+
*/
4+
import { describe, test, expect } from '@jest/globals';
5+
import { isNonTransientStripeError } from '../lib/billing.stripe-errors.js';
6+
7+
/**
8+
* Unit tests for isNonTransientStripeError — deterministic Stripe errors that
9+
* should short-circuit retries (invalid request / authentication / permission),
10+
* across both SDK-wrapped (.type = class name) and raw (.type = wire string) shapes.
11+
*/
12+
describe('isNonTransientStripeError', () => {
13+
test.each(['StripeInvalidRequestError', 'StripeAuthenticationError', 'StripePermissionError'])(
14+
'returns true for SDK error class %s (err.type = class name)',
15+
(type) => {
16+
expect(isNonTransientStripeError({ type })).toBe(true);
17+
},
18+
);
19+
20+
test('returns true for an SDK-wrapped error carrying rawType invalid_request_error', () => {
21+
expect(
22+
isNonTransientStripeError({ type: 'StripeInvalidRequestError', rawType: 'invalid_request_error' }),
23+
).toBe(true);
24+
});
25+
26+
test('returns true for an unwrapped API error object (type = invalid_request_error)', () => {
27+
expect(isNonTransientStripeError({ type: 'invalid_request_error' })).toBe(true);
28+
});
29+
30+
test.each(['StripeAPIError', 'StripeConnectionError', 'StripeRateLimitError'])(
31+
'returns false for transient SDK error class %s',
32+
(type) => {
33+
expect(isNonTransientStripeError({ type })).toBe(false);
34+
},
35+
);
36+
37+
test('returns false for a generic non-Stripe error', () => {
38+
expect(isNonTransientStripeError(new Error('boom'))).toBe(false);
39+
});
40+
41+
test('returns false for null / undefined / non-object', () => {
42+
expect(isNonTransientStripeError(null)).toBe(false);
43+
expect(isNonTransientStripeError(undefined)).toBe(false);
44+
expect(isNonTransientStripeError('invalid_request_error')).toBe(false);
45+
});
46+
});

0 commit comments

Comments
 (0)