From eb894994878ca376d529172f74d7ec0ba0aaba19 Mon Sep 17 00:00:00 2001 From: eli Date: Wed, 18 Feb 2026 09:59:47 -0600 Subject: [PATCH 1/9] Add retry mechanism for HTTP requests - Add built-in retry support with exponential backoff for transient failures - Retry on network errors and status codes: 408, 429, 500, 502, 503, 504 - Respect Retry-After headers for rate limiting (429) - Default: 3 retries with exponential backoff (1s, 2s, 4s) - Add IRetryConfig interface for full configuration control - Support separate retry config for PDP calls via pdpRetry option - Enable POST retry for PDP calls (check operations are idempotent) - Add comprehensive unit tests (33 tests) - Update README with retry configuration documentation Co-Authored-By: Claude Opus 4.5 --- README.md | 41 ++++ src/config.ts | 19 ++ src/enforcement/enforcer.ts | 16 ++ src/index.ts | 19 ++ src/tests/unit/retry.spec.ts | 339 +++++++++++++++++++++++++++++++++ src/utils/retry-interceptor.ts | 91 +++++++++ src/utils/retry.ts | 200 +++++++++++++++++++ yarn.lock | 47 ++--- 8 files changed, 743 insertions(+), 29 deletions(-) create mode 100644 src/tests/unit/retry.spec.ts create mode 100644 src/utils/retry-interceptor.ts create mode 100644 src/utils/retry.ts diff --git a/README.md b/README.md index 94ba1b7f..8f74f8c3 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,47 @@ npm install permitio 3. Execute `yarn docs ; git add docs/ ; git commit -m "update tsdoc"` to update the auto generated docs 4. Execute `yarn publish --access public` +## Retry Configuration + +The SDK includes built-in retry support for transient failures. By default, retries are **enabled** with the following settings: + +- **3 retry attempts** with exponential backoff +- Retries on network errors and status codes: `408`, `429`, `500`, `502`, `503`, `504` +- Respects `Retry-After` headers for rate limiting (429) + +### Customizing Retry Behavior + +```typescript +import { Permit } from 'permitio'; + +// Use defaults (retry enabled) +const permit = new Permit({ token: 'your-api-key' }); + +// Custom retry configuration +const permit = new Permit({ + token: 'your-api-key', + retry: { + maxRetries: 5, + retryDelay: 500, // Initial delay in ms + backoffMultiplier: 2, // Exponential backoff multiplier + maxDelay: 30000, // Maximum delay cap + }, +}); + +// Disable retry entirely +const permit = new Permit({ + token: 'your-api-key', + retry: false, +}); + +// Different config for PDP vs REST API +const permit = new Permit({ + token: 'your-api-key', + retry: { maxRetries: 3 }, + pdpRetry: { maxRetries: 5 }, +}); +``` + ## Documentation [Read the documentation at Permit.io website](https://docs.permit.io/sdk/nodejs/quickstart-nodejs#add-the-sdk-to-your-js-code) diff --git a/src/config.ts b/src/config.ts index a0218148..476c5e94 100644 --- a/src/config.ts +++ b/src/config.ts @@ -2,6 +2,7 @@ import globalAxios, { AxiosInstance } from 'axios'; import _ from 'lodash'; import { ApiContext } from './api/context'; +import { IRetryConfig } from './utils/retry'; import { RecursivePartial } from './utils/types'; export type FactsSyncTimeoutPolicy = 'ignore' | 'fail'; @@ -110,6 +111,24 @@ export interface IPermitConfig { * @see https://axios-http.com/docs/req_config */ opaAxiosInstance?: AxiosInstance; + + /** + * Configuration for automatic retry of failed requests. + * Set to false to disable retries entirely. + * Defaults to enabled with sensible defaults (3 retries, exponential backoff). + * + * @see {@link IRetryConfig} + */ + retry?: IRetryConfig | false; + + /** + * Optional separate retry configuration for PDP (enforcement) calls. + * If not provided, uses the main `retry` configuration. + * Set to false to disable retries for PDP calls only. + * + * @see {@link IRetryConfig} + */ + pdpRetry?: IRetryConfig | false; } /** diff --git a/src/enforcement/enforcer.ts b/src/enforcement/enforcer.ts index cf1ec9fb..410cd8d5 100644 --- a/src/enforcement/enforcer.ts +++ b/src/enforcement/enforcer.ts @@ -5,6 +5,8 @@ import URL from 'url-parse'; import { IPermitConfig } from '../config'; import { CheckConfig, Context, ContextStore } from '../utils/context'; import { AxiosLoggingInterceptor } from '../utils/http-logger'; +import { resolveRetryConfig } from '../utils/retry'; +import { AxiosRetryInterceptor } from '../utils/retry-interceptor'; import { AllTenantsResponse, @@ -163,6 +165,20 @@ export class Enforcer implements IEnforcer { } this.logger = logger; AxiosLoggingInterceptor.setupInterceptor(this.client, this.logger); + + // Setup retry interceptors for PDP clients + // Use pdpRetry config if provided, otherwise fall back to main retry config + const pdpRetryConfig = resolveRetryConfig(config.pdpRetry ?? config.retry); + if (pdpRetryConfig.enabled) { + // For PDP calls, enable POST retry since check operations are idempotent + const pdpRetryWithPost = { + ...pdpRetryConfig, + retryMethods: [...pdpRetryConfig.retryMethods, 'POST'], + }; + AxiosRetryInterceptor.setupInterceptor(this.client, pdpRetryWithPost, this.logger, 'PDP'); + AxiosRetryInterceptor.setupInterceptor(this.opaClient, pdpRetryWithPost, this.logger, 'OPA'); + } + this.contextStore = new ContextStore(); } diff --git a/src/index.ts b/src/index.ts index 082d4559..00dd70a8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -15,6 +15,8 @@ import { import { LoggerFactory } from './logger'; import { CheckConfig, Context } from './utils/context'; import { AxiosLoggingInterceptor } from './utils/http-logger'; +import { resolveRetryConfig } from './utils/retry'; +import { AxiosRetryInterceptor } from './utils/retry-interceptor'; import { RecursivePartial } from './utils/types'; // exported interfaces @@ -25,6 +27,12 @@ export { PermitConnectionError, PermitError, PermitPDPStatusError } from './enfo export { Context, ContextTransform } from './utils/context'; export { ApiContext, PermitContextError, ApiKeyLevel } from './api/context'; export { PermitApiError } from './api/base'; +export { + IRetryConfig, + RetryConditionFn, + RETRYABLE_STATUS_CODES, + NON_RETRYABLE_STATUS_CODES, +} from './utils/retry'; export interface IPermitClient extends IEnforcer { /** @@ -140,6 +148,17 @@ export class Permit implements IPermitClient { this.logger = LoggerFactory.createLogger(this.config); AxiosLoggingInterceptor.setupInterceptor(this.config.axiosInstance, this.logger); + // Setup retry interceptor for REST API calls + const resolvedRetryConfig = resolveRetryConfig(this.config.retry); + if (resolvedRetryConfig.enabled) { + AxiosRetryInterceptor.setupInterceptor( + this.config.axiosInstance, + resolvedRetryConfig, + this.logger, + 'API', + ); + } + this.api = new ApiClient(this.config, this.logger); this.enforcer = new Enforcer(this.config, this.logger); diff --git a/src/tests/unit/retry.spec.ts b/src/tests/unit/retry.spec.ts new file mode 100644 index 00000000..b4559240 --- /dev/null +++ b/src/tests/unit/retry.spec.ts @@ -0,0 +1,339 @@ +import test from 'ava'; +import { AxiosError, AxiosHeaders } from 'axios'; + +import { + DEFAULT_RETRY_CONFIG, + defaultRetryCondition, + calculateRetryDelay, + parseRetryAfter, + resolveRetryConfig, + RETRYABLE_STATUS_CODES, + NON_RETRYABLE_STATUS_CODES, + IRetryConfig, +} from '../../utils/retry'; + +// Helper to create mock AxiosError +function createAxiosError(status?: number): AxiosError { + const error = new Error('Request failed') as AxiosError; + error.isAxiosError = true; + error.config = { headers: new AxiosHeaders() }; + error.toJSON = () => ({}); + + if (status !== undefined) { + error.response = { + status, + statusText: 'Error', + headers: {}, + config: { headers: new AxiosHeaders() }, + data: {}, + }; + } + + return error; +} + +// ============================================ +// Tests for RETRYABLE_STATUS_CODES +// ============================================ + +test('RETRYABLE_STATUS_CODES contains expected status codes', (t) => { + t.deepEqual(RETRYABLE_STATUS_CODES, [408, 429, 500, 502, 503, 504]); +}); + +test('NON_RETRYABLE_STATUS_CODES contains expected status codes', (t) => { + t.deepEqual(NON_RETRYABLE_STATUS_CODES, [400, 401, 403, 404, 422]); +}); + +// ============================================ +// Tests for defaultRetryCondition +// ============================================ + +test('defaultRetryCondition returns true for network errors (no response)', (t) => { + const error = createAxiosError(); + t.true(defaultRetryCondition(error)); +}); + +test('defaultRetryCondition returns true for 408 Request Timeout', (t) => { + const error = createAxiosError(408); + t.true(defaultRetryCondition(error)); +}); + +test('defaultRetryCondition returns true for 429 Too Many Requests', (t) => { + const error = createAxiosError(429); + t.true(defaultRetryCondition(error)); +}); + +test('defaultRetryCondition returns true for 500 Internal Server Error', (t) => { + const error = createAxiosError(500); + t.true(defaultRetryCondition(error)); +}); + +test('defaultRetryCondition returns true for 502 Bad Gateway', (t) => { + const error = createAxiosError(502); + t.true(defaultRetryCondition(error)); +}); + +test('defaultRetryCondition returns true for 503 Service Unavailable', (t) => { + const error = createAxiosError(503); + t.true(defaultRetryCondition(error)); +}); + +test('defaultRetryCondition returns true for 504 Gateway Timeout', (t) => { + const error = createAxiosError(504); + t.true(defaultRetryCondition(error)); +}); + +test('defaultRetryCondition returns false for 400 Bad Request', (t) => { + const error = createAxiosError(400); + t.false(defaultRetryCondition(error)); +}); + +test('defaultRetryCondition returns false for 401 Unauthorized', (t) => { + const error = createAxiosError(401); + t.false(defaultRetryCondition(error)); +}); + +test('defaultRetryCondition returns false for 403 Forbidden', (t) => { + const error = createAxiosError(403); + t.false(defaultRetryCondition(error)); +}); + +test('defaultRetryCondition returns false for 404 Not Found', (t) => { + const error = createAxiosError(404); + t.false(defaultRetryCondition(error)); +}); + +test('defaultRetryCondition returns false for 422 Unprocessable Entity', (t) => { + const error = createAxiosError(422); + t.false(defaultRetryCondition(error)); +}); + +test('defaultRetryCondition returns false for 200 OK', (t) => { + const error = createAxiosError(200); + t.false(defaultRetryCondition(error)); +}); + +// ============================================ +// Tests for parseRetryAfter +// ============================================ + +test('parseRetryAfter parses integer seconds correctly', (t) => { + t.is(parseRetryAfter('5'), 5000); + t.is(parseRetryAfter('60'), 60000); + t.is(parseRetryAfter('0'), 0); + t.is(parseRetryAfter('120'), 120000); +}); + +test('parseRetryAfter returns null for invalid values', (t) => { + t.is(parseRetryAfter('invalid'), null); + t.is(parseRetryAfter(''), null); + t.is(parseRetryAfter('abc123'), null); +}); + +test('parseRetryAfter parses HTTP-date format', (t) => { + // Use a future date + const futureDate = new Date(Date.now() + 10000); + const httpDate = futureDate.toUTCString(); + const result = parseRetryAfter(httpDate); + + t.truthy(result); + // Should be approximately 10 seconds (10000ms), with some tolerance + t.true(result! > 9000 && result! < 11000); +}); + +test('parseRetryAfter returns 0 for past HTTP-date', (t) => { + const pastDate = new Date(Date.now() - 10000); + const httpDate = pastDate.toUTCString(); + const result = parseRetryAfter(httpDate); + + t.is(result, 0); +}); + +// ============================================ +// Tests for calculateRetryDelay +// ============================================ + +test('calculateRetryDelay calculates exponential backoff correctly', (t) => { + const config = { ...DEFAULT_RETRY_CONFIG, retryDelay: 1000, backoffMultiplier: 2 }; + + // First retry (attempt 0): 1000 * 2^0 = 1000ms + jitter + const delay0 = calculateRetryDelay(0, config); + t.true(delay0 >= 1000 && delay0 <= 1100); // 10% jitter max + + // Second retry (attempt 1): 1000 * 2^1 = 2000ms + jitter + const delay1 = calculateRetryDelay(1, config); + t.true(delay1 >= 2000 && delay1 <= 2200); + + // Third retry (attempt 2): 1000 * 2^2 = 4000ms + jitter + const delay2 = calculateRetryDelay(2, config); + t.true(delay2 >= 4000 && delay2 <= 4400); +}); + +test('calculateRetryDelay respects maxDelay limit', (t) => { + const config = { + ...DEFAULT_RETRY_CONFIG, + retryDelay: 1000, + backoffMultiplier: 10, + maxDelay: 5000, + }; + + // Even with high multiplier, should cap at maxDelay + const delay = calculateRetryDelay(5, config); + t.true(delay <= 5000); +}); + +test('calculateRetryDelay respects Retry-After header', (t) => { + const config = { ...DEFAULT_RETRY_CONFIG, respectRetryAfter: true }; + + const delay = calculateRetryDelay(0, config, '3'); + t.is(delay, 3000); +}); + +test('calculateRetryDelay caps Retry-After at maxDelay', (t) => { + const config = { ...DEFAULT_RETRY_CONFIG, respectRetryAfter: true, maxDelay: 5000 }; + + const delay = calculateRetryDelay(0, config, '60'); + t.is(delay, 5000); +}); + +test('calculateRetryDelay ignores Retry-After when disabled', (t) => { + const config = { + ...DEFAULT_RETRY_CONFIG, + respectRetryAfter: false, + retryDelay: 1000, + backoffMultiplier: 2, + }; + + const delay = calculateRetryDelay(0, config, '60'); + // Should use exponential backoff, not Retry-After + t.true(delay >= 1000 && delay <= 1100); +}); + +// ============================================ +// Tests for resolveRetryConfig +// ============================================ + +test('resolveRetryConfig returns defaults when config is undefined', (t) => { + const resolved = resolveRetryConfig(undefined); + + t.true(resolved.enabled); + t.is(resolved.maxRetries, 3); + t.is(resolved.retryDelay, 1000); + t.is(resolved.backoffMultiplier, 2); + t.is(resolved.maxDelay, 30000); + t.true(resolved.respectRetryAfter); + t.deepEqual(resolved.retryMethods, ['GET', 'HEAD', 'OPTIONS', 'PUT', 'DELETE']); +}); + +test('resolveRetryConfig returns disabled config when false', (t) => { + const resolved = resolveRetryConfig(false); + + t.false(resolved.enabled); + // Other defaults should still be present + t.is(resolved.maxRetries, 3); +}); + +test('resolveRetryConfig merges user config with defaults', (t) => { + const userConfig: IRetryConfig = { + maxRetries: 5, + retryDelay: 500, + }; + + const resolved = resolveRetryConfig(userConfig); + + t.true(resolved.enabled); + t.is(resolved.maxRetries, 5); // User value + t.is(resolved.retryDelay, 500); // User value + t.is(resolved.backoffMultiplier, 2); // Default + t.is(resolved.maxDelay, 30000); // Default +}); + +test('resolveRetryConfig allows custom retry condition', (t) => { + const customCondition = () => false; + const userConfig: IRetryConfig = { + retryCondition: customCondition, + }; + + const resolved = resolveRetryConfig(userConfig); + + t.is(resolved.retryCondition, customCondition); +}); + +test('resolveRetryConfig allows custom retry methods', (t) => { + const userConfig: IRetryConfig = { + retryMethods: ['GET', 'POST'], + }; + + const resolved = resolveRetryConfig(userConfig); + + t.deepEqual(resolved.retryMethods, ['GET', 'POST']); +}); + +test('resolveRetryConfig allows disabling via enabled: false', (t) => { + const userConfig: IRetryConfig = { + enabled: false, + maxRetries: 10, + }; + + const resolved = resolveRetryConfig(userConfig); + + t.false(resolved.enabled); + t.is(resolved.maxRetries, 10); +}); + +// ============================================ +// Tests for DEFAULT_RETRY_CONFIG +// ============================================ + +test('DEFAULT_RETRY_CONFIG has expected default values', (t) => { + t.true(DEFAULT_RETRY_CONFIG.enabled); + t.is(DEFAULT_RETRY_CONFIG.maxRetries, 3); + t.is(DEFAULT_RETRY_CONFIG.retryDelay, 1000); + t.is(DEFAULT_RETRY_CONFIG.backoffMultiplier, 2); + t.is(DEFAULT_RETRY_CONFIG.maxDelay, 30000); + t.true(DEFAULT_RETRY_CONFIG.respectRetryAfter); + t.deepEqual(DEFAULT_RETRY_CONFIG.retryMethods, ['GET', 'HEAD', 'OPTIONS', 'PUT', 'DELETE']); + t.is(typeof DEFAULT_RETRY_CONFIG.retryCondition, 'function'); +}); + +// ============================================ +// Tests for exports from SDK +// ============================================ + +test('retry types are exported from SDK', async (t) => { + const sdk = await import('../../index'); + + t.truthy(sdk.RETRYABLE_STATUS_CODES); + t.truthy(sdk.NON_RETRYABLE_STATUS_CODES); + t.deepEqual(sdk.RETRYABLE_STATUS_CODES, [408, 429, 500, 502, 503, 504]); +}); + +test('Permit accepts retry configuration', async (t) => { + const { Permit } = await import('../../index'); + + // With retry enabled (default) + const permit1 = new Permit({ token: 'test' }); + t.truthy(permit1); + + // With custom retry config + const permit2 = new Permit({ + token: 'test', + retry: { maxRetries: 5 }, + }); + t.truthy(permit2); + + // With retry disabled + const permit3 = new Permit({ + token: 'test', + retry: false, + }); + t.truthy(permit3); + + // With separate PDP retry config + const permit4 = new Permit({ + token: 'test', + retry: { maxRetries: 3 }, + pdpRetry: { maxRetries: 5 }, + }); + t.truthy(permit4); +}); diff --git a/src/utils/retry-interceptor.ts b/src/utils/retry-interceptor.ts new file mode 100644 index 00000000..9d57ae6f --- /dev/null +++ b/src/utils/retry-interceptor.ts @@ -0,0 +1,91 @@ +import { AxiosError, AxiosInstance, InternalAxiosRequestConfig } from 'axios'; +import { Logger } from 'pino'; + +import { calculateRetryDelay, IResolvedRetryConfig } from './retry'; + +// Symbols to track retry state on request config (avoids polluting the config object) +const RETRY_COUNT_KEY = '__permitRetryCount'; + +/** + * Extended request config with retry tracking + */ +interface RetryableRequestConfig extends InternalAxiosRequestConfig { + [RETRY_COUNT_KEY]?: number; +} + +/** + * Axios interceptor that implements retry logic with exponential backoff + */ +export class AxiosRetryInterceptor { + /** + * Setup retry interceptor on an axios instance + * + * @param axiosInstance - The axios instance to add retry capability to + * @param config - Resolved retry configuration + * @param logger - Logger instance for retry attempt logging + * @param clientName - Name of the client for logging purposes (e.g., 'API', 'PDP', 'OPA') + */ + static setupInterceptor( + axiosInstance: AxiosInstance, + config: IResolvedRetryConfig, + logger: Logger, + clientName = 'HTTP', + ): void { + if (!config.enabled) { + return; + } + + axiosInstance.interceptors.response.use( + // Success handler - pass through unchanged + (response) => response, + + // Error handler - implement retry logic + async (error: AxiosError) => { + const originalRequest = error.config as RetryableRequestConfig | undefined; + + // If no request config, cannot retry + if (!originalRequest) { + return Promise.reject(error); + } + + // Initialize or get current retry count + const currentRetryCount = originalRequest[RETRY_COUNT_KEY] ?? 0; + + // Check if we should retry this request + const method = (originalRequest.method ?? 'GET').toUpperCase(); + const shouldRetryMethod = config.retryMethods.includes(method); + const shouldRetryError = config.retryCondition(error); + const hasRetriesLeft = currentRetryCount < config.maxRetries; + + // If any condition fails, reject with original error + if (!shouldRetryMethod || !shouldRetryError || !hasRetriesLeft) { + return Promise.reject(error); + } + + // Increment retry count for next attempt + originalRequest[RETRY_COUNT_KEY] = currentRetryCount + 1; + + // Calculate delay before retry + const retryAfterHeader = error.response?.headers?.['retry-after'] as string | undefined; + const delay = calculateRetryDelay(currentRetryCount, config, retryAfterHeader); + + // Log retry attempt + const statusInfo = error.response?.status ?? 'network error'; + const url = originalRequest.url ?? 'unknown'; + logger.warn( + `[${clientName}] Request failed (${statusInfo}), ` + + `retrying in ${Math.round(delay)}ms (attempt ${currentRetryCount + 1}/${ + config.maxRetries + }): ` + + `${method} ${url}`, + ); + + // Wait for the calculated delay + await new Promise((resolve) => setTimeout(resolve, delay)); + + // Retry the request + return axiosInstance.request(originalRequest); + }, + ); + } +} diff --git a/src/utils/retry.ts b/src/utils/retry.ts new file mode 100644 index 00000000..e02a88b2 --- /dev/null +++ b/src/utils/retry.ts @@ -0,0 +1,200 @@ +import { AxiosError } from 'axios'; + +/** + * HTTP status codes that should trigger a retry + */ +export const RETRYABLE_STATUS_CODES = [408, 429, 500, 502, 503, 504]; + +/** + * HTTP status codes that should NOT trigger a retry + */ +export const NON_RETRYABLE_STATUS_CODES = [400, 401, 403, 404, 422]; + +/** + * Default HTTP methods that are safe to retry + */ +export const DEFAULT_RETRY_METHODS = ['GET', 'HEAD', 'OPTIONS', 'PUT', 'DELETE']; + +/** + * Function type for custom retry condition evaluation + */ +export type RetryConditionFn = (error: AxiosError) => boolean; + +/** + * Configuration options for the retry mechanism + */ +export interface IRetryConfig { + /** + * Whether retry is enabled. Defaults to true. + */ + enabled?: boolean; + + /** + * Maximum number of retry attempts. Defaults to 3. + */ + maxRetries?: number; + + /** + * Initial delay between retries in milliseconds. Defaults to 1000 (1 second). + */ + retryDelay?: number; + + /** + * Multiplier for exponential backoff. Defaults to 2. + * Each retry will wait: retryDelay * (backoffMultiplier ^ attemptNumber) + */ + backoffMultiplier?: number; + + /** + * Maximum delay between retries in milliseconds. Defaults to 30000 (30 seconds). + * Prevents exponential backoff from growing too large. + */ + maxDelay?: number; + + /** + * Custom function to determine if a request should be retried. + * If not provided, uses default retry condition (network errors + retryable status codes). + */ + retryCondition?: RetryConditionFn; + + /** + * Whether to respect the Retry-After header for 429 responses. Defaults to true. + */ + respectRetryAfter?: boolean; + + /** + * HTTP methods to retry. Defaults to ['GET', 'HEAD', 'OPTIONS', 'PUT', 'DELETE']. + * POST is excluded by default as it may not be idempotent. + */ + retryMethods?: string[]; +} + +/** + * Resolved retry configuration with all defaults applied + */ +export interface IResolvedRetryConfig { + enabled: boolean; + maxRetries: number; + retryDelay: number; + backoffMultiplier: number; + maxDelay: number; + retryCondition: RetryConditionFn; + respectRetryAfter: boolean; + retryMethods: string[]; +} + +/** + * Default retry condition: retry on network errors and retryable HTTP status codes + */ +export function defaultRetryCondition(error: AxiosError): boolean { + // Network errors (no response) - connection refused, timeout, etc. + if (!error.response) { + return true; + } + + // Retryable status codes + return RETRYABLE_STATUS_CODES.includes(error.response.status); +} + +/** + * Default retry configuration values + */ +export const DEFAULT_RETRY_CONFIG: IResolvedRetryConfig = { + enabled: true, + maxRetries: 3, + retryDelay: 1000, + backoffMultiplier: 2, + maxDelay: 30000, + retryCondition: defaultRetryCondition, + respectRetryAfter: true, + retryMethods: DEFAULT_RETRY_METHODS, +}; + +/** + * Parse Retry-After header value + * Supports both seconds (integer) and HTTP-date formats + * + * @param value - The Retry-After header value + * @returns The delay in milliseconds, or null if parsing fails + */ +export function parseRetryAfter(value: string): number | null { + // Try parsing as seconds (integer) + const seconds = parseInt(value, 10); + if (!isNaN(seconds)) { + return seconds * 1000; + } + + // Try parsing as HTTP-date (e.g., "Wed, 21 Oct 2015 07:28:00 GMT") + const date = Date.parse(value); + if (!isNaN(date)) { + return Math.max(0, date - Date.now()); + } + + return null; +} + +/** + * Calculate delay for next retry attempt using exponential backoff with jitter + * + * @param attemptNumber - The current retry attempt number (0-indexed) + * @param config - The resolved retry configuration + * @param retryAfterHeader - Optional Retry-After header value from response + * @returns The delay in milliseconds before the next retry + */ +export function calculateRetryDelay( + attemptNumber: number, + config: IResolvedRetryConfig, + retryAfterHeader?: string, +): number { + // Respect Retry-After header if present and configured + if (config.respectRetryAfter && retryAfterHeader) { + const retryAfterMs = parseRetryAfter(retryAfterHeader); + if (retryAfterMs !== null) { + return Math.min(retryAfterMs, config.maxDelay); + } + } + + // Exponential backoff: delay * (multiplier ^ attempt) + const exponentialDelay = config.retryDelay * Math.pow(config.backoffMultiplier, attemptNumber); + + // Add jitter (0-10% of the delay) to prevent thundering herd + const jitter = Math.random() * 0.1 * exponentialDelay; + + // Apply max delay cap + return Math.min(exponentialDelay + jitter, config.maxDelay); +} + +/** + * Resolve user-provided retry config with defaults + * + * @param userConfig - User-provided retry configuration or false to disable + * @returns Resolved configuration with all defaults applied + */ +export function resolveRetryConfig( + userConfig: IRetryConfig | false | undefined, +): IResolvedRetryConfig { + // If explicitly disabled, return disabled config + if (userConfig === false) { + return { + ...DEFAULT_RETRY_CONFIG, + enabled: false, + }; + } + + // If undefined or empty, use defaults + if (!userConfig) { + return DEFAULT_RETRY_CONFIG; + } + + // Merge user config with defaults + return { + enabled: userConfig.enabled ?? DEFAULT_RETRY_CONFIG.enabled, + maxRetries: userConfig.maxRetries ?? DEFAULT_RETRY_CONFIG.maxRetries, + retryDelay: userConfig.retryDelay ?? DEFAULT_RETRY_CONFIG.retryDelay, + backoffMultiplier: userConfig.backoffMultiplier ?? DEFAULT_RETRY_CONFIG.backoffMultiplier, + maxDelay: userConfig.maxDelay ?? DEFAULT_RETRY_CONFIG.maxDelay, + retryCondition: userConfig.retryCondition ?? DEFAULT_RETRY_CONFIG.retryCondition, + respectRetryAfter: userConfig.respectRetryAfter ?? DEFAULT_RETRY_CONFIG.respectRetryAfter, + retryMethods: userConfig.retryMethods ?? DEFAULT_RETRY_CONFIG.retryMethods, + }; +} diff --git a/yarn.lock b/yarn.lock index bf3d8fb3..f1da6aa1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -22,22 +22,7 @@ dependencies: escape-string-regexp "^2.0.0" -"@babel/code-frame@^7.0.0", "@babel/code-frame@7.12.11": - version "7.12.11" - resolved "https://registry.npmjs.org/@babel/code-frame/-/code-frame-7.12.11.tgz" - integrity sha512-Zt1yodBx1UcyiePMSkWnU4hPqhwq7hGi2nFL1LeA3EUl+q2LQx16MISgJ0+z7dnmgvP9QtIleuETGOiOH1RcIw== - dependencies: - "@babel/highlight" "^7.10.4" - -"@babel/code-frame@^7.22.10": - version "7.22.10" - resolved "https://registry.npmjs.org/@babel/code-frame/-/code-frame-7.22.10.tgz" - integrity sha512-/KKIMG4UEL35WmI9OlvMhurwtytjvXoFcGNrOvyG9zIzA8YmPjVtIZUf7b05+TPO7G7/GEmLHDaoCgACHl9hhA== - dependencies: - "@babel/highlight" "^7.22.10" - chalk "^2.4.2" - -"@babel/code-frame@^7.27.1": +"@babel/code-frame@^7.0.0", "@babel/code-frame@^7.22.10", "@babel/code-frame@^7.27.1": version "7.27.1" resolved "https://registry.npmjs.org/@babel/code-frame/-/code-frame-7.27.1.tgz" integrity sha512-cjQ7ZlQ0Mv3b47hABuTevyTuYN4i+loJKGeV9flcCgIK37cCXRh+L1bd3iBHlynerhQ7BhCkn2BPbQUL+rGqFg== @@ -46,6 +31,13 @@ js-tokens "^4.0.0" picocolors "^1.1.1" +"@babel/code-frame@7.12.11": + version "7.12.11" + resolved "https://registry.npmjs.org/@babel/code-frame/-/code-frame-7.12.11.tgz" + integrity sha512-Zt1yodBx1UcyiePMSkWnU4hPqhwq7hGi2nFL1LeA3EUl+q2LQx16MISgJ0+z7dnmgvP9QtIleuETGOiOH1RcIw== + dependencies: + "@babel/highlight" "^7.10.4" + "@babel/compat-data@^7.22.9": version "7.22.9" resolved "https://registry.npmjs.org/@babel/compat-data/-/compat-data-7.22.9.tgz" @@ -155,7 +147,7 @@ "@babel/traverse" "^7.22.10" "@babel/types" "^7.22.10" -"@babel/highlight@^7.10.4", "@babel/highlight@^7.22.10": +"@babel/highlight@^7.10.4": version "7.22.10" resolved "https://registry.npmjs.org/@babel/highlight/-/highlight-7.22.10.tgz" integrity sha512-78aUtVcT7MUscr0K5mIEnkwxPE0MaxkR5RxRwuHaQ+JuU5AmTPhY+do2mdzVTnIJJpyBglql2pehuBIWHug+WQ== @@ -3091,11 +3083,6 @@ fs.realpath@^1.0.0: resolved "https://registry.npmjs.org/fs.realpath/-/fs.realpath-1.0.0.tgz" integrity sha512-OO0pH2lK6a0hZnAdau5ItzHPI6pUlvI7jMVnxUQRtw4owF2wk8lOSabtGDCTP4Ggrg2MbGnWO9X8K1t4+fGMDw== -fsevents@~2.3.2: - version "2.3.2" - resolved "https://registry.npmjs.org/fsevents/-/fsevents-2.3.2.tgz" - integrity sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA== - function-bind@^1.1.1: version "1.1.1" resolved "https://registry.npmjs.org/function-bind/-/function-bind-1.1.1.tgz" @@ -3343,7 +3330,14 @@ globals@^11.1.0: resolved "https://registry.npmjs.org/globals/-/globals-11.12.0.tgz" integrity sha512-WOBp/EEGUiIsJSp7wcv/y6MO+lV9UoncWqxuFfm8eBwzWNgyfBd6Gz+IeKQ9jCmyhoH99g15M3T+QaVHFjizVA== -globals@^13.6.0, globals@^13.9.0: +globals@^13.6.0: + version "13.21.0" + resolved "https://registry.npmjs.org/globals/-/globals-13.21.0.tgz" + integrity sha512-ybyme3s4yy/t/3s35bewwXKOf7cvzfreG2lH0lZl0JB7I4GxRP2ghxOK/Nb9EkRXdbBXZLfq/p/0W2JUONB/Gg== + dependencies: + type-fest "^0.20.2" + +globals@^13.9.0: version "13.21.0" resolved "https://registry.npmjs.org/globals/-/globals-13.21.0.tgz" integrity sha512-ybyme3s4yy/t/3s35bewwXKOf7cvzfreG2lH0lZl0JB7I4GxRP2ghxOK/Nb9EkRXdbBXZLfq/p/0W2JUONB/Gg== @@ -6960,12 +6954,7 @@ yargs-parser@^18.1.2: camelcase "^5.0.0" decamelize "^1.2.0" -yargs-parser@^20.2.2: - version "20.2.9" - resolved "https://registry.npmjs.org/yargs-parser/-/yargs-parser-20.2.9.tgz" - integrity sha512-y11nGElTIV+CT3Zv9t7VKl+Q3hTQoT9a1Qzezhhl6Rp21gJ/IVTW7Z3y9EWXhuUBC2Shnf+DX0antecpAwSP8w== - -yargs-parser@^20.2.3: +yargs-parser@^20.2.2, yargs-parser@^20.2.3: version "20.2.9" resolved "https://registry.npmjs.org/yargs-parser/-/yargs-parser-20.2.9.tgz" integrity sha512-y11nGElTIV+CT3Zv9t7VKl+Q3hTQoT9a1Qzezhhl6Rp21gJ/IVTW7Z3y9EWXhuUBC2Shnf+DX0antecpAwSP8w== From ef4842d6e35d29bc5c4e714b950f3e3066414ecd Mon Sep 17 00:00:00 2001 From: eli Date: Wed, 18 Feb 2026 10:08:38 -0600 Subject: [PATCH 2/9] Fix import sort order in retry tests Sort imports alphabetically to satisfy eslint sort-imports rule. Co-Authored-By: Claude Opus 4.5 --- src/tests/unit/retry.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tests/unit/retry.spec.ts b/src/tests/unit/retry.spec.ts index b4559240..b234d4df 100644 --- a/src/tests/unit/retry.spec.ts +++ b/src/tests/unit/retry.spec.ts @@ -2,14 +2,14 @@ import test from 'ava'; import { AxiosError, AxiosHeaders } from 'axios'; import { + calculateRetryDelay, DEFAULT_RETRY_CONFIG, defaultRetryCondition, - calculateRetryDelay, + IRetryConfig, + NON_RETRYABLE_STATUS_CODES, parseRetryAfter, resolveRetryConfig, RETRYABLE_STATUS_CODES, - NON_RETRYABLE_STATUS_CODES, - IRetryConfig, } from '../../utils/retry'; // Helper to create mock AxiosError From 12f1a98b144149b373ac4c6ec37a347815a68d74 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Tue, 23 Jun 2026 16:41:53 +0300 Subject: [PATCH 3/9] Address review comments: isolate PDP retries, make retries opt-in - Give the PDP enforcer its own dedicated axios instance so PDP-only POST retries never land on the shared REST client (no double-writes). - Default retries to opt-in: DEFAULT_RETRY_CONFIG.enabled=false; passing a retry config object opts in. Avoids silent latency change on upgrade. - parseRetryAfter rejects non-digit delta-seconds (e.g. "5abc"). - resolveRetryConfig returns a fresh copy and normalizes retryMethods to uppercase; freeze DEFAULT_RETRY_CONFIG. - Dedup POST in the PDP retryMethods via Set. - Use a real Symbol for the retry-count key. - Add test:unit script so unit specs run in CI; add interceptor tests and deterministic Retry-After HTTP-date tests. - README: document the opt-in default and PDP-vs-REST POST distinction. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 24 ++-- package.json | 1 + src/enforcement/enforcer.ts | 21 ++- src/tests/unit/retry-interceptor.spec.ts | 172 +++++++++++++++++++++++ src/tests/unit/retry.spec.ts | 63 ++++++--- src/utils/retry-interceptor.ts | 4 +- src/utils/retry.ts | 43 +++--- 7 files changed, 270 insertions(+), 58 deletions(-) create mode 100644 src/tests/unit/retry-interceptor.spec.ts diff --git a/README.md b/README.md index 8f74f8c3..b812fac0 100644 --- a/README.md +++ b/README.md @@ -19,22 +19,30 @@ npm install permitio ## Retry Configuration -The SDK includes built-in retry support for transient failures. By default, retries are **enabled** with the following settings: +The SDK includes built-in retry support for transient failures. Retries are **opt-in**: +they are **off** unless you pass a `retry` config (or `retry: { enabled: true }`). + +When enabled, the defaults are: - **3 retry attempts** with exponential backoff - Retries on network errors and status codes: `408`, `429`, `500`, `502`, `503`, `504` - Respects `Retry-After` headers for rate limiting (429) +> **Behavioral note** +> +> - Retries are opt-in — providing a `retry` config object turns them on; omitting it (or passing `retry: false`) leaves them off. +> - When enabled, PDP/OPA calls additionally retry `POST` because check operations are idempotent. The REST API does **not** retry `POST`, so non-idempotent writes are never repeated. + ### Customizing Retry Behavior ```typescript import { Permit } from 'permitio'; -// Use defaults (retry enabled) -const permit = new Permit({ token: 'your-api-key' }); +// Retries are off by default (opt-in) +const permitDefault = new Permit({ token: 'your-api-key' }); -// Custom retry configuration -const permit = new Permit({ +// Enable with custom retry configuration +const permitCustom = new Permit({ token: 'your-api-key', retry: { maxRetries: 5, @@ -44,14 +52,14 @@ const permit = new Permit({ }, }); -// Disable retry entirely -const permit = new Permit({ +// Explicitly disable retry +const permitNoRetry = new Permit({ token: 'your-api-key', retry: false, }); // Different config for PDP vs REST API -const permit = new Permit({ +const permitPdp = new Permit({ token: 'your-api-key', retry: { maxRetries: 3 }, pdpRetry: { maxRetries: 5 }, diff --git a/package.json b/package.json index c8efff71..a6865911 100644 --- a/package.json +++ b/package.json @@ -30,6 +30,7 @@ "fix:prettier": "prettier --config .prettierrc \"src/**/*.{ts,css,less,scss,js}\" --write", "fix:lint": "eslint src --ext .ts --fix", "test": "run-s test:*", + "test:unit": "run-s build && ava --verbose 'build/tests/unit/**/*.spec.js'", "test:integration": "run-s build && ava --verbose build/tests/endpoints/**/*.spec.js", "test:module-imports": "run-s build && ava --verbose build/tests/module-imports/**/*.spec.js", "test:e2e:rbac": "run-s build && ava --verbose build/tests/e2e/rbac.e2e.spec.js", diff --git a/src/enforcement/enforcer.ts b/src/enforcement/enforcer.ts index 410cd8d5..f5218daf 100644 --- a/src/enforcement/enforcer.ts +++ b/src/enforcement/enforcer.ts @@ -139,18 +139,13 @@ export class Enforcer implements IEnforcer { opaBaseUrl.set('port', '8181'); opaBaseUrl.set('pathname', `${opaBaseUrl.pathname}v1/data/permit/`); const version = process.env.npm_package_version ?? 'unknown'; - if (config.axiosInstance) { - this.client = config.axiosInstance; - this.client.defaults.baseURL = `${this.config.pdp}/`; - this.client.defaults.headers.common['X-Permit-SDK-Version'] = `node:${version}`; - } else { - this.client = axios.create({ - baseURL: `${this.config.pdp}/`, - headers: { - 'X-Permit-SDK-Version': `node:${version}`, - }, - }); - } + // PDP gets its own dedicated axios instance so PDP-only POST retries never + // apply to the shared REST API client (config.axiosInstance) — REST writes + // must never be retried. + this.client = axios.create({ + baseURL: `${this.config.pdp}/`, + headers: { 'X-Permit-SDK-Version': `node:${version}` }, + }); if (config.opaAxiosInstance) { this.opaClient = config.opaAxiosInstance; this.opaClient.defaults.baseURL = opaBaseUrl.toString(); @@ -173,7 +168,7 @@ export class Enforcer implements IEnforcer { // For PDP calls, enable POST retry since check operations are idempotent const pdpRetryWithPost = { ...pdpRetryConfig, - retryMethods: [...pdpRetryConfig.retryMethods, 'POST'], + retryMethods: [...new Set([...pdpRetryConfig.retryMethods, 'POST'])], }; AxiosRetryInterceptor.setupInterceptor(this.client, pdpRetryWithPost, this.logger, 'PDP'); AxiosRetryInterceptor.setupInterceptor(this.opaClient, pdpRetryWithPost, this.logger, 'OPA'); diff --git a/src/tests/unit/retry-interceptor.spec.ts b/src/tests/unit/retry-interceptor.spec.ts new file mode 100644 index 00000000..f79384ce --- /dev/null +++ b/src/tests/unit/retry-interceptor.spec.ts @@ -0,0 +1,172 @@ +import test from 'ava'; +import { AxiosError, AxiosHeaders, AxiosInstance } from 'axios'; +import { Logger } from 'pino'; + +import { DEFAULT_RETRY_CONFIG, IResolvedRetryConfig } from '../../utils/retry'; +import { AxiosRetryInterceptor } from '../../utils/retry-interceptor'; + +// Minimal pino-like logger stub +const warnings: string[] = []; +const loggerStub = { + warn: (msg: string): void => { + warnings.push(msg); + }, +} as unknown as Logger; + +// Fast, deterministic retry config built on top of the defaults. +function fastConfig(overrides: Partial = {}): IResolvedRetryConfig { + return { + ...DEFAULT_RETRY_CONFIG, + enabled: true, + retryDelay: 1, + maxDelay: 5, + backoffMultiplier: 1, + retryMethods: ['GET'], + ...overrides, + }; +} + +// Fake axios that captures the interceptor's rejection handler and models the +// real interceptor chain: a failed retry re-enters the rejection handler with +// the same (retry-count-carrying) request config, exactly like axios does. +class FakeAxios { + public requestCount = 0; + public rejectionHandler!: (error: AxiosError) => Promise; + + // When set, request() fails by feeding the error back through the rejection + // handler so retry-count limiting is genuinely exercised. Otherwise it + // resolves, simulating a successful retry. + private failError: AxiosError | undefined; + + public interceptors = { + response: { + use: (_onFulfilled: unknown, onRejected: (error: AxiosError) => Promise): void => { + this.rejectionHandler = onRejected; + }, + }, + }; + + public request = (requestConfig: unknown): Promise => { + this.requestCount += 1; + if (this.failError) { + // Re-drive the chain with the (mutated) config the interceptor passed in. + this.failError.config = requestConfig as AxiosError['config']; + return this.rejectionHandler(this.failError); + } + return Promise.resolve({ ok: true }); + }; + + alwaysFail(error: AxiosError): void { + this.failError = error; + } + + asInstance(): AxiosInstance { + return this as unknown as AxiosInstance; + } +} + +function createError(method: string, status?: number, retryAfter?: string): AxiosError { + const error = new Error('Request failed') as AxiosError; + error.isAxiosError = true; + error.config = { method, url: '/test', headers: new AxiosHeaders() }; + error.toJSON = () => ({}); + + if (status !== undefined) { + const headers: Record = {}; + if (retryAfter !== undefined) { + headers['retry-after'] = retryAfter; + } + error.response = { + status, + statusText: 'Error', + headers, + config: { headers: new AxiosHeaders() }, + data: {}, + }; + } + + return error; +} + +function setup(config: IResolvedRetryConfig): FakeAxios { + const fake = new FakeAxios(); + AxiosRetryInterceptor.setupInterceptor(fake.asInstance(), config, loggerStub, 'TEST'); + return fake; +} + +test('setupInterceptor does not register a handler when disabled', (t) => { + const fake = new FakeAxios(); + AxiosRetryInterceptor.setupInterceptor( + fake.asInstance(), + fastConfig({ enabled: false }), + loggerStub, + 'TEST', + ); + + t.is(fake.rejectionHandler, undefined); +}); + +test('retries a retryable GET until maxRetries then rejects with the original error', async (t) => { + const fake = setup(fastConfig({ maxRetries: 2 })); + const originalError = createError('GET', 503); + fake.alwaysFail(originalError); + + const rejected = await t.throwsAsync(() => fake.rejectionHandler(originalError)); + + t.is(rejected, originalError); + // maxRetries=2 -> exactly two retry requests are issued before giving up. + t.is(fake.requestCount, 2); +}); + +test('retries a retryable GET and resolves on success', async (t) => { + const fake = setup(fastConfig({ maxRetries: 3 })); + + const result = await fake.rejectionHandler(createError('GET', 503)); + + t.deepEqual(result, { ok: true }); + t.is(fake.requestCount, 1); +}); + +test('does not retry a method outside retryMethods', async (t) => { + const fake = setup(fastConfig({ retryMethods: ['GET'] })); + const error = createError('POST', 503); + + const rejected = await t.throwsAsync(() => fake.rejectionHandler(error)); + + t.is(rejected, error); + t.is(fake.requestCount, 0); +}); + +test('does not retry a non-retryable status code', async (t) => { + const fake = setup(fastConfig()); + const error = createError('GET', 400); + + const rejected = await t.throwsAsync(() => fake.rejectionHandler(error)); + + t.is(rejected, error); + t.is(fake.requestCount, 0); +}); + +test('rejects without retry when request config is missing', async (t) => { + const fake = setup(fastConfig()); + const error = createError('GET', 503); + // Simulate axios giving us no config to retry with. + (error as { config?: unknown }).config = undefined; + + const rejected = await t.throwsAsync(() => fake.rejectionHandler(error)); + + t.is(rejected, error); + t.is(fake.requestCount, 0); +}); + +test('honors a Retry-After header and still retries', async (t) => { + // "0" seconds keeps the delay immediate and the test deterministic; the + // delay-value math for Retry-After is covered in retry.spec. + const fake = setup(fastConfig({ respectRetryAfter: true, maxDelay: 50 })); + const error = createError('GET', 429, '0'); + + const result = await fake.rejectionHandler(error); + + t.deepEqual(result, { ok: true }); + t.is(fake.requestCount, 1); +}); diff --git a/src/tests/unit/retry.spec.ts b/src/tests/unit/retry.spec.ts index b234d4df..173ecdef 100644 --- a/src/tests/unit/retry.spec.ts +++ b/src/tests/unit/retry.spec.ts @@ -130,23 +130,35 @@ test('parseRetryAfter returns null for invalid values', (t) => { t.is(parseRetryAfter('abc123'), null); }); -test('parseRetryAfter parses HTTP-date format', (t) => { - // Use a future date - const futureDate = new Date(Date.now() + 10000); - const httpDate = futureDate.toUTCString(); - const result = parseRetryAfter(httpDate); +test('parseRetryAfter returns null for non digits-only delta-seconds', (t) => { + t.is(parseRetryAfter('5abc'), null); +}); - t.truthy(result); - // Should be approximately 10 seconds (10000ms), with some tolerance - t.true(result! > 9000 && result! < 11000); +test('parseRetryAfter parses HTTP-date format', (t) => { + const realNow = Date.now; + try { + const fixedNow = Date.UTC(2015, 9, 21, 7, 28, 0); // "Wed, 21 Oct 2015 07:28:00 GMT" + Date.now = () => fixedNow; + + // 10 seconds in the future relative to the stubbed clock + const httpDate = new Date(fixedNow + 10000).toUTCString(); + t.is(parseRetryAfter(httpDate), 10000); + } finally { + Date.now = realNow; + } }); test('parseRetryAfter returns 0 for past HTTP-date', (t) => { - const pastDate = new Date(Date.now() - 10000); - const httpDate = pastDate.toUTCString(); - const result = parseRetryAfter(httpDate); - - t.is(result, 0); + const realNow = Date.now; + try { + const fixedNow = Date.UTC(2015, 9, 21, 7, 28, 0); + Date.now = () => fixedNow; + + const httpDate = new Date(fixedNow - 10000).toUTCString(); + t.is(parseRetryAfter(httpDate), 0); + } finally { + Date.now = realNow; + } }); // ============================================ @@ -216,7 +228,7 @@ test('calculateRetryDelay ignores Retry-After when disabled', (t) => { test('resolveRetryConfig returns defaults when config is undefined', (t) => { const resolved = resolveRetryConfig(undefined); - t.true(resolved.enabled); + t.false(resolved.enabled); t.is(resolved.maxRetries, 3); t.is(resolved.retryDelay, 1000); t.is(resolved.backoffMultiplier, 2); @@ -225,6 +237,25 @@ test('resolveRetryConfig returns defaults when config is undefined', (t) => { t.deepEqual(resolved.retryMethods, ['GET', 'HEAD', 'OPTIONS', 'PUT', 'DELETE']); }); +test('resolveRetryConfig opts in when a config object is provided', (t) => { + const resolved = resolveRetryConfig({ maxRetries: 5 }); + + t.true(resolved.enabled); +}); + +test('resolveRetryConfig normalizes retry methods to uppercase', (t) => { + const resolved = resolveRetryConfig({ retryMethods: ['get', 'post'] }); + + t.deepEqual(resolved.retryMethods, ['GET', 'POST']); +}); + +test('resolveRetryConfig returns a copy that does not mutate the default', (t) => { + const resolved = resolveRetryConfig(undefined); + resolved.retryMethods.push('PATCH'); + + t.deepEqual(DEFAULT_RETRY_CONFIG.retryMethods, ['GET', 'HEAD', 'OPTIONS', 'PUT', 'DELETE']); +}); + test('resolveRetryConfig returns disabled config when false', (t) => { const resolved = resolveRetryConfig(false); @@ -286,7 +317,7 @@ test('resolveRetryConfig allows disabling via enabled: false', (t) => { // ============================================ test('DEFAULT_RETRY_CONFIG has expected default values', (t) => { - t.true(DEFAULT_RETRY_CONFIG.enabled); + t.false(DEFAULT_RETRY_CONFIG.enabled); t.is(DEFAULT_RETRY_CONFIG.maxRetries, 3); t.is(DEFAULT_RETRY_CONFIG.retryDelay, 1000); t.is(DEFAULT_RETRY_CONFIG.backoffMultiplier, 2); @@ -311,7 +342,7 @@ test('retry types are exported from SDK', async (t) => { test('Permit accepts retry configuration', async (t) => { const { Permit } = await import('../../index'); - // With retry enabled (default) + // With retry off (opt-in default) const permit1 = new Permit({ token: 'test' }); t.truthy(permit1); diff --git a/src/utils/retry-interceptor.ts b/src/utils/retry-interceptor.ts index 9d57ae6f..e6f1190a 100644 --- a/src/utils/retry-interceptor.ts +++ b/src/utils/retry-interceptor.ts @@ -3,8 +3,8 @@ import { Logger } from 'pino'; import { calculateRetryDelay, IResolvedRetryConfig } from './retry'; -// Symbols to track retry state on request config (avoids polluting the config object) -const RETRY_COUNT_KEY = '__permitRetryCount'; +// Symbol to track retry state on request config (avoids polluting the config object) +const RETRY_COUNT_KEY = Symbol('permitRetryCount'); /** * Extended request config with retry tracking diff --git a/src/utils/retry.ts b/src/utils/retry.ts index e02a88b2..c0c30b22 100644 --- a/src/utils/retry.ts +++ b/src/utils/retry.ts @@ -25,7 +25,9 @@ export type RetryConditionFn = (error: AxiosError) => boolean; */ export interface IRetryConfig { /** - * Whether retry is enabled. Defaults to true. + * Whether retry is enabled. Retries are opt-in: they are off unless a retry + * config object is supplied (which opts you in) or `enabled: true` is set + * explicitly. The default config is disabled. */ enabled?: boolean; @@ -97,10 +99,11 @@ export function defaultRetryCondition(error: AxiosError): boolean { } /** - * Default retry configuration values + * Default retry configuration values. + * Retries are opt-in, so the default config is disabled. */ -export const DEFAULT_RETRY_CONFIG: IResolvedRetryConfig = { - enabled: true, +export const DEFAULT_RETRY_CONFIG: IResolvedRetryConfig = Object.freeze({ + enabled: false, maxRetries: 3, retryDelay: 1000, backoffMultiplier: 2, @@ -108,7 +111,7 @@ export const DEFAULT_RETRY_CONFIG: IResolvedRetryConfig = { retryCondition: defaultRetryCondition, respectRetryAfter: true, retryMethods: DEFAULT_RETRY_METHODS, -}; +}); /** * Parse Retry-After header value @@ -118,10 +121,11 @@ export const DEFAULT_RETRY_CONFIG: IResolvedRetryConfig = { * @returns The delay in milliseconds, or null if parsing fails */ export function parseRetryAfter(value: string): number | null { - // Try parsing as seconds (integer) - const seconds = parseInt(value, 10); - if (!isNaN(seconds)) { - return seconds * 1000; + // Try parsing as delta-seconds. Per the Retry-After spec this must be + // digits only, so reject values like "5abc" that parseInt would accept. + const trimmed = value.trim(); + if (/^\d+$/.test(trimmed)) { + return Number(trimmed) * 1000; } // Try parsing as HTTP-date (e.g., "Wed, 21 Oct 2015 07:28:00 GMT") @@ -173,28 +177,29 @@ export function calculateRetryDelay( export function resolveRetryConfig( userConfig: IRetryConfig | false | undefined, ): IResolvedRetryConfig { - // If explicitly disabled, return disabled config - if (userConfig === false) { + // No retry config (undefined) or explicit `false` => retries are off. + // Return a fresh object with a cloned retryMethods array so callers can + // never mutate DEFAULT_RETRY_CONFIG or its array. + if (!userConfig) { return { ...DEFAULT_RETRY_CONFIG, enabled: false, + retryMethods: [...DEFAULT_RETRY_CONFIG.retryMethods], }; } - // If undefined or empty, use defaults - if (!userConfig) { - return DEFAULT_RETRY_CONFIG; - } - - // Merge user config with defaults + // Providing a retry config object opts you in, unless `enabled: false` is set. + const retryMethods = userConfig.retryMethods ?? DEFAULT_RETRY_CONFIG.retryMethods; return { - enabled: userConfig.enabled ?? DEFAULT_RETRY_CONFIG.enabled, + enabled: userConfig.enabled ?? true, maxRetries: userConfig.maxRetries ?? DEFAULT_RETRY_CONFIG.maxRetries, retryDelay: userConfig.retryDelay ?? DEFAULT_RETRY_CONFIG.retryDelay, backoffMultiplier: userConfig.backoffMultiplier ?? DEFAULT_RETRY_CONFIG.backoffMultiplier, maxDelay: userConfig.maxDelay ?? DEFAULT_RETRY_CONFIG.maxDelay, retryCondition: userConfig.retryCondition ?? DEFAULT_RETRY_CONFIG.retryCondition, respectRetryAfter: userConfig.respectRetryAfter ?? DEFAULT_RETRY_CONFIG.respectRetryAfter, - retryMethods: userConfig.retryMethods ?? DEFAULT_RETRY_CONFIG.retryMethods, + // Normalize to uppercase so lowercase user methods match the interceptor, + // which uppercases the request method. + retryMethods: retryMethods.map((m) => m.toUpperCase()), }; } From 3047aa3db0002ad51fab0e7c4ca61831e134af6a Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Tue, 23 Jun 2026 18:30:52 +0300 Subject: [PATCH 4/9] Fix retry-count key regression; add isolation tests and doc fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Revert the retry-count key from Symbol back to the string '__permitRetryCount'. axios's mergeConfig (run on every request() during a retry) copies only string-keyed props via Object.keys, so a Symbol key was dropped each retry — the counter reset to 0 and caused an infinite retry loop. Comment explains why a string is required. - Add isolation regression tests: REST and PDP use separate axios instances, and the REST instance does not retry POST while the PDP instance does (caught the Symbol infinite loop). - config.ts: correct the retry JSDoc to describe opt-in behavior; note that a custom axiosInstance applies to REST only (PDP/OPA use dedicated internal instances); fix the opaAxiosInstance JSDoc. - README: note that a custom axiosInstance applies to REST only. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 1 + src/config.ts | 14 +++-- src/tests/unit/retry-interceptor.spec.ts | 77 +++++++++++++++++++++++- src/utils/retry-interceptor.ts | 8 ++- 4 files changed, 93 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index b812fac0..9a6b5b47 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,7 @@ When enabled, the defaults are: > > - Retries are opt-in — providing a `retry` config object turns them on; omitting it (or passing `retry: false`) leaves them off. > - When enabled, PDP/OPA calls additionally retry `POST` because check operations are idempotent. The REST API does **not** retry `POST`, so non-idempotent writes are never repeated. +> - A custom `axiosInstance` applies to the REST API only; PDP and OPA calls use dedicated internal axios instances. ### Customizing Retry Behavior diff --git a/src/config.ts b/src/config.ts index 476c5e94..fa6b06d7 100644 --- a/src/config.ts +++ b/src/config.ts @@ -84,6 +84,10 @@ export interface IPermitConfig { * an optional custom axios instance, to control the behavior of the HTTP client * used to connect to the Permit REST API. * + * This instance applies to the REST API only. PDP and OPA calls use dedicated + * internal axios instances, so their retry policy can differ and non-idempotent + * POST writes on the shared REST client are never retried. + * * @see https://axios-http.com/docs/instance * @see https://axios-http.com/docs/req_config */ @@ -104,8 +108,9 @@ export interface IPermitConfig { */ factsSyncTimeoutPolicy: FactsSyncTimeoutPolicy | null; /** - * an optional custom axios instance for opa, to control the behavior of the HTTP client - * used to connect to the Permit REST API. + * an optional custom axios instance for OPA, to control the behavior of the HTTP + * client used to connect to OPA. This applies to OPA calls only and is separate + * from `axiosInstance` (REST API) and the dedicated internal PDP instance. * * @see https://axios-http.com/docs/instance * @see https://axios-http.com/docs/req_config @@ -114,8 +119,9 @@ export interface IPermitConfig { /** * Configuration for automatic retry of failed requests. - * Set to false to disable retries entirely. - * Defaults to enabled with sensible defaults (3 retries, exponential backoff). + * Retries are opt-in: when omitted or set to false, retries are disabled. + * Providing a config object enables them (3 retries with exponential backoff + * by default). * * @see {@link IRetryConfig} */ diff --git a/src/tests/unit/retry-interceptor.spec.ts b/src/tests/unit/retry-interceptor.spec.ts index f79384ce..98c19ae8 100644 --- a/src/tests/unit/retry-interceptor.spec.ts +++ b/src/tests/unit/retry-interceptor.spec.ts @@ -1,10 +1,17 @@ import test from 'ava'; -import { AxiosError, AxiosHeaders, AxiosInstance } from 'axios'; +import { AxiosError, AxiosHeaders, AxiosInstance, InternalAxiosRequestConfig } from 'axios'; import { Logger } from 'pino'; import { DEFAULT_RETRY_CONFIG, IResolvedRetryConfig } from '../../utils/retry'; import { AxiosRetryInterceptor } from '../../utils/retry-interceptor'; +// Reaches the private REST (config.axiosInstance) and PDP (enforcer.client) +// instances for the isolation regression tests below. +interface PermitInternals { + config: { axiosInstance: AxiosInstance }; + enforcer: { client: AxiosInstance }; +} + // Minimal pino-like logger stub const warnings: string[] = []; const loggerStub = { @@ -170,3 +177,71 @@ test('honors a Retry-After header and still retries', async (t) => { t.deepEqual(result, { ok: true }); t.is(fake.requestCount, 1); }); + +// ============================================ +// Isolation regression tests (CRITICAL + HIGH fixes) +// ============================================ + +// Installs a custom adapter that counts invocations and always rejects with a +// synthetic 503 carrying the live request config, so the retry interceptor can +// re-enter the chain via axiosInstance.request(...). +function installRejectingAdapter(instance: AxiosInstance): { count: () => number } { + let calls = 0; + instance.defaults.adapter = (config: InternalAxiosRequestConfig): Promise => { + calls += 1; + const error = new Error('synthetic failure') as AxiosError; + error.isAxiosError = true; + error.config = config; + error.toJSON = () => ({}); + error.response = { + status: 503, + statusText: 'Service Unavailable', + headers: {}, + config, + data: {}, + }; + return Promise.reject(error); + }; + return { count: () => calls }; +} + +test('REST and PDP use separate axios instances', async (t) => { + const { Permit } = await import('../../index'); + + const permit = new Permit({ + token: 'test', + retry: { maxRetries: 1 }, + pdpRetry: { maxRetries: 1 }, + }); + + const restInstance = (permit as unknown as PermitInternals).config.axiosInstance; + const pdpInstance = (permit as unknown as PermitInternals).enforcer.client; + + t.not(restInstance, pdpInstance); +}); + +test('REST instance does not retry POST while PDP instance does', async (t) => { + const { Permit } = await import('../../index'); + + // Tiny delays keep the retry near-instant and deterministic. + const tiny = { maxRetries: 1, retryDelay: 1, maxDelay: 5, backoffMultiplier: 1 }; + const permit = new Permit({ + token: 'test', + retry: tiny, + pdpRetry: tiny, + }); + + const restInstance = (permit as unknown as PermitInternals).config.axiosInstance; + const pdpInstance = (permit as unknown as PermitInternals).enforcer.client; + + const rest = installRejectingAdapter(restInstance); + const pdp = installRejectingAdapter(pdpInstance); + + await t.throwsAsync(() => restInstance.request({ method: 'POST', url: '/x' })); + await t.throwsAsync(() => pdpInstance.request({ method: 'POST', url: '/x' })); + + // REST never retries POST -> exactly one adapter call. + t.is(rest.count(), 1); + // PDP retries POST (maxRetries: 1) -> initial call + one retry = two. + t.is(pdp.count(), 2); +}); diff --git a/src/utils/retry-interceptor.ts b/src/utils/retry-interceptor.ts index e6f1190a..6ba7a975 100644 --- a/src/utils/retry-interceptor.ts +++ b/src/utils/retry-interceptor.ts @@ -3,8 +3,12 @@ import { Logger } from 'pino'; import { calculateRetryDelay, IResolvedRetryConfig } from './retry'; -// Symbol to track retry state on request config (avoids polluting the config object) -const RETRY_COUNT_KEY = Symbol('permitRetryCount'); +// String key used to track the retry count on the request config. A string +// (rather than a Symbol) is required: axios's mergeConfig — run on every +// axiosInstance.request() during a retry — only carries string-keyed +// properties, so a Symbol key would be dropped and the counter would reset to +// 0 each retry, causing an infinite retry loop. +const RETRY_COUNT_KEY = '__permitRetryCount'; /** * Extended request config with retry tracking From e1d3d305a7db50b4c3d50175855198e36d161db7 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Tue, 23 Jun 2026 22:14:12 +0300 Subject: [PATCH 5/9] Replace hand-rolled retry interceptor with axios-retry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the battle-tested axios-retry library for the retry mechanics (counting, re-dispatch, timeout reset, abort handling) instead of the custom response interceptor that previously shipped an infinite-loop bug. The public config surface (retry/pdpRetry, IRetryConfig) and our delay/condition policy (resolveRetryConfig, calculateRetryDelay, parseRetryAfter) are unchanged — retry-interceptor.ts is now a thin adapter translating the resolved config into axiosRetry() options: - retries = maxRetries; retryCondition = per-instance method filter + our network/status condition (REST excludes POST, PDP includes it); retryDelay reuses calculateRetryDelay (1-based retryCount -> 0-based attempt); shouldResetTimeout = true; onRetry preserves the log line. - Add axios-retry ^4.5.0 (1 transitive dep, is-retry-allowed). - Rewrite the interceptor tests as behavior tests through real axios instances + a rejecting adapter (isolation, POST distinction, retry count, non-retryable status, disabled path, retryCount mapping). Co-Authored-By: Claude Opus 4.8 (1M context) --- package.json | 1 + src/tests/unit/retry-interceptor.spec.ts | 291 +++++++---------------- src/utils/retry-interceptor.ts | 89 +++---- yarn.lock | 12 + 4 files changed, 128 insertions(+), 265 deletions(-) diff --git a/package.json b/package.json index a6865911..f5ce238c 100644 --- a/package.json +++ b/package.json @@ -60,6 +60,7 @@ "dependencies": { "@bitauth/libauth": "^1.17.1", "axios": "^1.7.4", + "axios-retry": "^4.5.0", "lodash": "^4.17.21", "path-to-regexp": "^6.2.1", "pino": "8.11.0", diff --git a/src/tests/unit/retry-interceptor.spec.ts b/src/tests/unit/retry-interceptor.spec.ts index 98c19ae8..eb3c7a61 100644 --- a/src/tests/unit/retry-interceptor.spec.ts +++ b/src/tests/unit/retry-interceptor.spec.ts @@ -1,247 +1,130 @@ import test from 'ava'; -import { AxiosError, AxiosHeaders, AxiosInstance, InternalAxiosRequestConfig } from 'axios'; -import { Logger } from 'pino'; - -import { DEFAULT_RETRY_CONFIG, IResolvedRetryConfig } from '../../utils/retry'; -import { AxiosRetryInterceptor } from '../../utils/retry-interceptor'; +import { AxiosError, AxiosInstance, InternalAxiosRequestConfig } from 'axios'; // Reaches the private REST (config.axiosInstance) and PDP (enforcer.client) -// instances for the isolation regression tests below. +// instances for the behavior tests below. interface PermitInternals { config: { axiosInstance: AxiosInstance }; enforcer: { client: AxiosInstance }; } -// Minimal pino-like logger stub -const warnings: string[] = []; -const loggerStub = { - warn: (msg: string): void => { - warnings.push(msg); - }, -} as unknown as Logger; - -// Fast, deterministic retry config built on top of the defaults. -function fastConfig(overrides: Partial = {}): IResolvedRetryConfig { - return { - ...DEFAULT_RETRY_CONFIG, - enabled: true, - retryDelay: 1, - maxDelay: 5, - backoffMultiplier: 1, - retryMethods: ['GET'], - ...overrides, - }; -} - -// Fake axios that captures the interceptor's rejection handler and models the -// real interceptor chain: a failed retry re-enters the rejection handler with -// the same (retry-count-carrying) request config, exactly like axios does. -class FakeAxios { - public requestCount = 0; - public rejectionHandler!: (error: AxiosError) => Promise; - - // When set, request() fails by feeding the error back through the rejection - // handler so retry-count limiting is genuinely exercised. Otherwise it - // resolves, simulating a successful retry. - private failError: AxiosError | undefined; - - public interceptors = { - response: { - use: (_onFulfilled: unknown, onRejected: (error: AxiosError) => Promise): void => { - this.rejectionHandler = onRejected; - }, - }, - }; - - public request = (requestConfig: unknown): Promise => { - this.requestCount += 1; - if (this.failError) { - // Re-drive the chain with the (mutated) config the interceptor passed in. - this.failError.config = requestConfig as AxiosError['config']; - return this.rejectionHandler(this.failError); - } - return Promise.resolve({ ok: true }); - }; +// Tiny delays keep every retry near-instant and deterministic (no real waits). +const tiny = { maxRetries: 2, retryDelay: 1, maxDelay: 5, backoffMultiplier: 1 }; - alwaysFail(error: AxiosError): void { - this.failError = error; - } - - asInstance(): AxiosInstance { - return this as unknown as AxiosInstance; - } -} - -function createError(method: string, status?: number, retryAfter?: string): AxiosError { - const error = new Error('Request failed') as AxiosError; - error.isAxiosError = true; - error.config = { method, url: '/test', headers: new AxiosHeaders() }; - error.toJSON = () => ({}); - - if (status !== undefined) { - const headers: Record = {}; - if (retryAfter !== undefined) { - headers['retry-after'] = retryAfter; - } +// Installs a custom adapter that counts invocations and always rejects with a +// synthetic AxiosError carrying the live request config, so axios-retry can +// re-dispatch the request and we can observe how many times it ran. +function installRejectingAdapter(instance: AxiosInstance, status = 503): { count: () => number } { + let calls = 0; + instance.defaults.adapter = (config: InternalAxiosRequestConfig): Promise => { + calls += 1; + const error = new Error('synthetic failure') as AxiosError; + error.isAxiosError = true; + error.config = config; + error.toJSON = () => ({}); error.response = { status, statusText: 'Error', - headers, - config: { headers: new AxiosHeaders() }, + headers: {}, + config, data: {}, }; - } - - return error; + return Promise.reject(error); + }; + return { count: () => calls }; } -function setup(config: IResolvedRetryConfig): FakeAxios { - const fake = new FakeAxios(); - AxiosRetryInterceptor.setupInterceptor(fake.asInstance(), config, loggerStub, 'TEST'); - return fake; +async function newPermit(overrides: Record): Promise { + const { Permit } = await import('../../index'); + return new Permit({ token: 'test', ...overrides }) as unknown as PermitInternals; } -test('setupInterceptor does not register a handler when disabled', (t) => { - const fake = new FakeAxios(); - AxiosRetryInterceptor.setupInterceptor( - fake.asInstance(), - fastConfig({ enabled: false }), - loggerStub, - 'TEST', - ); - - t.is(fake.rejectionHandler, undefined); -}); - -test('retries a retryable GET until maxRetries then rejects with the original error', async (t) => { - const fake = setup(fastConfig({ maxRetries: 2 })); - const originalError = createError('GET', 503); - fake.alwaysFail(originalError); - - const rejected = await t.throwsAsync(() => fake.rejectionHandler(originalError)); +test('REST and PDP use separate axios instances', async (t) => { + const permit = await newPermit({ retry: { maxRetries: 1 }, pdpRetry: { maxRetries: 1 } }); - t.is(rejected, originalError); - // maxRetries=2 -> exactly two retry requests are issued before giving up. - t.is(fake.requestCount, 2); + t.not(permit.config.axiosInstance, permit.enforcer.client); }); -test('retries a retryable GET and resolves on success', async (t) => { - const fake = setup(fastConfig({ maxRetries: 3 })); - - const result = await fake.rejectionHandler(createError('GET', 503)); - - t.deepEqual(result, { ok: true }); - t.is(fake.requestCount, 1); -}); +test('REST instance does not retry POST while PDP instance does', async (t) => { + const permit = await newPermit({ retry: tiny, pdpRetry: tiny }); -test('does not retry a method outside retryMethods', async (t) => { - const fake = setup(fastConfig({ retryMethods: ['GET'] })); - const error = createError('POST', 503); + const rest = installRejectingAdapter(permit.config.axiosInstance); + const pdp = installRejectingAdapter(permit.enforcer.client); - const rejected = await t.throwsAsync(() => fake.rejectionHandler(error)); + await t.throwsAsync(() => permit.config.axiosInstance.request({ method: 'POST', url: '/x' })); + await t.throwsAsync(() => permit.enforcer.client.request({ method: 'POST', url: '/x' })); - t.is(rejected, error); - t.is(fake.requestCount, 0); + // REST never retries POST -> exactly one adapter call. + t.is(rest.count(), 1); + // PDP retries POST (maxRetries: 2) -> initial call + two retries = three. + t.is(pdp.count(), 3); }); -test('does not retry a non-retryable status code', async (t) => { - const fake = setup(fastConfig()); - const error = createError('GET', 400); +test('a retryable GET retries up to maxRetries then rejects', async (t) => { + const permit = await newPermit({ retry: tiny }); + const rest = installRejectingAdapter(permit.config.axiosInstance); - const rejected = await t.throwsAsync(() => fake.rejectionHandler(error)); + await t.throwsAsync(() => permit.config.axiosInstance.request({ method: 'GET', url: '/x' })); - t.is(rejected, error); - t.is(fake.requestCount, 0); + // maxRetries: 2 -> initial call + two retries = three total. + t.is(rest.count(), 3); }); -test('rejects without retry when request config is missing', async (t) => { - const fake = setup(fastConfig()); - const error = createError('GET', 503); - // Simulate axios giving us no config to retry with. - (error as { config?: unknown }).config = undefined; +test('a non-retryable status (400) is not retried', async (t) => { + const permit = await newPermit({ retry: tiny }); + const rest = installRejectingAdapter(permit.config.axiosInstance, 400); - const rejected = await t.throwsAsync(() => fake.rejectionHandler(error)); + await t.throwsAsync(() => permit.config.axiosInstance.request({ method: 'GET', url: '/x' })); - t.is(rejected, error); - t.is(fake.requestCount, 0); + t.is(rest.count(), 1); }); -test('honors a Retry-After header and still retries', async (t) => { - // "0" seconds keeps the delay immediate and the test deterministic; the - // delay-value math for Retry-After is covered in retry.spec. - const fake = setup(fastConfig({ respectRetryAfter: true, maxDelay: 50 })); - const error = createError('GET', 429, '0'); +test('a disallowed method on the REST instance is not retried', async (t) => { + const permit = await newPermit({ retry: tiny }); + const rest = installRejectingAdapter(permit.config.axiosInstance); - const result = await fake.rejectionHandler(error); + // PUT is in DEFAULT_RETRY_METHODS but POST is not, so POST must not retry. + await t.throwsAsync(() => permit.config.axiosInstance.request({ method: 'POST', url: '/x' })); - t.deepEqual(result, { ok: true }); - t.is(fake.requestCount, 1); + t.is(rest.count(), 1); }); -// ============================================ -// Isolation regression tests (CRITICAL + HIGH fixes) -// ============================================ +test('disabled retry (retry: false) installs no retry', async (t) => { + const permit = await newPermit({ retry: false }); + const rest = installRejectingAdapter(permit.config.axiosInstance); -// Installs a custom adapter that counts invocations and always rejects with a -// synthetic 503 carrying the live request config, so the retry interceptor can -// re-enter the chain via axiosInstance.request(...). -function installRejectingAdapter(instance: AxiosInstance): { count: () => number } { - let calls = 0; - instance.defaults.adapter = (config: InternalAxiosRequestConfig): Promise => { - calls += 1; - const error = new Error('synthetic failure') as AxiosError; - error.isAxiosError = true; - error.config = config; - error.toJSON = () => ({}); - error.response = { - status: 503, - statusText: 'Service Unavailable', - headers: {}, - config, - data: {}, - }; - return Promise.reject(error); - }; - return { count: () => calls }; -} - -test('REST and PDP use separate axios instances', async (t) => { - const { Permit } = await import('../../index'); + await t.throwsAsync(() => permit.config.axiosInstance.request({ method: 'GET', url: '/x' })); - const permit = new Permit({ - token: 'test', - retry: { maxRetries: 1 }, - pdpRetry: { maxRetries: 1 }, - }); - - const restInstance = (permit as unknown as PermitInternals).config.axiosInstance; - const pdpInstance = (permit as unknown as PermitInternals).enforcer.client; - - t.not(restInstance, pdpInstance); + t.is(rest.count(), 1); }); -test('REST instance does not retry POST while PDP instance does', async (t) => { - const { Permit } = await import('../../index'); - - // Tiny delays keep the retry near-instant and deterministic. - const tiny = { maxRetries: 1, retryDelay: 1, maxDelay: 5, backoffMultiplier: 1 }; - const permit = new Permit({ - token: 'test', - retry: tiny, - pdpRetry: tiny, - }); - - const restInstance = (permit as unknown as PermitInternals).config.axiosInstance; - const pdpInstance = (permit as unknown as PermitInternals).enforcer.client; - - const rest = installRejectingAdapter(restInstance); - const pdp = installRejectingAdapter(pdpInstance); - - await t.throwsAsync(() => restInstance.request({ method: 'POST', url: '/x' })); - await t.throwsAsync(() => pdpInstance.request({ method: 'POST', url: '/x' })); - - // REST never retries POST -> exactly one adapter call. - t.is(rest.count(), 1); - // PDP retries POST (maxRetries: 1) -> initial call + one retry = two. - t.is(pdp.count(), 2); +test.serial('maps axios-retry retryCount to our 0-based attempt number', async (t) => { + // axios-retry passes a 1-based retryCount; the interceptor must subtract one + // so the first retry uses attempt 0. With jitter removed, attempt 0 -> 30ms + // and attempt 1 (the off-by-one bug) -> 90ms. We capture the delay axios-retry + // schedules via setTimeout instead of measuring wall-clock time, so the check + // is deterministic and concurrency-safe. + const realRandom = Math.random; + const realSetTimeout = global.setTimeout; + const scheduledDelays: number[] = []; + Math.random = () => 0; // strip jitter + // Capture the delay, then fire immediately so the retry still proceeds. + global.setTimeout = ((fn: () => void, delay?: number): ReturnType => { + scheduledDelays.push(delay ?? 0); + return realSetTimeout(fn, 0); + }) as typeof setTimeout; + try { + const permit = await newPermit({ + retry: { maxRetries: 1, retryDelay: 30, backoffMultiplier: 3, maxDelay: 10000 }, + }); + installRejectingAdapter(permit.config.axiosInstance); + + await t.throwsAsync(() => permit.config.axiosInstance.request({ method: 'GET', url: '/x' })); + + t.true(scheduledDelays.includes(30), `expected a 30ms delay, got ${scheduledDelays.join(',')}`); + t.false(scheduledDelays.includes(90), 'attempt-1 (off-by-one) delay must not be used'); + } finally { + Math.random = realRandom; + global.setTimeout = realSetTimeout; + } }); diff --git a/src/utils/retry-interceptor.ts b/src/utils/retry-interceptor.ts index 6ba7a975..b18cf0f8 100644 --- a/src/utils/retry-interceptor.ts +++ b/src/utils/retry-interceptor.ts @@ -1,28 +1,16 @@ -import { AxiosError, AxiosInstance, InternalAxiosRequestConfig } from 'axios'; +import { AxiosError, AxiosInstance } from 'axios'; +import axiosRetry from 'axios-retry'; import { Logger } from 'pino'; import { calculateRetryDelay, IResolvedRetryConfig } from './retry'; -// String key used to track the retry count on the request config. A string -// (rather than a Symbol) is required: axios's mergeConfig — run on every -// axiosInstance.request() during a retry — only carries string-keyed -// properties, so a Symbol key would be dropped and the counter would reset to -// 0 each retry, causing an infinite retry loop. -const RETRY_COUNT_KEY = '__permitRetryCount'; - -/** - * Extended request config with retry tracking - */ -interface RetryableRequestConfig extends InternalAxiosRequestConfig { - [RETRY_COUNT_KEY]?: number; -} - /** - * Axios interceptor that implements retry logic with exponential backoff + * Installs retry behavior on an axios instance using the axios-retry library, + * driven by our resolved retry configuration (delay/condition policy). */ export class AxiosRetryInterceptor { /** - * Setup retry interceptor on an axios instance + * Setup retry on an axios instance. * * @param axiosInstance - The axios instance to add retry capability to * @param config - Resolved retry configuration @@ -39,57 +27,36 @@ export class AxiosRetryInterceptor { return; } - axiosInstance.interceptors.response.use( - // Success handler - pass through unchanged - (response) => response, - - // Error handler - implement retry logic - async (error: AxiosError) => { - const originalRequest = error.config as RetryableRequestConfig | undefined; + axiosRetry(axiosInstance, { + retries: config.maxRetries, + shouldResetTimeout: true, - // If no request config, cannot retry - if (!originalRequest) { - return Promise.reject(error); + // Preserve our policy: per-instance method filtering (REST excludes POST, + // PDP includes it) plus our error condition (network + retryable status). + retryCondition: (error: AxiosError): boolean => { + const method = (error.config?.method ?? 'GET').toUpperCase(); + if (!config.retryMethods.includes(method)) { + return false; } + return config.retryCondition(error); + }, - // Initialize or get current retry count - const currentRetryCount = originalRequest[RETRY_COUNT_KEY] ?? 0; - - // Check if we should retry this request - const method = (originalRequest.method ?? 'GET').toUpperCase(); - const shouldRetryMethod = config.retryMethods.includes(method); - const shouldRetryError = config.retryCondition(error); - const hasRetriesLeft = currentRetryCount < config.maxRetries; - - // If any condition fails, reject with original error - if (!shouldRetryMethod || !shouldRetryError || !hasRetriesLeft) { - return Promise.reject(error); - } - - // Increment retry count for next attempt - originalRequest[RETRY_COUNT_KEY] = currentRetryCount + 1; - - // Calculate delay before retry + // axios-retry's retryCount is 1-based (1 on the first retry); our + // calculateRetryDelay expects a 0-based attempt number, so subtract one. + retryDelay: (retryCount: number, error: AxiosError): number => { const retryAfterHeader = error.response?.headers?.['retry-after'] as string | undefined; - const delay = calculateRetryDelay(currentRetryCount, config, retryAfterHeader); + return calculateRetryDelay(retryCount - 1, config, retryAfterHeader); + }, - // Log retry attempt - const statusInfo = error.response?.status ?? 'network error'; - const url = originalRequest.url ?? 'unknown'; + onRetry: (retryCount: number, error: AxiosError): void => { + const status = error.response?.status ?? 'network error'; + const method = (error.config?.method ?? 'GET').toUpperCase(); + const url = error.config?.url ?? 'unknown'; logger.warn( - `[${clientName}] Request failed (${statusInfo}), ` + - `retrying in ${Math.round(delay)}ms (attempt ${currentRetryCount + 1}/${ - config.maxRetries - }): ` + - `${method} ${url}`, + `[${clientName}] Request failed (${status}), ` + + `retrying (attempt ${retryCount}/${config.maxRetries}): ${method} ${url}`, ); - - // Wait for the calculated delay - await new Promise((resolve) => setTimeout(resolve, delay)); - - // Retry the request - return axiosInstance.request(originalRequest); }, - ); + }); } } diff --git a/yarn.lock b/yarn.lock index f1da6aa1..4d08fd79 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1274,6 +1274,13 @@ axios@0.27.2: follow-redirects "^1.14.9" form-data "^4.0.0" +axios-retry@^4.5.0: + version "4.5.0" + resolved "https://registry.npmjs.org/axios-retry/-/axios-retry-4.5.0.tgz" + integrity sha512-aR99oXhpEDGo0UuAlYcn2iGRds30k366Zfa05XWScR9QaQD4JYiP3/1Qt1u7YlefUOK+cn0CcwoL1oefavQUlQ== + dependencies: + is-retry-allowed "^2.2.0" + balanced-match@^1.0.0: version "1.0.2" resolved "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz" @@ -3848,6 +3855,11 @@ is-regex@^1.1.4: call-bind "^1.0.2" has-tostringtag "^1.0.0" +is-retry-allowed@^2.2.0: + version "2.2.0" + resolved "https://registry.npmjs.org/is-retry-allowed/-/is-retry-allowed-2.2.0.tgz" + integrity sha512-XVm7LOeLpTW4jV19QSH38vkswxoLud8sQ57YwJVTPWdiaI9I8keEhGFpBlslyVsgdQy4Opg8QOLb8YRgsyZiQg== + is-shared-array-buffer@^1.0.2: version "1.0.2" resolved "https://registry.npmjs.org/is-shared-array-buffer/-/is-shared-array-buffer-1.0.2.tgz" From e12e28b0ca30a63b43431364849c3b706f469908 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Tue, 23 Jun 2026 22:23:30 +0300 Subject: [PATCH 6/9] Add success-after-retry and Retry-After end-to-end tests Close the coverage gap from the axios-retry migration review: - a retryable GET that fails once then succeeds resolves (proves a retry actually recovers, not just exhausts); - a 429 with `retry-after: 2` schedules a 2000ms delay, proving the Retry-After header flows through calculateRetryDelay via the axios-retry retryDelay callback. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/tests/unit/retry-interceptor.spec.ts | 92 ++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/src/tests/unit/retry-interceptor.spec.ts b/src/tests/unit/retry-interceptor.spec.ts index eb3c7a61..9a8776ef 100644 --- a/src/tests/unit/retry-interceptor.spec.ts +++ b/src/tests/unit/retry-interceptor.spec.ts @@ -34,6 +34,35 @@ function installRejectingAdapter(instance: AxiosInstance, status = 503): { count return { count: () => calls }; } +// Installs an adapter that rejects with a synthetic 503 for the first +// `failTimes` calls, then resolves with a success response, so we can prove a +// retry actually recovers. +function installRejectThenResolveAdapter( + instance: AxiosInstance, + failTimes: number, +): { count: () => number } { + let calls = 0; + instance.defaults.adapter = (config: InternalAxiosRequestConfig) => { + calls += 1; + if (calls <= failTimes) { + const error = new Error('synthetic failure') as AxiosError; + error.isAxiosError = true; + error.config = config; + error.toJSON = () => ({}); + error.response = { status: 503, statusText: 'Error', headers: {}, config, data: {} }; + return Promise.reject(error); + } + return Promise.resolve({ + status: 200, + statusText: 'OK', + headers: {}, + config, + data: { ok: true }, + }); + }; + return { count: () => calls }; +} + async function newPermit(overrides: Record): Promise { const { Permit } = await import('../../index'); return new Permit({ token: 'test', ...overrides }) as unknown as PermitInternals; @@ -128,3 +157,66 @@ test.serial('maps axios-retry retryCount to our 0-based attempt number', async ( global.setTimeout = realSetTimeout; } }); + +test('a retry recovers: GET succeeds after one failed attempt', async (t) => { + const permit = await newPermit({ + retry: { maxRetries: 2, retryDelay: 1, maxDelay: 5, backoffMultiplier: 1 }, + }); + const rest = installRejectThenResolveAdapter(permit.config.axiosInstance, 1); + + const response = await permit.config.axiosInstance.request({ method: 'GET', url: '/x' }); + + // Initial failure + one retry that succeeds. + t.is(rest.count(), 2); + t.is(response.status, 200); + t.deepEqual(response.data, { ok: true }); +}); + +test.serial('Retry-After header drives the retry delay end-to-end', async (t) => { + // 429 with `retry-after: 2` (seconds) must produce a 2000ms scheduled delay, + // which exceeds the 10ms backoff and is under maxDelay, proving Retry-After + // flows through calculateRetryDelay via our retryDelay callback. The + // Retry-After branch returns the exact value with no jitter, so we only need + // to capture the setTimeout delay (nothing is actually awaited). + const realSetTimeout = global.setTimeout; + const scheduledDelays: number[] = []; + global.setTimeout = ((fn: () => void, delay?: number): ReturnType => { + scheduledDelays.push(delay ?? 0); + return realSetTimeout(fn, 0); + }) as typeof setTimeout; + try { + const permit = await newPermit({ + retry: { maxRetries: 1, retryDelay: 10, maxDelay: 30000, backoffMultiplier: 1 }, + }); + + let calls = 0; + permit.config.axiosInstance.defaults.adapter = ( + config: InternalAxiosRequestConfig, + ): Promise => { + calls += 1; + const error = new Error('rate limited') as AxiosError; + error.isAxiosError = true; + error.config = config; + error.toJSON = () => ({}); + error.response = { + status: 429, + statusText: 'Too Many Requests', + headers: { 'retry-after': '2' }, + config, + data: {}, + }; + return Promise.reject(error); + }; + + await t.throwsAsync(() => permit.config.axiosInstance.request({ method: 'GET', url: '/x' })); + + // 429 is retryable and GET is allowed, so it retried once. + t.is(calls, 2); + t.true( + scheduledDelays.includes(2000), + `expected a 2000ms delay, got ${scheduledDelays.join(',')}`, + ); + } finally { + global.setTimeout = realSetTimeout; + } +}); From ee95be9512b3d435fa46df104b179255a05388f8 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Tue, 23 Jun 2026 23:16:59 +0300 Subject: [PATCH 7/9] Remove dead retry exports - Delete NON_RETRYABLE_STATUS_CODES: it was defined and publicly re-exported but never used in any retry logic (the condition uses RETRYABLE_STATUS_CODES). Drop its definition, the index.ts re-export, and the tests that only asserted its value. - Make DEFAULT_RETRY_METHODS module-private; it's only used in-module by DEFAULT_RETRY_CONFIG and imported nowhere. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/index.ts | 7 +------ src/tests/unit/retry.spec.ts | 6 ------ src/utils/retry.ts | 7 +------ 3 files changed, 2 insertions(+), 18 deletions(-) diff --git a/src/index.ts b/src/index.ts index 00dd70a8..9f34cf49 100644 --- a/src/index.ts +++ b/src/index.ts @@ -27,12 +27,7 @@ export { PermitConnectionError, PermitError, PermitPDPStatusError } from './enfo export { Context, ContextTransform } from './utils/context'; export { ApiContext, PermitContextError, ApiKeyLevel } from './api/context'; export { PermitApiError } from './api/base'; -export { - IRetryConfig, - RetryConditionFn, - RETRYABLE_STATUS_CODES, - NON_RETRYABLE_STATUS_CODES, -} from './utils/retry'; +export { IRetryConfig, RetryConditionFn, RETRYABLE_STATUS_CODES } from './utils/retry'; export interface IPermitClient extends IEnforcer { /** diff --git a/src/tests/unit/retry.spec.ts b/src/tests/unit/retry.spec.ts index 173ecdef..10d48a96 100644 --- a/src/tests/unit/retry.spec.ts +++ b/src/tests/unit/retry.spec.ts @@ -6,7 +6,6 @@ import { DEFAULT_RETRY_CONFIG, defaultRetryCondition, IRetryConfig, - NON_RETRYABLE_STATUS_CODES, parseRetryAfter, resolveRetryConfig, RETRYABLE_STATUS_CODES, @@ -40,10 +39,6 @@ test('RETRYABLE_STATUS_CODES contains expected status codes', (t) => { t.deepEqual(RETRYABLE_STATUS_CODES, [408, 429, 500, 502, 503, 504]); }); -test('NON_RETRYABLE_STATUS_CODES contains expected status codes', (t) => { - t.deepEqual(NON_RETRYABLE_STATUS_CODES, [400, 401, 403, 404, 422]); -}); - // ============================================ // Tests for defaultRetryCondition // ============================================ @@ -335,7 +330,6 @@ test('retry types are exported from SDK', async (t) => { const sdk = await import('../../index'); t.truthy(sdk.RETRYABLE_STATUS_CODES); - t.truthy(sdk.NON_RETRYABLE_STATUS_CODES); t.deepEqual(sdk.RETRYABLE_STATUS_CODES, [408, 429, 500, 502, 503, 504]); }); diff --git a/src/utils/retry.ts b/src/utils/retry.ts index c0c30b22..28ed4725 100644 --- a/src/utils/retry.ts +++ b/src/utils/retry.ts @@ -5,15 +5,10 @@ import { AxiosError } from 'axios'; */ export const RETRYABLE_STATUS_CODES = [408, 429, 500, 502, 503, 504]; -/** - * HTTP status codes that should NOT trigger a retry - */ -export const NON_RETRYABLE_STATUS_CODES = [400, 401, 403, 404, 422]; - /** * Default HTTP methods that are safe to retry */ -export const DEFAULT_RETRY_METHODS = ['GET', 'HEAD', 'OPTIONS', 'PUT', 'DELETE']; +const DEFAULT_RETRY_METHODS = ['GET', 'HEAD', 'OPTIONS', 'PUT', 'DELETE']; /** * Function type for custom retry condition evaluation From c2235c6af36ef3f71684a597b69a1c3d68c56650 Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Tue, 23 Jun 2026 23:54:17 +0300 Subject: [PATCH 8/9] Address Copilot review: REST never retries POST, doc/test hardening - index.ts: strip POST from the REST retryMethods regardless of user config, so the REST client never retries non-idempotent writes (hard guarantee, symmetric with the enforcer adding POST for PDP/OPA). - retry.ts: clarify that maxRetries is the number of retries after the initial request (default 3 = up to 4 attempts); freeze the DEFAULT_RETRY_METHODS array so the exported DEFAULT_RETRY_CONFIG can't be mutated by consumers (Object.freeze was shallow). - retry-interceptor.ts: reword the retry log to "retry N/M" to avoid implying total attempts. - README: clarify "3 retries (up to 4 total attempts)". - tests: mark the two Date.now()-stubbing HTTP-date tests serial to avoid concurrent races; add tests for the frozen default and for REST refusing to retry POST even when the user adds it to retryMethods. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 4 +++- src/index.ts | 11 +++++++++-- src/tests/unit/retry-interceptor.spec.ts | 11 +++++++++++ src/tests/unit/retry.spec.ts | 8 ++++++-- src/utils/retry-interceptor.ts | 2 +- src/utils/retry.ts | 19 +++++++++++++++---- 6 files changed, 45 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 9a6b5b47..dc9836e7 100644 --- a/README.md +++ b/README.md @@ -24,10 +24,12 @@ they are **off** unless you pass a `retry` config (or `retry: { enabled: true }` When enabled, the defaults are: -- **3 retry attempts** with exponential backoff +- **3 retries** (up to 4 total attempts) with exponential backoff - Retries on network errors and status codes: `408`, `429`, `500`, `502`, `503`, `504` - Respects `Retry-After` headers for rate limiting (429) +`maxRetries` is the number of retries _after_ the initial request, so the default of `3` means up to 4 total requests. + > **Behavioral note** > > - Retries are opt-in — providing a `retry` config object turns them on; omitting it (or passing `retry: false`) leaves them off. diff --git a/src/index.ts b/src/index.ts index 9f34cf49..bb23d80f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -143,12 +143,19 @@ export class Permit implements IPermitClient { this.logger = LoggerFactory.createLogger(this.config); AxiosLoggingInterceptor.setupInterceptor(this.config.axiosInstance, this.logger); - // Setup retry interceptor for REST API calls + // Setup retry interceptor for REST API calls. + // Strip POST from the REST retryMethods regardless of user config: REST + // writes are non-idempotent and must never be repeated. (This is symmetric + // with the enforcer, which ADDS POST for the idempotent PDP/OPA check calls.) const resolvedRetryConfig = resolveRetryConfig(this.config.retry); if (resolvedRetryConfig.enabled) { + const restRetryConfig = { + ...resolvedRetryConfig, + retryMethods: resolvedRetryConfig.retryMethods.filter((m) => m !== 'POST'), + }; AxiosRetryInterceptor.setupInterceptor( this.config.axiosInstance, - resolvedRetryConfig, + restRetryConfig, this.logger, 'API', ); diff --git a/src/tests/unit/retry-interceptor.spec.ts b/src/tests/unit/retry-interceptor.spec.ts index 9a8776ef..5fd1e099 100644 --- a/src/tests/unit/retry-interceptor.spec.ts +++ b/src/tests/unit/retry-interceptor.spec.ts @@ -127,6 +127,17 @@ test('disabled retry (retry: false) installs no retry', async (t) => { t.is(rest.count(), 1); }); +test('REST instance never retries POST even when the user opts POST in', async (t) => { + // POST is stripped from the REST retryMethods regardless of user config, so + // non-idempotent REST writes are never repeated. + const permit = await newPermit({ retry: { ...tiny, retryMethods: ['GET', 'POST'] } }); + const rest = installRejectingAdapter(permit.config.axiosInstance); + + await t.throwsAsync(() => permit.config.axiosInstance.request({ method: 'POST', url: '/x' })); + + t.is(rest.count(), 1); +}); + test.serial('maps axios-retry retryCount to our 0-based attempt number', async (t) => { // axios-retry passes a 1-based retryCount; the interceptor must subtract one // so the first retry uses attempt 0. With jitter removed, attempt 0 -> 30ms diff --git a/src/tests/unit/retry.spec.ts b/src/tests/unit/retry.spec.ts index 10d48a96..bd9440f7 100644 --- a/src/tests/unit/retry.spec.ts +++ b/src/tests/unit/retry.spec.ts @@ -129,7 +129,7 @@ test('parseRetryAfter returns null for non digits-only delta-seconds', (t) => { t.is(parseRetryAfter('5abc'), null); }); -test('parseRetryAfter parses HTTP-date format', (t) => { +test.serial('parseRetryAfter parses HTTP-date format', (t) => { const realNow = Date.now; try { const fixedNow = Date.UTC(2015, 9, 21, 7, 28, 0); // "Wed, 21 Oct 2015 07:28:00 GMT" @@ -143,7 +143,7 @@ test('parseRetryAfter parses HTTP-date format', (t) => { } }); -test('parseRetryAfter returns 0 for past HTTP-date', (t) => { +test.serial('parseRetryAfter returns 0 for past HTTP-date', (t) => { const realNow = Date.now; try { const fixedNow = Date.UTC(2015, 9, 21, 7, 28, 0); @@ -251,6 +251,10 @@ test('resolveRetryConfig returns a copy that does not mutate the default', (t) = t.deepEqual(DEFAULT_RETRY_CONFIG.retryMethods, ['GET', 'HEAD', 'OPTIONS', 'PUT', 'DELETE']); }); +test('DEFAULT_RETRY_CONFIG.retryMethods is frozen', (t) => { + t.true(Object.isFrozen(DEFAULT_RETRY_CONFIG.retryMethods)); +}); + test('resolveRetryConfig returns disabled config when false', (t) => { const resolved = resolveRetryConfig(false); diff --git a/src/utils/retry-interceptor.ts b/src/utils/retry-interceptor.ts index b18cf0f8..69cd418e 100644 --- a/src/utils/retry-interceptor.ts +++ b/src/utils/retry-interceptor.ts @@ -54,7 +54,7 @@ export class AxiosRetryInterceptor { const url = error.config?.url ?? 'unknown'; logger.warn( `[${clientName}] Request failed (${status}), ` + - `retrying (attempt ${retryCount}/${config.maxRetries}): ${method} ${url}`, + `retry ${retryCount}/${config.maxRetries}: ${method} ${url}`, ); }, }); diff --git a/src/utils/retry.ts b/src/utils/retry.ts index 28ed4725..d8fa7db8 100644 --- a/src/utils/retry.ts +++ b/src/utils/retry.ts @@ -6,9 +6,16 @@ import { AxiosError } from 'axios'; export const RETRYABLE_STATUS_CODES = [408, 429, 500, 502, 503, 504]; /** - * Default HTTP methods that are safe to retry + * Default HTTP methods that are safe to retry. + * Frozen so the shared DEFAULT_RETRY_CONFIG array cannot be mutated by consumers. */ -const DEFAULT_RETRY_METHODS = ['GET', 'HEAD', 'OPTIONS', 'PUT', 'DELETE']; +const DEFAULT_RETRY_METHODS: readonly string[] = Object.freeze([ + 'GET', + 'HEAD', + 'OPTIONS', + 'PUT', + 'DELETE', +]); /** * Function type for custom retry condition evaluation @@ -27,7 +34,8 @@ export interface IRetryConfig { enabled?: boolean; /** - * Maximum number of retry attempts. Defaults to 3. + * Maximum number of retries after the initial request. Defaults to 3 + * (i.e. up to 4 total attempts). */ maxRetries?: number; @@ -105,7 +113,10 @@ export const DEFAULT_RETRY_CONFIG: IResolvedRetryConfig = Object.freeze({ maxDelay: 30000, retryCondition: defaultRetryCondition, respectRetryAfter: true, - retryMethods: DEFAULT_RETRY_METHODS, + // Cast keeps the public IResolvedRetryConfig.retryMethods type as string[]; + // the array is frozen at runtime and resolveRetryConfig always clones it + // before returning, so callers receive a mutable copy. + retryMethods: DEFAULT_RETRY_METHODS as string[], }); /** From 8fbb78cff47a6581cc16550842945a6da15d579e Mon Sep 17 00:00:00 2001 From: Zeev Manilovich Date: Wed, 24 Jun 2026 00:03:32 +0300 Subject: [PATCH 9/9] Harden retry config: freeze status codes, skip empty REST retry - Freeze RETRYABLE_STATUS_CODES (readonly number[]) so the exported array can't be mutated to change SDK-wide retry behavior. - Skip installing the REST retry interceptor when the POST-filtered retryMethods is empty (e.g. user passes retryMethods: ['POST']), avoiding a no-op interceptor. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/index.ts | 12 +++++++----- src/tests/unit/retry.spec.ts | 4 ++++ src/utils/retry.ts | 7 +++++-- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/index.ts b/src/index.ts index bb23d80f..a96af183 100644 --- a/src/index.ts +++ b/src/index.ts @@ -148,11 +148,13 @@ export class Permit implements IPermitClient { // writes are non-idempotent and must never be repeated. (This is symmetric // with the enforcer, which ADDS POST for the idempotent PDP/OPA check calls.) const resolvedRetryConfig = resolveRetryConfig(this.config.retry); - if (resolvedRetryConfig.enabled) { - const restRetryConfig = { - ...resolvedRetryConfig, - retryMethods: resolvedRetryConfig.retryMethods.filter((m) => m !== 'POST'), - }; + const restRetryConfig = { + ...resolvedRetryConfig, + retryMethods: resolvedRetryConfig.retryMethods.filter((m) => m !== 'POST'), + }; + // Skip the install when no methods remain (e.g. retryMethods: ['POST']), + // which would otherwise add an interceptor that can never retry. + if (resolvedRetryConfig.enabled && restRetryConfig.retryMethods.length > 0) { AxiosRetryInterceptor.setupInterceptor( this.config.axiosInstance, restRetryConfig, diff --git a/src/tests/unit/retry.spec.ts b/src/tests/unit/retry.spec.ts index bd9440f7..4d13dcd1 100644 --- a/src/tests/unit/retry.spec.ts +++ b/src/tests/unit/retry.spec.ts @@ -255,6 +255,10 @@ test('DEFAULT_RETRY_CONFIG.retryMethods is frozen', (t) => { t.true(Object.isFrozen(DEFAULT_RETRY_CONFIG.retryMethods)); }); +test('RETRYABLE_STATUS_CODES is frozen', (t) => { + t.true(Object.isFrozen(RETRYABLE_STATUS_CODES)); +}); + test('resolveRetryConfig returns disabled config when false', (t) => { const resolved = resolveRetryConfig(false); diff --git a/src/utils/retry.ts b/src/utils/retry.ts index d8fa7db8..0cec80f2 100644 --- a/src/utils/retry.ts +++ b/src/utils/retry.ts @@ -1,9 +1,12 @@ import { AxiosError } from 'axios'; /** - * HTTP status codes that should trigger a retry + * HTTP status codes that should trigger a retry. + * Frozen so consumers cannot mutate the SDK-wide retry behavior. */ -export const RETRYABLE_STATUS_CODES = [408, 429, 500, 502, 503, 504]; +export const RETRYABLE_STATUS_CODES: readonly number[] = Object.freeze([ + 408, 429, 500, 502, 503, 504, +]); /** * Default HTTP methods that are safe to retry.