fix(billing): classify non-transient Stripe errors via rawType + auth/permission (#3700)

* 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

* fix(billing): add StripeIdempotencyError to non-transient set + cover rawType branch

Pre-push review flagged the general isNonTransientStripeError helper omitted
StripeIdempotencyError (400, deterministic — reused key with conflicting params).
Add it. StripeCardError (402) stays excluded since some decline codes
(processing_error, issuer_unavailable) are transient. Add tests for the
rawType-only branch and the card exclusion; drop a mislabeled test.

Author: PierreBrisorgueil

modules/billing/lib/billing.stripe-errors.js

@@ -0,0 +1,33 @@
+/**
+ * Classify Stripe errors for retry / short-circuit decisions.
+ *
+ * stripe-node sets `err.type` to the error CLASS NAME (Error.js: `this.type = type || this.constructor.name`,
+ * and every subclass passes its own name via `super(raw, 'StripeXError')`), while the raw API error type
+ * string (e.g. `'invalid_request_error'`) lives on `err.rawType`. Unwrapped/raw API error objects instead
+ * carry the wire type directly on `.type`. Both shapes are handled below.
+ */
+
+const NON_TRANSIENT_STRIPE_ERROR_CLASSES = new Set([
+  'StripeInvalidRequestError', // 400/404 — bad params, deterministic
+  'StripeIdempotencyError', // 400 — idempotency key reused with conflicting params, deterministic
+  'StripeAuthenticationError', // 401 — bad/missing API key, deterministic
+  'StripePermissionError', // 403 — key lacks permission for the resource, deterministic
+]);
+
+/**
+ * True when a Stripe error is deterministic and will never succeed on retry
+ * (invalid request, idempotency, authentication, or permission failures). Transient
+ * errors (api_error/500, connection, rate_limit/429) return false so they keep
+ * retrying. StripeCardError (402) is intentionally excluded — some decline codes
+ * (processing_error, issuer_unavailable) are transient.
+ *
+ * @param {unknown} err
+ * @returns {boolean}
+ */
+export function isNonTransientStripeError(err) {
+  if (!err || typeof err !== 'object') return false;
+  // SDK-wrapped errors expose the class name on `.type`.
+  if (NON_TRANSIENT_STRIPE_ERROR_CLASSES.has(err.type)) return true;
+  // SDK mirrors the wire type on `.rawType`; unwrapped API error objects carry it on `.type`.
+  return err.rawType === 'invalid_request_error' || err.type === 'invalid_request_error';
+}

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

@@ -15,6 +15,7 @@ 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';
+import { isNonTransientStripeError } from '../lib/billing.stripe-errors.js';
 
 /**
  * Treats a stripeSessionId as "unresolved" when absent, empty, or still the
@@ -326,11 +327,9 @@ const handleCheckoutPaymentCompleted = async (session) => {
           {
             attempts: 3,
             baseMs: 200,
-            // Skip retries on Stripe invalid-request errors (invalid_request_error /
-            // StripeInvalidRequestError) — these are deterministic client errors that never
-            // succeed on retry and only delay the dead-letter path.
-            shouldRetry: (err) =>
-              err?.type !== 'StripeInvalidRequestError' && err?.type !== 'invalid_request_error',
+            // Skip retries on deterministic Stripe errors (invalid request / auth / permission) —
+            // they never succeed on retry and only delay the dead-letter path. See billing.stripe-errors.js.
+            shouldRetry: (err) => !isNonTransientStripeError(err),
           },
         );
       } catch (err) {

modules/billing/tests/billing.stripe-errors.unit.tests.js

@@ -0,0 +1,52 @@
+/**
+ * Module dependencies.
+ */
+import { describe, test, expect } from '@jest/globals';
+import { isNonTransientStripeError } from '../lib/billing.stripe-errors.js';
+
+/**
+ * Unit tests for isNonTransientStripeError — deterministic Stripe errors that
+ * should short-circuit retries (invalid request / authentication / permission),
+ * across both SDK-wrapped (.type = class name) and raw (.type = wire string) shapes.
+ */
+describe('isNonTransientStripeError', () => {
+  test.each([
+    'StripeInvalidRequestError',
+    'StripeIdempotencyError',
+    'StripeAuthenticationError',
+    'StripePermissionError',
+  ])('returns true for SDK error class %s (err.type = class name)', (type) => {
+    expect(isNonTransientStripeError({ type })).toBe(true);
+  });
+
+  test('returns true via the rawType branch when the class is not listed', () => {
+    expect(isNonTransientStripeError({ type: 'StripeFutureUnknownError', rawType: 'invalid_request_error' })).toBe(
+      true,
+    );
+  });
+
+  test('returns true for an unwrapped API error object (type = invalid_request_error)', () => {
+    expect(isNonTransientStripeError({ type: 'invalid_request_error' })).toBe(true);
+  });
+
+  test.each(['StripeAPIError', 'StripeConnectionError', 'StripeRateLimitError'])(
+    'returns false for transient SDK error class %s',
+    (type) => {
+      expect(isNonTransientStripeError({ type })).toBe(false);
+    },
+  );
+
+  test('returns false for StripeCardError (402 — some decline codes are transient)', () => {
+    expect(isNonTransientStripeError({ type: 'StripeCardError' })).toBe(false);
+  });
+
+  test('returns false for a generic non-Stripe error', () => {
+    expect(isNonTransientStripeError(new Error('boom'))).toBe(false);
+  });
+
+  test('returns false for null / undefined / non-object', () => {
+    expect(isNonTransientStripeError(null)).toBe(false);
+    expect(isNonTransientStripeError(undefined)).toBe(false);
+    expect(isNonTransientStripeError('invalid_request_error')).toBe(false);
+  });
+});