From ed2f9c7eb8186e6f7d1b608667288e996dacc131 Mon Sep 17 00:00:00 2001 From: Akshay Dodeja Date: Wed, 24 Jun 2026 10:53:03 -0700 Subject: [PATCH 1/4] fix(sdk): request timeout, network-error normalization, idempotent-only retries, Retry-After, bounded iterate Harden the TypeScript SDK transport against the failure modes that can wedge or corrupt a caller in production: - Request timeout: every request is now bounded by an AbortController-based timeout (configurable via `timeoutMs`, default 30s, `0` disables). A hung upstream is aborted and rejected with a new `TimeoutError`. Applied once in Transport so it covers both the openapi-fetch client and `executeManual`. (new `client/timeout.ts`) - Network-error normalization + retry: a thrown `fetch` failure (DNS, ECONNRESET, "fetch failed", ...) is normalized to a `Terminal49Error` (`NetworkError`) and run through the same retry policy as a 5xx, via the RetryInterceptor `onError` hook. (`errors.ts`, `interceptors.ts`) - Idempotent-only retries: retries are gated to GET/HEAD (and writes that carry an `Idempotency-Key`). A POST/PATCH that hits a transient 5xx/network error is no longer silently replayed, which could have created duplicate tracking requests. (new pure `client/retry-policy.ts`) - Retry-After: 429 backoff honors the server `Retry-After` header (delta-seconds or HTTP-date) instead of fixed exponential only. - Bounded iterate(): `BaseManager.createIterator` now stops at documented `maxPages` / `maxRows` safety caps so a no-op/overly-broad filter cannot walk the entire dataset. (`managers/base.ts`) - Search path: `executeManual` reads success bodies with `readSuccessBody` (new `client/body.ts`) so a non-JSON 200 is surfaced instead of being silently collapsed to `undefined`, and a thrown network error is normalized. - Documented the load-bearing interceptor registration order (Retry registered last so it runs before error-mapping on the reverse onResponse/onError pass). Pure logic is extracted into unit-testable modules (retry-policy, timeout, body). New + updated mock-transport tests cover: timeout abort, network-error normalize+retry, Retry-After wait, write-not-retried, and the iterate bound. Closes DEV-10663 Co-Authored-By: Claude Opus 4.8 (1M context) --- sdks/typescript-sdk/src/client.test.ts | 45 ++-- .../src/client.transport.test.ts | 232 ++++++++++++++++++ sdks/typescript-sdk/src/client.ts | 11 + sdks/typescript-sdk/src/client/body.test.ts | 27 ++ sdks/typescript-sdk/src/client/body.ts | 26 ++ sdks/typescript-sdk/src/client/errors.ts | 35 +++ .../typescript-sdk/src/client/interceptors.ts | 87 ++++++- .../src/client/managers/base.ts | 44 +++- .../src/client/retry-policy.test.ts | 111 +++++++++ .../typescript-sdk/src/client/retry-policy.ts | 117 +++++++++ .../typescript-sdk/src/client/timeout.test.ts | 50 ++++ sdks/typescript-sdk/src/client/timeout.ts | 58 +++++ sdks/typescript-sdk/src/client/transport.ts | 61 ++++- 13 files changed, 854 insertions(+), 50 deletions(-) create mode 100644 sdks/typescript-sdk/src/client.transport.test.ts create mode 100644 sdks/typescript-sdk/src/client/body.test.ts create mode 100644 sdks/typescript-sdk/src/client/body.ts create mode 100644 sdks/typescript-sdk/src/client/retry-policy.test.ts create mode 100644 sdks/typescript-sdk/src/client/retry-policy.ts create mode 100644 sdks/typescript-sdk/src/client/timeout.test.ts create mode 100644 sdks/typescript-sdk/src/client/timeout.ts diff --git a/sdks/typescript-sdk/src/client.test.ts b/sdks/typescript-sdk/src/client.test.ts index e7dfa500..9a149385 100644 --- a/sdks/typescript-sdk/src/client.test.ts +++ b/sdks/typescript-sdk/src/client.test.ts @@ -3,6 +3,7 @@ import { FeatureNotEnabledError, NotFoundError, Terminal49Client, + UpstreamError, ValidationError, } from './client.js'; import { createMockFetch, jsonResponse } from './test/mock-fetch.js'; @@ -42,8 +43,11 @@ describe('Terminal49Client', () => { } }); - it('retries body requests after the original request is consumed', async () => { - vi.useFakeTimers(); + it('does NOT retry a write (POST) on a 5xx so the body is sent once', async () => { + // Retries are gated to idempotent methods. A POST (createTrackingRequest) + // with no Idempotency-Key must not be replayed, otherwise a transient 5xx + // could create duplicate tracking requests. The body is therefore sent + // exactly once and the UpstreamError is surfaced. let attempt = 0; const requestBodies: string[] = []; const fetchImpl = async ( @@ -53,52 +57,33 @@ describe('Terminal49Client', () => { const request = input instanceof Request ? input : new Request(input, init); const url = new URL(request.url); - const body = await request.text(); - requestBodies.push(body); + requestBodies.push(await request.text()); if (url.pathname !== '/v2/tracking_requests') { throw new Error(`Unexpected request to ${url.pathname}`); } attempt += 1; - if (attempt === 1) { - return jsonResponse({ errors: [{ detail: 'server error' }] }, 500); - } - - return jsonResponse({ data: { id: 'tr-1' } }); + return jsonResponse({ errors: [{ detail: 'server error' }] }, 500); }; const client = new Terminal49Client({ apiToken: 'token-123', apiBaseUrl: baseUrl, fetchImpl, - maxRetries: 1, + maxRetries: 3, } as any); - try { - const resultPromise = client.createTrackingRequest({ + await expect( + client.createTrackingRequest({ requestType: 'container', requestNumber: 'MSCU1234567', scac: 'MSCU', - }); - await vi.advanceTimersByTimeAsync(500); - const result = await resultPromise; + }), + ).rejects.toBeInstanceOf(UpstreamError); - expect(result.data.id).toBe('tr-1'); - expect(requestBodies).toHaveLength(2); - expect(JSON.parse(requestBodies[1])).toEqual({ - data: { - type: 'tracking_request', - attributes: { - request_type: 'container', - request_number: 'MSCU1234567', - scac: 'MSCU', - }, - }, - }); - } finally { - vi.useRealTimers(); - } + expect(attempt).toBe(1); + expect(requestBodies).toHaveLength(1); }); it('maps 404 responses to NotFoundError', async () => { diff --git a/sdks/typescript-sdk/src/client.transport.test.ts b/sdks/typescript-sdk/src/client.transport.test.ts new file mode 100644 index 00000000..6813ec93 --- /dev/null +++ b/sdks/typescript-sdk/src/client.transport.test.ts @@ -0,0 +1,232 @@ +import { describe, expect, it, vi } from 'vitest'; +import { + RateLimitError, + Terminal49Client, + Terminal49Error, + TimeoutError, + UpstreamError, +} from './client.js'; +import { jsonResponse } from './test/mock-fetch.js'; + +const baseUrl = 'https://api.test/v2'; + +describe('Terminal49Client transport resilience', () => { + it('normalizes a thrown network error and retries it like a 5xx', async () => { + vi.useFakeTimers(); + try { + let attempt = 0; + const fetchImpl = vi.fn(async () => { + attempt += 1; + if (attempt === 1) { + throw new TypeError('fetch failed'); + } + return jsonResponse({ data: { id: 'route-1' } }); + }); + + const client = new Terminal49Client({ + apiToken: 'token-123', + apiBaseUrl: baseUrl, + fetchImpl: fetchImpl as unknown as typeof fetch, + maxRetries: 1, + }); + + const resultPromise = client.getContainerRoute('abc'); + await vi.advanceTimersByTimeAsync(500); + const result = await resultPromise; + + expect(result.data.id).toBe('route-1'); + expect(attempt).toBe(2); + } finally { + vi.useRealTimers(); + } + }); + + it('surfaces a Terminal49Error when a network error exhausts retries', async () => { + vi.useFakeTimers(); + try { + const fetchImpl = vi.fn(async () => { + throw new TypeError('fetch failed'); + }); + + const client = new Terminal49Client({ + apiToken: 'token-123', + apiBaseUrl: baseUrl, + fetchImpl: fetchImpl as unknown as typeof fetch, + maxRetries: 1, + }); + + const resultPromise = client.getContainerRoute('abc'); + const assertion = + expect(resultPromise).rejects.toBeInstanceOf(Terminal49Error); + await vi.advanceTimersByTimeAsync(500); + await assertion; + // 1 initial + 1 retry + expect(fetchImpl).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } + }); + + it('waits for the Retry-After header before retrying a 429', async () => { + vi.useFakeTimers(); + try { + let attempt = 0; + const fetchImpl = vi.fn(async () => { + attempt += 1; + if (attempt === 1) { + return jsonResponse({ errors: [{ detail: 'slow down' }] }, 429, { + 'Retry-After': '3', + }); + } + return jsonResponse({ data: { id: 'route-1' } }); + }); + + const client = new Terminal49Client({ + apiToken: 'token-123', + apiBaseUrl: baseUrl, + fetchImpl: fetchImpl as unknown as typeof fetch, + maxRetries: 1, + }); + + const resultPromise = client.getContainerRoute('abc'); + + // The fixed exponential backoff for attempt 0 would be 500ms; the + // Retry-After header asks for 3s, so nothing should fire before then. + await vi.advanceTimersByTimeAsync(2000); + expect(attempt).toBe(1); + + await vi.advanceTimersByTimeAsync(1000); + const result = await resultPromise; + expect(result.data.id).toBe('route-1'); + expect(attempt).toBe(2); + } finally { + vi.useRealTimers(); + } + }); + + it('does not retry a write (POST) on a 5xx', async () => { + let attempt = 0; + const fetchImpl = vi.fn(async () => { + attempt += 1; + return jsonResponse({ errors: [{ detail: 'server error' }] }, 500); + }); + + const client = new Terminal49Client({ + apiToken: 'token-123', + apiBaseUrl: baseUrl, + fetchImpl: fetchImpl as unknown as typeof fetch, + maxRetries: 3, + }); + + await expect( + client.createTrackingRequest({ + requestType: 'container', + requestNumber: 'MSCU1234567', + }), + ).rejects.toBeInstanceOf(UpstreamError); + + // Write must NOT be retried: exactly one attempt. + expect(attempt).toBe(1); + }); + + it('still maps a 429 on a write to RateLimitError without retrying', async () => { + let attempt = 0; + const fetchImpl = vi.fn(async () => { + attempt += 1; + return jsonResponse({ errors: [{ detail: 'slow down' }] }, 429); + }); + + const client = new Terminal49Client({ + apiToken: 'token-123', + apiBaseUrl: baseUrl, + fetchImpl: fetchImpl as unknown as typeof fetch, + maxRetries: 3, + }); + + await expect( + client.createTrackingRequest({ + requestType: 'container', + requestNumber: 'MSCU1234567', + }), + ).rejects.toBeInstanceOf(RateLimitError); + expect(attempt).toBe(1); + }); + + it('stops iterate() at the safety page bound even when next never ends', async () => { + // A misconfigured/no-op filter where the server always advertises a + // `next` link would otherwise walk the entire dataset forever. The + // iterator must stop at its documented max-pages cap. + let pageRequests = 0; + const fetchImpl = vi.fn(async (input: Request | URL | string) => { + pageRequests += 1; + const url = new URL( + typeof input === 'string' + ? input + : input instanceof URL + ? input.toString() + : input.url, + ); + const page = Number(url.searchParams.get('page[number]') ?? '1'); + return jsonResponse({ + data: [ + { + id: `ship-${page}`, + type: 'shipment', + attributes: { status: 'in_transit' }, + }, + ], + // Always advertise a next page — an unbounded iterator would loop. + links: { next: `page=${page + 1}` }, + }); + }); + + const client = new Terminal49Client({ + apiToken: 'token-123', + apiBaseUrl: baseUrl, + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + const items = []; + for await (const shipment of client.shipments.iterate({}, { + pageSize: 1, + maxPages: 3, + } as { pageSize?: number; maxPages?: number })) { + items.push(shipment); + } + + expect(items).toHaveLength(3); + expect(pageRequests).toBe(3); + }); + + it('aborts a hung request via the configured request timeout', async () => { + vi.useFakeTimers(); + try { + const fetchImpl = vi.fn( + (_input: Request | URL | string, init?: RequestInit) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + reject( + Object.assign(new Error('aborted'), { name: 'AbortError' }), + ); + }); + }), + ); + + const client = new Terminal49Client({ + apiToken: 'token-123', + apiBaseUrl: baseUrl, + fetchImpl: fetchImpl as unknown as typeof fetch, + maxRetries: 0, + timeoutMs: 100, + }); + + const resultPromise = client.getContainer('abc'); + const assertion = + expect(resultPromise).rejects.toBeInstanceOf(TimeoutError); + await vi.advanceTimersByTimeAsync(100); + await assertion; + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/sdks/typescript-sdk/src/client.ts b/sdks/typescript-sdk/src/client.ts index fabbcb57..f5b740be 100644 --- a/sdks/typescript-sdk/src/client.ts +++ b/sdks/typescript-sdk/src/client.ts @@ -3,9 +3,11 @@ import { AuthenticationError, AuthorizationError, FeatureNotEnabledError, + NetworkError, NotFoundError, RateLimitError, Terminal49Error, + TimeoutError, UpstreamError, ValidationError, } from './client/errors.js'; @@ -41,9 +43,11 @@ export { AuthenticationError, AuthorizationError, FeatureNotEnabledError, + NetworkError, NotFoundError, RateLimitError, Terminal49Error, + TimeoutError, UpstreamError, ValidationError, }; @@ -58,6 +62,12 @@ export interface Terminal49ClientConfig { apiBaseUrl?: string; /** Number of retry attempts for rate-limit and server errors. Defaults to `2`. */ maxRetries?: number; + /** + * Per-request timeout in milliseconds. A hung request is aborted once it + * elapses and rejected with a `TimeoutError`. Defaults to `30000`. Set to `0` + * to disable the timeout. + */ + timeoutMs?: number; /** Optional fetch implementation, useful for tests or custom runtimes. */ fetchImpl?: typeof fetch; /** Default response format for methods that support mapped responses. Defaults to `raw`. */ @@ -108,6 +118,7 @@ export class Terminal49Client { accountId: config.accountId, baseUrl, maxRetries: config.maxRetries, + timeoutMs: config.timeoutMs, fetchImpl: config.fetchImpl, }); diff --git a/sdks/typescript-sdk/src/client/body.test.ts b/sdks/typescript-sdk/src/client/body.test.ts new file mode 100644 index 00000000..b527eaa7 --- /dev/null +++ b/sdks/typescript-sdk/src/client/body.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from 'vitest'; +import { readSuccessBody } from './body.js'; + +describe('readSuccessBody', () => { + it('parses a JSON body', async () => { + const res = new Response(JSON.stringify({ hits: 1 }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + await expect(readSuccessBody(res)).resolves.toEqual({ hits: 1 }); + }); + + it('returns raw text for a non-JSON success body instead of undefined', async () => { + const res = new Response('not json', { status: 200 }); + await expect(readSuccessBody(res)).resolves.toBe('not json'); + }); + + it('returns undefined for an empty body', async () => { + const res = new Response('', { status: 200 }); + await expect(readSuccessBody(res)).resolves.toBeUndefined(); + }); + + it('returns undefined for a 204 No Content', async () => { + const res = new Response(null, { status: 204 }); + await expect(readSuccessBody(res)).resolves.toBeUndefined(); + }); +}); diff --git a/sdks/typescript-sdk/src/client/body.ts b/sdks/typescript-sdk/src/client/body.ts new file mode 100644 index 00000000..a90f6a6d --- /dev/null +++ b/sdks/typescript-sdk/src/client/body.ts @@ -0,0 +1,26 @@ +/** + * Read a successful response body without silently discarding it. + * + * The previous behavior parsed JSON and, on any failure, returned `undefined` — + * which meant a 200 with a non-JSON or unexpected body looked identical to an + * empty body to the caller. This instead: + * + * - returns `undefined` only for a genuinely empty body (204 / empty text); + * - returns parsed JSON when the body is JSON; + * - returns the raw text when the body is present but not JSON, so a non-JSON + * success body is surfaced rather than swallowed. + */ +export async function readSuccessBody( + response: Response, +): Promise { + if (response.status === 204) return undefined; + + const text = await response.clone().text(); + if (text === '') return undefined; + + try { + return JSON.parse(text) as T; + } catch { + return text; + } +} diff --git a/sdks/typescript-sdk/src/client/errors.ts b/sdks/typescript-sdk/src/client/errors.ts index 256f9af5..15f73140 100644 --- a/sdks/typescript-sdk/src/client/errors.ts +++ b/sdks/typescript-sdk/src/client/errors.ts @@ -67,6 +67,41 @@ export class UpstreamError extends Terminal49Error { } } +/** + * Thrown when a transport-level failure occurs before a response is received — + * a DNS failure, a refused/reset connection, or an otherwise failed `fetch`. + * Has no HTTP status because no response was produced. + */ +export class NetworkError extends Terminal49Error { + constructor(message: string, details?: unknown) { + super(message, undefined, details); + this.name = 'NetworkError'; + } +} + +/** + * Thrown when a request exceeds the configured request timeout and is aborted + * by the SDK. Has no HTTP status because no response was produced. + */ +export class TimeoutError extends Terminal49Error { + constructor(message = 'Request timed out', details?: unknown) { + super(message, undefined, details); + this.name = 'TimeoutError'; + } +} + +/** + * Normalize a thrown transport error (from `fetch`) into a {@link Terminal49Error}. + * A pre-existing `Terminal49Error` (e.g. our own {@link TimeoutError}) is passed + * through unchanged; everything else becomes a {@link NetworkError}. + */ +export function toNetworkError(error: unknown): Terminal49Error { + if (error instanceof Terminal49Error) return error; + const message = + error instanceof Error ? error.message : 'Network request failed'; + return new NetworkError(`Network request failed: ${message}`, error); +} + export function extractErrorMessage(body: any): string { if (typeof body === 'string') { return body; diff --git a/sdks/typescript-sdk/src/client/interceptors.ts b/sdks/typescript-sdk/src/client/interceptors.ts index bce27f6b..19494f67 100644 --- a/sdks/typescript-sdk/src/client/interceptors.ts +++ b/sdks/typescript-sdk/src/client/interceptors.ts @@ -1,8 +1,22 @@ import type { Middleware, MiddlewareCallbackParams } from 'openapi-fetch'; -import { extractErrorMessage, toTerminal49Error } from './errors.js'; +import { + extractErrorMessage, + toNetworkError, + toTerminal49Error, +} from './errors.js'; +import { + computeBackoffDelay, + isRetryableNetworkError, + isRetryableStatus, + parseRetryAfterMs, + shouldRetryRequest, +} from './retry-policy.js'; export type Interceptor = Middleware; +/** Header a caller can set to make a non-idempotent write safe to retry. */ +const IDEMPOTENCY_KEY_HEADER = 'Idempotency-Key'; + export class AuthInterceptor { constructor( private apiToken: string, @@ -26,6 +40,17 @@ export class AuthInterceptor { } } +/** + * Retries transient failures with backoff. Two kinds of failure are handled: + * + * - A response with a retryable status (429 / 5xx), handled in `onResponse`. + * - A thrown transport error (DNS/connection/"fetch failed"), handled in + * `onError` — these never reach `onResponse` because `fetch` rejected. + * + * Retries are gated by {@link shouldRetryRequest}: idempotent methods are always + * eligible, but non-idempotent writes are only retried when the caller supplied + * an `Idempotency-Key` header. 429 backoff honors the server's `Retry-After`. + */ export class RetryInterceptor { private replayableRequests = new Map(); @@ -61,11 +86,14 @@ export class RetryInterceptor { try { while ( replayableRequest && - (currentResponse.status === 429 || currentResponse.status >= 500) && + isRetryableStatus(currentResponse.status) && + this.isRetryable(replayableRequest) && attempt < this.maxRetries ) { - const delay = 2 ** attempt * 500; - await new Promise((resolve) => setTimeout(resolve, delay)); + const retryAfterMs = parseRetryAfterMs( + currentResponse.headers.get('Retry-After'), + ); + await this.sleep(computeBackoffDelay(attempt, retryAfterMs)); currentResponse = await this.fetchImpl(replayableRequest.clone()); attempt++; @@ -77,13 +105,58 @@ export class RetryInterceptor { } } - onError({ + /** + * Recover from a thrown transport error by retrying eligible requests. If a + * retry produces a response we return it (openapi-fetch then runs the normal + * `onResponse` chain on it); otherwise we surface a normalized + * {@link NetworkError} so error mapping is consistent with the response path. + */ + async onError({ request, + error, id, }: Pick & { error: unknown; - }) { - this.replayableRequests.delete(this.requestKey(request, id)); + }): Promise { + const requestKey = this.requestKey(request, id); + const replayableRequest = this.replayableRequests.get(requestKey); + + try { + if ( + replayableRequest && + isRetryableNetworkError(error) && + this.isRetryable(replayableRequest) + ) { + let attempt = 0; + while (attempt < this.maxRetries) { + await this.sleep(computeBackoffDelay(attempt)); + try { + return await this.fetchImpl(replayableRequest.clone()); + } catch (retryError) { + attempt++; + if (attempt >= this.maxRetries) { + return toNetworkError(retryError); + } + } + } + } + + return toNetworkError(error); + } finally { + this.replayableRequests.delete(requestKey); + } + } + + private isRetryable(request: Request): boolean { + return shouldRetryRequest({ + method: request.method, + hasIdempotencyKey: request.headers.has(IDEMPOTENCY_KEY_HEADER), + }); + } + + private sleep(ms: number): Promise { + if (ms <= 0) return Promise.resolve(); + return new Promise((resolve) => setTimeout(resolve, ms)); } private requestKey(request: Request, id?: string) { diff --git a/sdks/typescript-sdk/src/client/managers/base.ts b/sdks/typescript-sdk/src/client/managers/base.ts index c0429863..a63995af 100644 --- a/sdks/typescript-sdk/src/client/managers/base.ts +++ b/sdks/typescript-sdk/src/client/managers/base.ts @@ -3,6 +3,26 @@ import type { ListOptions, ResponseFormat } from '../../types/options.js'; import { applyPagination } from '../query.js'; import type { Transport } from '../transport.js'; +/** + * Hard safety caps for {@link BaseManager.createIterator}. They exist so a no-op + * or mistakenly broad filter cannot silently walk the entire dataset (and make + * thousands of requests). They are deliberately large enough not to interfere + * with realistic pagination, and can be raised per call via + * `maxPages` / `maxRows` when a caller genuinely needs more. + */ +export const DEFAULT_ITERATE_MAX_PAGES = 1000; +export const DEFAULT_ITERATE_MAX_ROWS = 100_000; + +/** Options accepted by {@link BaseManager.createIterator}. */ +export interface IterateOptions { + /** Records per page passed to the underlying list call. */ + pageSize?: number; + /** Maximum number of pages to fetch. Defaults to {@link DEFAULT_ITERATE_MAX_PAGES}. */ + maxPages?: number; + /** Maximum number of rows to yield. Defaults to {@link DEFAULT_ITERATE_MAX_ROWS}. */ + maxRows?: number; +} + export abstract class BaseManager { constructor( protected transport: Transport, @@ -43,21 +63,35 @@ export abstract class BaseManager { } /** - * Helper to create an async iterator from a list method that returns PaginatedResult + * Helper to create an async iterator from a list method that returns + * `PaginatedResult`. + * + * The iterator is bounded by `maxPages` and `maxRows` (see + * {@link DEFAULT_ITERATE_MAX_PAGES} / {@link DEFAULT_ITERATE_MAX_ROWS}). Once + * either cap is reached iteration stops even if the API keeps advertising a + * `next` link, so an empty or overly broad filter cannot walk the whole + * dataset unbounded. Callers that genuinely need more can raise the caps. */ protected async *createIterator( listMethod: (options: { page?: number; pageSize?: number; }) => Promise | any>, - options?: { pageSize?: number }, + options?: IterateOptions, ): AsyncGenerator { + const maxPages = options?.maxPages ?? DEFAULT_ITERATE_MAX_PAGES; + const maxRows = options?.maxRows ?? DEFAULT_ITERATE_MAX_ROWS; + let currentPage = 1; - while (true) { + let pagesFetched = 0; + let rowsYielded = 0; + + while (pagesFetched < maxPages) { const result = await listMethod({ page: currentPage, pageSize: options?.pageSize, }); + pagesFetched++; // If the user requested raw or both format, we might need to extract mapped. // Iterate is meant for mapped items. const paginatedResult = result.mapped ? result.mapped : result; @@ -71,7 +105,11 @@ export abstract class BaseManager { } for (const item of paginatedResult.items) { + if (rowsYielded >= maxRows) { + return; + } yield item; + rowsYielded++; } if (!paginatedResult.links?.next) { diff --git a/sdks/typescript-sdk/src/client/retry-policy.test.ts b/sdks/typescript-sdk/src/client/retry-policy.test.ts new file mode 100644 index 00000000..7f7f0a24 --- /dev/null +++ b/sdks/typescript-sdk/src/client/retry-policy.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it } from 'vitest'; +import { + computeBackoffDelay, + isIdempotentMethod, + isRetryableNetworkError, + isRetryableStatus, + parseRetryAfterMs, + shouldRetryRequest, +} from './retry-policy.js'; + +describe('retry-policy', () => { + describe('isIdempotentMethod', () => { + it('treats GET and HEAD as idempotent', () => { + expect(isIdempotentMethod('GET')).toBe(true); + expect(isIdempotentMethod('get')).toBe(true); + expect(isIdempotentMethod('HEAD')).toBe(true); + }); + + it('treats writes as non-idempotent', () => { + expect(isIdempotentMethod('POST')).toBe(false); + expect(isIdempotentMethod('PATCH')).toBe(false); + expect(isIdempotentMethod('DELETE')).toBe(false); + expect(isIdempotentMethod('PUT')).toBe(false); + }); + }); + + describe('isRetryableStatus', () => { + it('retries 429 and 5xx', () => { + expect(isRetryableStatus(429)).toBe(true); + expect(isRetryableStatus(500)).toBe(true); + expect(isRetryableStatus(503)).toBe(true); + }); + + it('does not retry 4xx other than 429', () => { + expect(isRetryableStatus(400)).toBe(false); + expect(isRetryableStatus(404)).toBe(false); + expect(isRetryableStatus(200)).toBe(false); + }); + }); + + describe('isRetryableNetworkError', () => { + it('treats fetch/connection errors as retryable', () => { + expect(isRetryableNetworkError(new TypeError('fetch failed'))).toBe(true); + const econn = Object.assign(new Error('boom'), { code: 'ECONNRESET' }); + expect(isRetryableNetworkError(econn)).toBe(true); + const dns = Object.assign(new Error('boom'), { code: 'ENOTFOUND' }); + expect(isRetryableNetworkError(dns)).toBe(true); + }); + + it('does not treat an AbortError (timeout) as a retryable network error', () => { + const abort = Object.assign(new Error('aborted'), { name: 'AbortError' }); + expect(isRetryableNetworkError(abort)).toBe(false); + }); + }); + + describe('shouldRetryRequest', () => { + it('allows retry for idempotent methods', () => { + expect(shouldRetryRequest({ method: 'GET' })).toBe(true); + expect(shouldRetryRequest({ method: 'HEAD' })).toBe(true); + }); + + it('blocks retry for writes without an Idempotency-Key', () => { + expect(shouldRetryRequest({ method: 'POST' })).toBe(false); + expect(shouldRetryRequest({ method: 'PATCH' })).toBe(false); + }); + + it('allows retry for writes that carry an Idempotency-Key', () => { + expect( + shouldRetryRequest({ method: 'POST', hasIdempotencyKey: true }), + ).toBe(true); + }); + }); + + describe('parseRetryAfterMs', () => { + it('parses delta-seconds', () => { + expect(parseRetryAfterMs('2')).toBe(2000); + expect(parseRetryAfterMs('0')).toBe(0); + }); + + it('parses an HTTP-date relative to now', () => { + const now = Date.parse('2026-01-01T00:00:00Z'); + const date = new Date(now + 5000).toUTCString(); + expect(parseRetryAfterMs(date, now)).toBe(5000); + }); + + it('returns undefined for missing or malformed values', () => { + expect(parseRetryAfterMs(null)).toBeUndefined(); + expect(parseRetryAfterMs('')).toBeUndefined(); + expect(parseRetryAfterMs('not-a-date')).toBeUndefined(); + }); + + it('never returns a negative delay for a past date', () => { + const now = Date.parse('2026-01-01T00:00:00Z'); + const past = new Date(now - 5000).toUTCString(); + expect(parseRetryAfterMs(past, now)).toBe(0); + }); + }); + + describe('computeBackoffDelay', () => { + it('uses exponential backoff when no Retry-After is present', () => { + expect(computeBackoffDelay(0)).toBe(500); + expect(computeBackoffDelay(1)).toBe(1000); + expect(computeBackoffDelay(2)).toBe(2000); + }); + + it('honors Retry-After over exponential backoff', () => { + expect(computeBackoffDelay(0, 3000)).toBe(3000); + expect(computeBackoffDelay(2, 100)).toBe(100); + }); + }); +}); diff --git a/sdks/typescript-sdk/src/client/retry-policy.ts b/sdks/typescript-sdk/src/client/retry-policy.ts new file mode 100644 index 00000000..7863d377 --- /dev/null +++ b/sdks/typescript-sdk/src/client/retry-policy.ts @@ -0,0 +1,117 @@ +/** + * Pure, side-effect-free helpers that decide whether and when a request may be + * retried. Kept separate from the interceptor so the policy can be unit-tested + * in isolation and reasoned about without a live transport. + */ + +/** HTTP methods that are safe to retry automatically (no observable side effect). */ +const IDEMPOTENT_METHODS = new Set(['GET', 'HEAD', 'OPTIONS']); + +/** Node/undici/browser error codes that indicate a transient connection failure. */ +const RETRYABLE_NETWORK_CODES = new Set([ + 'ECONNRESET', + 'ECONNREFUSED', + 'ETIMEDOUT', + 'EPIPE', + 'EAI_AGAIN', + 'ENOTFOUND', + 'EHOSTUNREACH', + 'ENETUNREACH', + 'UND_ERR_CONNECT_TIMEOUT', + 'UND_ERR_SOCKET', +]); + +/** Whether the HTTP method can be retried without risking a duplicate write. */ +export function isIdempotentMethod(method: string): boolean { + return IDEMPOTENT_METHODS.has(method.toUpperCase()); +} + +/** Whether the response status is one the SDK retries (429 + 5xx). */ +export function isRetryableStatus(status: number): boolean { + return status === 429 || status >= 500; +} + +/** + * Whether a thrown `fetch` error is a transient network failure that should be + * retried (DNS, connection reset, socket hang-up, "fetch failed", ...). + * + * An `AbortError` is explicitly NOT retryable: it means our own request-timeout + * fired, so replaying it would just hang again. + */ +export function isRetryableNetworkError(error: unknown): boolean { + if (!error || typeof error !== 'object') return false; + + const err = error as { name?: string; code?: string; message?: string }; + if (err.name === 'AbortError' || err.name === 'TimeoutError') return false; + + if (typeof err.code === 'string' && RETRYABLE_NETWORK_CODES.has(err.code)) { + return true; + } + + // undici/whatwg surface generic connection failures as a TypeError whose + // message is "fetch failed" (often with a `cause`). + if (error instanceof TypeError) return true; + + const message = typeof err.message === 'string' ? err.message : ''; + return /fetch failed|network|socket hang up|terminated/i.test(message); +} + +/** Inputs to {@link shouldRetryRequest}. */ +export interface RetryRequestContext { + method: string; + /** Set when the caller supplied an `Idempotency-Key`, making a write safe to replay. */ + hasIdempotencyKey?: boolean; +} + +/** + * Whether a request may be retried at all. Idempotent methods are always + * eligible; non-idempotent writes are only eligible when the caller opted in + * with an `Idempotency-Key` header. + */ +export function shouldRetryRequest(ctx: RetryRequestContext): boolean { + if (isIdempotentMethod(ctx.method)) return true; + return ctx.hasIdempotencyKey === true; +} + +/** + * Parse a `Retry-After` header into milliseconds. Supports both delta-seconds + * (`"120"`) and an HTTP-date. Returns `undefined` when absent/unparseable, and + * never returns a negative delay. + * + * @param now - Reference time (ms since epoch) used for HTTP-date math; defaults + * to `Date.now()` and is injectable for deterministic tests. + */ +export function parseRetryAfterMs( + value: string | null | undefined, + now: number = Date.now(), +): number | undefined { + if (value == null) return undefined; + const trimmed = value.trim(); + if (trimmed === '') return undefined; + + if (/^\d+$/.test(trimmed)) { + return Number(trimmed) * 1000; + } + + const dateMs = Date.parse(trimmed); + if (Number.isNaN(dateMs)) return undefined; + return Math.max(0, dateMs - now); +} + +/** Base unit (ms) for exponential backoff: attempt 0 -> 500ms, 1 -> 1000ms, ... */ +const BACKOFF_BASE_MS = 500; + +/** + * Delay (ms) before the next retry attempt. When the server provided a + * `Retry-After` value (`retryAfterMs`), that is honored; otherwise the SDK uses + * exponential backoff `2 ** attempt * 500`. + */ +export function computeBackoffDelay( + attempt: number, + retryAfterMs?: number, +): number { + if (typeof retryAfterMs === 'number' && retryAfterMs >= 0) { + return retryAfterMs; + } + return 2 ** attempt * BACKOFF_BASE_MS; +} diff --git a/sdks/typescript-sdk/src/client/timeout.test.ts b/sdks/typescript-sdk/src/client/timeout.test.ts new file mode 100644 index 00000000..d8ee5464 --- /dev/null +++ b/sdks/typescript-sdk/src/client/timeout.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it, vi } from 'vitest'; +import { TimeoutError } from './errors.js'; +import { withTimeout } from './timeout.js'; + +describe('withTimeout', () => { + it('passes through a fast response unchanged', async () => { + const inner = vi.fn(async () => new Response('ok')); + const fetchImpl = withTimeout(inner, 1000); + + const res = await fetchImpl(new Request('https://api.test/v2/ping')); + expect(await res.text()).toBe('ok'); + expect(inner).toHaveBeenCalledTimes(1); + }); + + it('aborts a hung request once the timeout elapses', async () => { + vi.useFakeTimers(); + try { + const inner = vi.fn( + (input: Request | URL | string, init?: RequestInit) => + new Promise((_resolve, reject) => { + const signal = init?.signal; + signal?.addEventListener('abort', () => { + reject( + Object.assign(new Error('aborted'), { name: 'AbortError' }), + ); + }); + }), + ); + const fetchImpl = withTimeout(inner, 50); + + const promise = fetchImpl(new Request('https://api.test/v2/hang')); + const assertion = expect(promise).rejects.toBeInstanceOf(TimeoutError); + await vi.advanceTimersByTimeAsync(50); + await assertion; + } finally { + vi.useRealTimers(); + } + }); + + it('does not wrap with a timeout when timeoutMs is 0', async () => { + const inner = vi.fn(async (_input, init?: RequestInit) => { + // No timeout signal should be injected. + expect(init?.signal).toBeUndefined(); + return new Response('ok'); + }); + const fetchImpl = withTimeout(inner, 0); + await fetchImpl(new Request('https://api.test/v2/ping')); + expect(inner).toHaveBeenCalledTimes(1); + }); +}); diff --git a/sdks/typescript-sdk/src/client/timeout.ts b/sdks/typescript-sdk/src/client/timeout.ts new file mode 100644 index 00000000..ff37b488 --- /dev/null +++ b/sdks/typescript-sdk/src/client/timeout.ts @@ -0,0 +1,58 @@ +import { TimeoutError } from './errors.js'; + +/** + * Wrap a `fetch` implementation so every request is bounded by an + * AbortController-based timeout. If `timeoutMs <= 0` the original fetch is + * returned untouched (no signal is injected), which keeps timeouts opt-out and + * preserves any caller-provided `signal`. + * + * When the timeout fires the in-flight request is aborted and the returned + * promise rejects with a {@link TimeoutError}. A caller `signal` is honored too: + * if it aborts first we forward that abort to the underlying request. + */ +export function withTimeout( + fetchImpl: typeof fetch, + timeoutMs: number, +): typeof fetch { + if (!Number.isFinite(timeoutMs) || timeoutMs <= 0) { + return fetchImpl; + } + + return async function timedFetch( + input: Parameters[0], + init?: Parameters[1], + ): Promise { + const controller = new AbortController(); + let timedOut = false; + + const timer = setTimeout(() => { + timedOut = true; + controller.abort(); + }, timeoutMs); + + // Forward a caller-supplied signal so an external cancel still works. + const callerSignal = init?.signal ?? undefined; + const onCallerAbort = () => controller.abort(); + if (callerSignal) { + if (callerSignal.aborted) { + controller.abort(); + } else { + callerSignal.addEventListener('abort', onCallerAbort, { once: true }); + } + } + + try { + return await fetchImpl(input, { ...init, signal: controller.signal }); + } catch (error) { + if (timedOut) { + throw new TimeoutError(`Request timed out after ${timeoutMs}ms`); + } + throw error; + } finally { + clearTimeout(timer); + if (callerSignal) { + callerSignal.removeEventListener('abort', onCallerAbort); + } + } + }; +} diff --git a/sdks/typescript-sdk/src/client/transport.ts b/sdks/typescript-sdk/src/client/transport.ts index 8310712e..3055638e 100644 --- a/sdks/typescript-sdk/src/client/transport.ts +++ b/sdks/typescript-sdk/src/client/transport.ts @@ -4,18 +4,29 @@ import createClient, { type MiddlewareCallbackParams, } from 'openapi-fetch'; import type { paths } from '../generated/terminal49.js'; +import { readSuccessBody } from './body.js'; +import { toNetworkError } from './errors.js'; import { AuthInterceptor, ErrorMappingInterceptor, type Interceptor, RetryInterceptor, } from './interceptors.js'; +import { withTimeout } from './timeout.js'; + +/** Default per-request timeout (ms) applied when the caller does not override it. */ +export const DEFAULT_REQUEST_TIMEOUT_MS = 30_000; export interface TransportConfig { apiToken: string; accountId?: string; baseUrl: string; maxRetries?: number; + /** + * Per-request timeout in milliseconds. Defaults to + * {@link DEFAULT_REQUEST_TIMEOUT_MS}. Set to `0` to disable the timeout. + */ + timeoutMs?: number; fetchImpl?: typeof fetch; } @@ -26,6 +37,7 @@ export class Transport { private accountId?: string; public baseUrl: string; private maxRetries: number; + /** Timeout-wrapped fetch used for both the typed client and manual requests. */ private fetchImpl: typeof fetch; public client: ApiClient; @@ -34,13 +46,24 @@ export class Transport { this.accountId = config.accountId; this.baseUrl = config.baseUrl; this.maxRetries = config.maxRetries ?? 2; - this.fetchImpl = config.fetchImpl ?? fetch; + + const baseFetch = config.fetchImpl ?? fetch; + const timeoutMs = config.timeoutMs ?? DEFAULT_REQUEST_TIMEOUT_MS; + // Every request is bounded by an AbortController-based timeout so a hung + // upstream cannot wedge a caller forever. Applied once here so it covers + // both the openapi-fetch client and the manual `executeManual` path. + this.fetchImpl = withTimeout(baseFetch, timeoutMs); this.client = createClient({ baseUrl: this.baseUrl, fetch: this.fetchImpl, }); + // Interceptor registration order is load-bearing. openapi-fetch runs + // `onRequest` in registration order but `onResponse`/`onError` in REVERSE + // order. Registering Retry LAST means that on the response/error pass it + // runs BEFORE error-mapping — so transient 429/5xx and network failures get + // a chance to retry before ErrorMapping would otherwise throw on them. this.client.use(new AuthInterceptor(this.apiToken, this.accountId)); this.client.use(new ErrorMappingInterceptor()); this.client.use(new RetryInterceptor(this.maxRetries, this.fetchImpl)); @@ -57,13 +80,17 @@ export class Transport { return data as T; } + /** + * Run a request that has no entry in the generated OpenAPI types (currently + * only `search()`) through the same Auth -> Retry -> ErrorMapping pipeline the + * typed client uses, including the timeout-wrapped fetch. Successful bodies are + * read with {@link readSuccessBody} so a non-JSON success body is surfaced + * rather than silently collapsed to `undefined`. + */ public async executeManual( input: Request | URL | string, init?: RequestInit, ): Promise { - // This goes through the raw fetchImpl, bypassing openapi-fetch middleware for now, - // or we could construct a Request and run it through the middleware manually. - // For search(), which is the only user, we'll just run it directly. const req = new Request(input, init); const auth = new AuthInterceptor(this.apiToken, this.accountId); const retry = new RetryInterceptor(this.maxRetries, this.fetchImpl); @@ -73,7 +100,25 @@ export class Transport { const authedReq = auth.onRequest(middlewareContext) || req; const retryContext = this.manualMiddlewareContext(authedReq); const retryableReq = retry.onRequest(retryContext); - let res = await this.fetchImpl(retryableReq); + + let res: Response; + try { + res = await this.fetchImpl(retryableReq.clone()); + } catch (error) { + // Mirror openapi-fetch's onError contract: a recovered Response is used, + // otherwise the normalized error is thrown. + const recovered = await retry.onError({ + request: retryableReq, + error, + id: retryContext.id, + }); + if (recovered instanceof Response) { + res = recovered; + } else { + throw recovered instanceof Error ? recovered : toNetworkError(error); + } + } + res = await retry.onResponse({ request: retryableReq, response: res, @@ -85,11 +130,7 @@ export class Transport { id: retryContext.id, }); - try { - return (await res.clone().json()) as T; - } catch { - return undefined as any; - } + return (await readSuccessBody(res)) as T; } private manualMiddlewareContext( From 96e9b2eee25f53b37608753267d883e3c4d6349f Mon Sep 17 00:00:00 2001 From: Akshay Dodeja Date: Wed, 24 Jun 2026 11:12:21 -0700 Subject: [PATCH 2/4] docs(sdk): regenerate reference for transport resilience exports Co-Authored-By: Claude Opus 4.8 (1M context) --- .../reference/client/classes/NetworkError.mdx | 136 ++++++++++++++++++ .../client/classes/Terminal49Error.mdx | 2 + .../reference/client/classes/TimeoutError.mdx | 135 +++++++++++++++++ docs/sdk/reference/client/index.mdx | 2 + .../interceptors/classes/RetryInterceptor.mdx | 19 ++- .../reference/client/interceptors/index.mdx | 2 +- .../interfaces/Terminal49ClientConfig.mdx | 1 + docs/sdk/reference/client/managers/index.mdx | 8 ++ .../managers/interfaces/IterateOptions.mdx | 15 ++ .../variables/DEFAULT_ITERATE_MAX_PAGES.mdx | 13 ++ .../variables/DEFAULT_ITERATE_MAX_ROWS.mdx | 7 + .../client/transport/classes/Transport.mdx | 6 + docs/sdk/reference/client/transport/index.mdx | 6 + .../transport/interfaces/TransportConfig.mdx | 15 +- .../variables/DEFAULT_REQUEST_TIMEOUT_MS.mdx | 9 ++ 15 files changed, 366 insertions(+), 10 deletions(-) create mode 100644 docs/sdk/reference/client/classes/NetworkError.mdx create mode 100644 docs/sdk/reference/client/classes/TimeoutError.mdx create mode 100644 docs/sdk/reference/client/managers/interfaces/IterateOptions.mdx create mode 100644 docs/sdk/reference/client/managers/variables/DEFAULT_ITERATE_MAX_PAGES.mdx create mode 100644 docs/sdk/reference/client/managers/variables/DEFAULT_ITERATE_MAX_ROWS.mdx create mode 100644 docs/sdk/reference/client/transport/variables/DEFAULT_REQUEST_TIMEOUT_MS.mdx diff --git a/docs/sdk/reference/client/classes/NetworkError.mdx b/docs/sdk/reference/client/classes/NetworkError.mdx new file mode 100644 index 00000000..b63735f2 --- /dev/null +++ b/docs/sdk/reference/client/classes/NetworkError.mdx @@ -0,0 +1,136 @@ +--- +title: "Class: NetworkError" +--- + +# Class: NetworkError + +Thrown when a transport-level failure occurs before a response is received — +a DNS failure, a refused/reset connection, or an otherwise failed `fetch`. +Has no HTTP status because no response was produced. + +## Extends + +- [`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error) + +## Constructors + +### Constructor + +> **new NetworkError**(`message`, `details?`): `NetworkError` + +#### Parameters + +| Parameter | Type | +| ------ | ------ | +| `message` | `string` | +| `details?` | `unknown` | + +#### Returns + +`NetworkError` + +#### Overrides + +[`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`constructor`](/sdk/reference/client/classes/Terminal49Error#constructor) + +## Properties + +| Property | Modifier | Type | Description | Inherited from | +| ------ | ------ | ------ | ------ | ------ | +| `cause?` | `public` | `unknown` | - | [`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`cause`](/sdk/reference/client/classes/Terminal49Error#property-cause) | +| `details?` | `public` | `unknown` | - | [`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`details`](/sdk/reference/client/classes/Terminal49Error#property-details) | +| `message` | `public` | `string` | - | [`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`message`](/sdk/reference/client/classes/Terminal49Error#property-message) | +| `name` | `public` | `string` | - | [`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`name`](/sdk/reference/client/classes/Terminal49Error#property-name) | +| `stack?` | `public` | `string` | - | [`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`stack`](/sdk/reference/client/classes/Terminal49Error#property-stack) | +| `status?` | `public` | `number` | - | [`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`status`](/sdk/reference/client/classes/Terminal49Error#property-status) | +| `stackTraceLimit` | `static` | `number` | The `Error.stackTraceLimit` property specifies the number of stack frames collected by a stack trace (whether generated by `new Error().stack` or `Error.captureStackTrace(obj)`). The default value is `10` but may be set to any valid JavaScript number. Changes will affect any stack trace captured _after_ the value has been changed. If set to a non-number value, or set to a negative number, stack traces will not capture any frames. | [`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`stackTraceLimit`](/sdk/reference/client/classes/Terminal49Error#property-stacktracelimit) | + +## Methods + +### captureStackTrace() + +> `static` **captureStackTrace**(`targetObject`, `constructorOpt?`): `void` + +Creates a `.stack` property on `targetObject`, which when accessed returns +a string representing the location in the code at which +`Error.captureStackTrace()` was called. + +```js +const myObject = {}; +Error.captureStackTrace(myObject); +myObject.stack; // Similar to `new Error().stack` +``` + +The first line of the trace will be prefixed with +`${myObject.name}: ${myObject.message}`. + +The optional `constructorOpt` argument accepts a function. If given, all frames +above `constructorOpt`, including `constructorOpt`, will be omitted from the +generated stack trace. + +The `constructorOpt` argument is useful for hiding implementation +details of error generation from the user. For instance: + +```js +function a() { + b(); +} + +function b() { + c(); +} + +function c() { + // Create an error without stack trace to avoid calculating the stack trace twice. + const { stackTraceLimit } = Error; + Error.stackTraceLimit = 0; + const error = new Error(); + Error.stackTraceLimit = stackTraceLimit; + + // Capture the stack trace above function b + Error.captureStackTrace(error, b); // Neither function c, nor b is included in the stack trace + throw error; +} + +a(); +``` + +#### Parameters + +| Parameter | Type | +| ------ | ------ | +| `targetObject` | `object` | +| `constructorOpt?` | `Function` | + +#### Returns + +`void` + +#### Inherited from + +[`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`captureStackTrace`](/sdk/reference/client/classes/Terminal49Error#capturestacktrace) + +*** + +### prepareStackTrace() + +> `static` **prepareStackTrace**(`err`, `stackTraces`): `any` + +#### Parameters + +| Parameter | Type | +| ------ | ------ | +| `err` | `Error` | +| `stackTraces` | `CallSite`[] | + +#### Returns + +`any` + +#### See + +https://v8.dev/docs/stack-trace-api#customizing-stack-traces + +#### Inherited from + +[`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`prepareStackTrace`](/sdk/reference/client/classes/Terminal49Error#preparestacktrace) diff --git a/docs/sdk/reference/client/classes/Terminal49Error.mdx b/docs/sdk/reference/client/classes/Terminal49Error.mdx index 17da9fe0..590913d2 100644 --- a/docs/sdk/reference/client/classes/Terminal49Error.mdx +++ b/docs/sdk/reference/client/classes/Terminal49Error.mdx @@ -15,8 +15,10 @@ Base error for all Terminal49 API errors. Subclassed by status-specific errors. - [`AuthenticationError`](/sdk/reference/client/classes/AuthenticationError) - [`AuthorizationError`](/sdk/reference/client/classes/AuthorizationError) +- [`NetworkError`](/sdk/reference/client/classes/NetworkError) - [`NotFoundError`](/sdk/reference/client/classes/NotFoundError) - [`RateLimitError`](/sdk/reference/client/classes/RateLimitError) +- [`TimeoutError`](/sdk/reference/client/classes/TimeoutError) - [`UpstreamError`](/sdk/reference/client/classes/UpstreamError) - [`ValidationError`](/sdk/reference/client/classes/ValidationError) diff --git a/docs/sdk/reference/client/classes/TimeoutError.mdx b/docs/sdk/reference/client/classes/TimeoutError.mdx new file mode 100644 index 00000000..cb5a1552 --- /dev/null +++ b/docs/sdk/reference/client/classes/TimeoutError.mdx @@ -0,0 +1,135 @@ +--- +title: "Class: TimeoutError" +--- + +# Class: TimeoutError + +Thrown when a request exceeds the configured request timeout and is aborted +by the SDK. Has no HTTP status because no response was produced. + +## Extends + +- [`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error) + +## Constructors + +### Constructor + +> **new TimeoutError**(`message?`, `details?`): `TimeoutError` + +#### Parameters + +| Parameter | Type | Default value | +| ------ | ------ | ------ | +| `message` | `string` | `'Request timed out'` | +| `details?` | `unknown` | `undefined` | + +#### Returns + +`TimeoutError` + +#### Overrides + +[`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`constructor`](/sdk/reference/client/classes/Terminal49Error#constructor) + +## Properties + +| Property | Modifier | Type | Description | Inherited from | +| ------ | ------ | ------ | ------ | ------ | +| `cause?` | `public` | `unknown` | - | [`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`cause`](/sdk/reference/client/classes/Terminal49Error#property-cause) | +| `details?` | `public` | `unknown` | - | [`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`details`](/sdk/reference/client/classes/Terminal49Error#property-details) | +| `message` | `public` | `string` | - | [`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`message`](/sdk/reference/client/classes/Terminal49Error#property-message) | +| `name` | `public` | `string` | - | [`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`name`](/sdk/reference/client/classes/Terminal49Error#property-name) | +| `stack?` | `public` | `string` | - | [`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`stack`](/sdk/reference/client/classes/Terminal49Error#property-stack) | +| `status?` | `public` | `number` | - | [`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`status`](/sdk/reference/client/classes/Terminal49Error#property-status) | +| `stackTraceLimit` | `static` | `number` | The `Error.stackTraceLimit` property specifies the number of stack frames collected by a stack trace (whether generated by `new Error().stack` or `Error.captureStackTrace(obj)`). The default value is `10` but may be set to any valid JavaScript number. Changes will affect any stack trace captured _after_ the value has been changed. If set to a non-number value, or set to a negative number, stack traces will not capture any frames. | [`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`stackTraceLimit`](/sdk/reference/client/classes/Terminal49Error#property-stacktracelimit) | + +## Methods + +### captureStackTrace() + +> `static` **captureStackTrace**(`targetObject`, `constructorOpt?`): `void` + +Creates a `.stack` property on `targetObject`, which when accessed returns +a string representing the location in the code at which +`Error.captureStackTrace()` was called. + +```js +const myObject = {}; +Error.captureStackTrace(myObject); +myObject.stack; // Similar to `new Error().stack` +``` + +The first line of the trace will be prefixed with +`${myObject.name}: ${myObject.message}`. + +The optional `constructorOpt` argument accepts a function. If given, all frames +above `constructorOpt`, including `constructorOpt`, will be omitted from the +generated stack trace. + +The `constructorOpt` argument is useful for hiding implementation +details of error generation from the user. For instance: + +```js +function a() { + b(); +} + +function b() { + c(); +} + +function c() { + // Create an error without stack trace to avoid calculating the stack trace twice. + const { stackTraceLimit } = Error; + Error.stackTraceLimit = 0; + const error = new Error(); + Error.stackTraceLimit = stackTraceLimit; + + // Capture the stack trace above function b + Error.captureStackTrace(error, b); // Neither function c, nor b is included in the stack trace + throw error; +} + +a(); +``` + +#### Parameters + +| Parameter | Type | +| ------ | ------ | +| `targetObject` | `object` | +| `constructorOpt?` | `Function` | + +#### Returns + +`void` + +#### Inherited from + +[`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`captureStackTrace`](/sdk/reference/client/classes/Terminal49Error#capturestacktrace) + +*** + +### prepareStackTrace() + +> `static` **prepareStackTrace**(`err`, `stackTraces`): `any` + +#### Parameters + +| Parameter | Type | +| ------ | ------ | +| `err` | `Error` | +| `stackTraces` | `CallSite`[] | + +#### Returns + +`any` + +#### See + +https://v8.dev/docs/stack-trace-api#customizing-stack-traces + +#### Inherited from + +[`Terminal49Error`](/sdk/reference/client/classes/Terminal49Error).[`prepareStackTrace`](/sdk/reference/client/classes/Terminal49Error#preparestacktrace) diff --git a/docs/sdk/reference/client/index.mdx b/docs/sdk/reference/client/index.mdx index 449e5335..76b0d48c 100644 --- a/docs/sdk/reference/client/index.mdx +++ b/docs/sdk/reference/client/index.mdx @@ -12,10 +12,12 @@ description: "Reference index for the Terminal49 TypeScript SDK client module, l | [AuthenticationError](/sdk/reference/client/classes/AuthenticationError) | Thrown when the API token is invalid or missing (HTTP 401). | | [AuthorizationError](/sdk/reference/client/classes/AuthorizationError) | Thrown when the API token is valid but lacks permission for the request (HTTP 403). | | [FeatureNotEnabledError](/sdk/reference/client/classes/FeatureNotEnabledError) | Thrown when the requested feature requires a plan upgrade (HTTP 403). | +| [NetworkError](/sdk/reference/client/classes/NetworkError) | Thrown when a transport-level failure occurs before a response is received — a DNS failure, a refused/reset connection, or an otherwise failed `fetch`. Has no HTTP status because no response was produced. | | [NotFoundError](/sdk/reference/client/classes/NotFoundError) | Thrown when the requested resource does not exist (HTTP 404). | | [RateLimitError](/sdk/reference/client/classes/RateLimitError) | Thrown when the API rate limit has been exceeded (HTTP 429). The SDK retries automatically. | | [Terminal49Client](/sdk/reference/client/classes/Terminal49Client) | Server-side TypeScript client for the Terminal49 JSON:API. | | [Terminal49Error](/sdk/reference/client/classes/Terminal49Error) | Base error for all Terminal49 API errors. Subclassed by status-specific errors. | +| [TimeoutError](/sdk/reference/client/classes/TimeoutError) | Thrown when a request exceeds the configured request timeout and is aborted by the SDK. Has no HTTP status because no response was produced. | | [UpstreamError](/sdk/reference/client/classes/UpstreamError) | Thrown when the carrier or terminal upstream API is unavailable (HTTP 5xx). | | [ValidationError](/sdk/reference/client/classes/ValidationError) | Thrown when the request payload fails server-side validation (HTTP 400/422). | diff --git a/docs/sdk/reference/client/interceptors/classes/RetryInterceptor.mdx b/docs/sdk/reference/client/interceptors/classes/RetryInterceptor.mdx index 88b8e542..9ea2e835 100644 --- a/docs/sdk/reference/client/interceptors/classes/RetryInterceptor.mdx +++ b/docs/sdk/reference/client/interceptors/classes/RetryInterceptor.mdx @@ -5,6 +5,16 @@ description: "RetryInterceptor class in the Terminal49 TypeScript SDK, automatic # Class: RetryInterceptor +Retries transient failures with backoff. Two kinds of failure are handled: + + - A response with a retryable status (429 / 5xx), handled in `onResponse`. + - A thrown transport error (DNS/connection/"fetch failed"), handled in + `onError` — these never reach `onResponse` because `fetch` rejected. + +Retries are gated by shouldRetryRequest: idempotent methods are always +eligible, but non-idempotent writes are only retried when the caller supplied +an `Idempotency-Key` header. 429 backoff honors the server's `Retry-After`. + ## Constructors ### Constructor @@ -26,7 +36,12 @@ description: "RetryInterceptor class in the Terminal49 TypeScript SDK, automatic ### onError() -> **onError**(`__namedParameters`): `void` +> **onError**(`__namedParameters`): `Promise`\<`Error` \| `Response`\> + +Recover from a thrown transport error by retrying eligible requests. If a +retry produces a response we return it (openapi-fetch then runs the normal +`onResponse` chain on it); otherwise we surface a normalized +NetworkError so error mapping is consistent with the response path. #### Parameters @@ -36,7 +51,7 @@ description: "RetryInterceptor class in the Terminal49 TypeScript SDK, automatic #### Returns -`void` +`Promise`\<`Error` \| `Response`\> *** diff --git a/docs/sdk/reference/client/interceptors/index.mdx b/docs/sdk/reference/client/interceptors/index.mdx index ee478b90..f5a15ece 100644 --- a/docs/sdk/reference/client/interceptors/index.mdx +++ b/docs/sdk/reference/client/interceptors/index.mdx @@ -11,7 +11,7 @@ description: "Reference for the Terminal49 TypeScript SDK interceptors module, c | ------ | ------ | | [AuthInterceptor](/sdk/reference/client/interceptors/classes/AuthInterceptor) | - | | [ErrorMappingInterceptor](/sdk/reference/client/interceptors/classes/ErrorMappingInterceptor) | - | -| [RetryInterceptor](/sdk/reference/client/interceptors/classes/RetryInterceptor) | - | +| [RetryInterceptor](/sdk/reference/client/interceptors/classes/RetryInterceptor) | Retries transient failures with backoff. Two kinds of failure are handled: | ## Type Aliases diff --git a/docs/sdk/reference/client/interfaces/Terminal49ClientConfig.mdx b/docs/sdk/reference/client/interfaces/Terminal49ClientConfig.mdx index 1f24ed03..9de68a4c 100644 --- a/docs/sdk/reference/client/interfaces/Terminal49ClientConfig.mdx +++ b/docs/sdk/reference/client/interfaces/Terminal49ClientConfig.mdx @@ -17,3 +17,4 @@ Configuration for [Terminal49Client](/sdk/reference/client/classes/Terminal49Cli | `defaultFormat?` | [`ResponseFormat`](/sdk/reference/types/options/type-aliases/ResponseFormat) | Default response format for methods that support mapped responses. Defaults to `raw`. | | `fetchImpl?` | (`input`, `init?`) => `Promise`\<`Response`\> | Optional fetch implementation, useful for tests or custom runtimes. | | `maxRetries?` | `number` | Number of retry attempts for rate-limit and server errors. Defaults to `2`. | +| `timeoutMs?` | `number` | Per-request timeout in milliseconds. A hung request is aborted once it elapses and rejected with a `TimeoutError`. Defaults to `30000`. Set to `0` to disable the timeout. | diff --git a/docs/sdk/reference/client/managers/index.mdx b/docs/sdk/reference/client/managers/index.mdx index 7b15480f..2dda5a89 100644 --- a/docs/sdk/reference/client/managers/index.mdx +++ b/docs/sdk/reference/client/managers/index.mdx @@ -20,6 +20,7 @@ description: "Reference for the Terminal49 TypeScript SDK managers module: conta | Interface | Description | | ------ | ------ | | [CreateTrackingRequestFromInferOptions](/sdk/reference/client/managers/interfaces/CreateTrackingRequestFromInferOptions) | - | +| [IterateOptions](/sdk/reference/client/managers/interfaces/IterateOptions) | Options accepted by BaseManager.createIterator. | | [TrackingRequestListFilters](/sdk/reference/client/managers/interfaces/TrackingRequestListFilters) | - | ## Type Aliases @@ -27,3 +28,10 @@ description: "Reference for the Terminal49 TypeScript SDK managers module: conta | Type Alias | Description | | ------ | ------ | | [TrackingRequestType](/sdk/reference/client/managers/type-aliases/TrackingRequestType) | - | + +## Variables + +| Variable | Description | +| ------ | ------ | +| [DEFAULT\_ITERATE\_MAX\_PAGES](/sdk/reference/client/managers/variables/DEFAULT_ITERATE_MAX_PAGES) | Hard safety caps for BaseManager.createIterator. They exist so a no-op or mistakenly broad filter cannot silently walk the entire dataset (and make thousands of requests). They are deliberately large enough not to interfere with realistic pagination, and can be raised per call via `maxPages` / `maxRows` when a caller genuinely needs more. | +| [DEFAULT\_ITERATE\_MAX\_ROWS](/sdk/reference/client/managers/variables/DEFAULT_ITERATE_MAX_ROWS) | - | diff --git a/docs/sdk/reference/client/managers/interfaces/IterateOptions.mdx b/docs/sdk/reference/client/managers/interfaces/IterateOptions.mdx new file mode 100644 index 00000000..f11a8f45 --- /dev/null +++ b/docs/sdk/reference/client/managers/interfaces/IterateOptions.mdx @@ -0,0 +1,15 @@ +--- +title: "Interface: IterateOptions" +--- + +# Interface: IterateOptions + +Options accepted by BaseManager.createIterator. + +## Properties + +| Property | Type | Description | +| ------ | ------ | ------ | +| `maxPages?` | `number` | Maximum number of pages to fetch. Defaults to [DEFAULT\_ITERATE\_MAX\_PAGES](/sdk/reference/client/managers/variables/DEFAULT_ITERATE_MAX_PAGES). | +| `maxRows?` | `number` | Maximum number of rows to yield. Defaults to [DEFAULT\_ITERATE\_MAX\_ROWS](/sdk/reference/client/managers/variables/DEFAULT_ITERATE_MAX_ROWS). | +| `pageSize?` | `number` | Records per page passed to the underlying list call. | diff --git a/docs/sdk/reference/client/managers/variables/DEFAULT_ITERATE_MAX_PAGES.mdx b/docs/sdk/reference/client/managers/variables/DEFAULT_ITERATE_MAX_PAGES.mdx new file mode 100644 index 00000000..a2041393 --- /dev/null +++ b/docs/sdk/reference/client/managers/variables/DEFAULT_ITERATE_MAX_PAGES.mdx @@ -0,0 +1,13 @@ +--- +title: "Variable: DEFAULT\\_ITERATE\\_MAX\\_PAGES" +--- + +# Variable: DEFAULT\_ITERATE\_MAX\_PAGES + +> `const` **DEFAULT\_ITERATE\_MAX\_PAGES**: `1000` = `1000` + +Hard safety caps for BaseManager.createIterator. They exist so a no-op +or mistakenly broad filter cannot silently walk the entire dataset (and make +thousands of requests). They are deliberately large enough not to interfere +with realistic pagination, and can be raised per call via +`maxPages` / `maxRows` when a caller genuinely needs more. diff --git a/docs/sdk/reference/client/managers/variables/DEFAULT_ITERATE_MAX_ROWS.mdx b/docs/sdk/reference/client/managers/variables/DEFAULT_ITERATE_MAX_ROWS.mdx new file mode 100644 index 00000000..6c5bf21f --- /dev/null +++ b/docs/sdk/reference/client/managers/variables/DEFAULT_ITERATE_MAX_ROWS.mdx @@ -0,0 +1,7 @@ +--- +title: "Variable: DEFAULT\\_ITERATE\\_MAX\\_ROWS" +--- + +# Variable: DEFAULT\_ITERATE\_MAX\_ROWS + +> `const` **DEFAULT\_ITERATE\_MAX\_ROWS**: `100000` = `100_000` diff --git a/docs/sdk/reference/client/transport/classes/Transport.mdx b/docs/sdk/reference/client/transport/classes/Transport.mdx index fa20463d..ca7090a4 100644 --- a/docs/sdk/reference/client/transport/classes/Transport.mdx +++ b/docs/sdk/reference/client/transport/classes/Transport.mdx @@ -56,6 +56,12 @@ description: "Transport class in the Terminal49 TypeScript SDK, the low-level HT > **executeManual**\<`T`\>(`input`, `init?`): `Promise`\<`T`\> +Run a request that has no entry in the generated OpenAPI types (currently +only `search()`) through the same Auth -> Retry -> ErrorMapping pipeline the +typed client uses, including the timeout-wrapped fetch. Successful bodies are +read with readSuccessBody so a non-JSON success body is surfaced +rather than silently collapsed to `undefined`. + #### Type Parameters | Type Parameter | Default type | diff --git a/docs/sdk/reference/client/transport/index.mdx b/docs/sdk/reference/client/transport/index.mdx index 9ab74284..1a0b0e31 100644 --- a/docs/sdk/reference/client/transport/index.mdx +++ b/docs/sdk/reference/client/transport/index.mdx @@ -22,3 +22,9 @@ description: "Reference for the Terminal49 TypeScript SDK transport module: the | Type Alias | Description | | ------ | ------ | | [ApiClient](/sdk/reference/client/transport/type-aliases/ApiClient) | - | + +## Variables + +| Variable | Description | +| ------ | ------ | +| [DEFAULT\_REQUEST\_TIMEOUT\_MS](/sdk/reference/client/transport/variables/DEFAULT_REQUEST_TIMEOUT_MS) | Default per-request timeout (ms) applied when the caller does not override it. | diff --git a/docs/sdk/reference/client/transport/interfaces/TransportConfig.mdx b/docs/sdk/reference/client/transport/interfaces/TransportConfig.mdx index 4299024e..182306c6 100644 --- a/docs/sdk/reference/client/transport/interfaces/TransportConfig.mdx +++ b/docs/sdk/reference/client/transport/interfaces/TransportConfig.mdx @@ -7,10 +7,11 @@ description: "TransportConfig interface in the Terminal49 TypeScript SDK, config ## Properties -| Property | Type | -| ------ | ------ | -| `accountId?` | `string` | -| `apiToken` | `string` | -| `baseUrl` | `string` | -| `fetchImpl?` | (`input`, `init?`) => `Promise`\<`Response`\> | -| `maxRetries?` | `number` | +| Property | Type | Description | +| ------ | ------ | ------ | +| `accountId?` | `string` | - | +| `apiToken` | `string` | - | +| `baseUrl` | `string` | - | +| `fetchImpl?` | (`input`, `init?`) => `Promise`\<`Response`\> | - | +| `maxRetries?` | `number` | - | +| `timeoutMs?` | `number` | Per-request timeout in milliseconds. Defaults to [DEFAULT\_REQUEST\_TIMEOUT\_MS](/sdk/reference/client/transport/variables/DEFAULT_REQUEST_TIMEOUT_MS). Set to `0` to disable the timeout. | diff --git a/docs/sdk/reference/client/transport/variables/DEFAULT_REQUEST_TIMEOUT_MS.mdx b/docs/sdk/reference/client/transport/variables/DEFAULT_REQUEST_TIMEOUT_MS.mdx new file mode 100644 index 00000000..0abd41a4 --- /dev/null +++ b/docs/sdk/reference/client/transport/variables/DEFAULT_REQUEST_TIMEOUT_MS.mdx @@ -0,0 +1,9 @@ +--- +title: "Variable: DEFAULT\\_REQUEST\\_TIMEOUT\\_MS" +--- + +# Variable: DEFAULT\_REQUEST\_TIMEOUT\_MS + +> `const` **DEFAULT\_REQUEST\_TIMEOUT\_MS**: `30000` = `30_000` + +Default per-request timeout (ms) applied when the caller does not override it. From 58ddf6f37af8e9dd793bbe38c1730116906f1d78 Mon Sep 17 00:00:00 2001 From: Akshay Dodeja Date: Thu, 25 Jun 2026 18:31:16 -0700 Subject: [PATCH 3/4] =?UTF-8?q?fix(sdk):=20address=20PR=20review=20?= =?UTF-8?q?=E2=80=94=20bounded=20iterate=20caps,=20retry=20hardening,=20ma?= =?UTF-8?q?nual-path=20error=20normalization?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on the transport-resilience PR: - Expose `maxPages`/`maxRows` on the public `iterate()` signatures by adding them to `ListOptions`, so TypeScript callers can raise the iterator safety caps with types (previously type-unreachable). Regenerated SDK reference docs for the new `ListOptions` properties. - Normalize a network error thrown from a retry kicked off inside `RetryInterceptor.onResponse` in the manual `/search` path: wrap the `retry.onResponse` call in `executeManual` so the caller always sees a `NetworkError` rather than a raw `TypeError`. - Cap a honored `Retry-After` delay at 60s (`MAX_RETRY_AFTER_MS`) so an adversarial/misbehaving upstream cannot wedge the caller in a multi-hour sleep (the request timeout guards `fetch`, not the backoff sleep). - Tighten network-error detection: a bare `TypeError` is no longer treated as retryable unless its message looks network-ish, so a programming-bug `TypeError` is not retried up to `maxRetries` and masked. - Document why `search()` uses `executeManual` (no `/search` entry in the generated OpenAPI types, so it cannot route through the typed client). Added unit tests for the Retry-After cap and the tightened TypeError guard. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../types/options/interfaces/ListOptions.mdx | 2 ++ sdks/typescript-sdk/src/client.ts | 3 +++ .../src/client/retry-policy.test.ts | 20 +++++++++++++++++ .../typescript-sdk/src/client/retry-policy.ts | 22 +++++++++++++------ sdks/typescript-sdk/src/client/transport.ts | 18 ++++++++++----- sdks/typescript-sdk/src/types/options.ts | 10 +++++++++ 6 files changed, 63 insertions(+), 12 deletions(-) diff --git a/docs/sdk/reference/types/options/interfaces/ListOptions.mdx b/docs/sdk/reference/types/options/interfaces/ListOptions.mdx index 1f789ca6..cd59f196 100644 --- a/docs/sdk/reference/types/options/interfaces/ListOptions.mdx +++ b/docs/sdk/reference/types/options/interfaces/ListOptions.mdx @@ -16,5 +16,7 @@ Per-call options accepted by list methods. | Property | Type | Description | Inherited from | | ------ | ------ | ------ | ------ | | `format?` | [`ResponseFormat`](/sdk/reference/types/options/type-aliases/ResponseFormat) | Override the client's default response format for this call. | [`CallOptions`](/sdk/reference/types/options/interfaces/CallOptions).[`format`](/sdk/reference/types/options/interfaces/CallOptions#property-format) | +| `maxPages?` | `number` | Maximum number of pages `iterate()` will fetch before stopping. Defaults to the manager's safety cap; raise it to walk past the default bound. | - | +| `maxRows?` | `number` | Maximum number of rows `iterate()` will yield before stopping. Defaults to the manager's safety cap; raise it to walk past the default bound. | - | | `page?` | `number` | 1-based page number. | - | | `pageSize?` | `number` | Number of records per page. | - | diff --git a/sdks/typescript-sdk/src/client.ts b/sdks/typescript-sdk/src/client.ts index f5b740be..a8309cbf 100644 --- a/sdks/typescript-sdk/src/client.ts +++ b/sdks/typescript-sdk/src/client.ts @@ -149,6 +149,9 @@ export class Terminal49Client { /** Search across shipments and containers by number, reference, or keyword. */ async search(query: string): Promise { const params = new URLSearchParams({ query }); + // `/search` is not present in the generated OpenAPI types, so it cannot be + // routed through the typed `client.GET(...)`. `executeManual` runs it through + // the same Auth -> Retry -> ErrorMapping -> timeout pipeline as the typed client. return this.transport.executeManual( `${this.transport.baseUrl}/search?${params.toString()}`, ); diff --git a/sdks/typescript-sdk/src/client/retry-policy.test.ts b/sdks/typescript-sdk/src/client/retry-policy.test.ts index 7f7f0a24..a36959de 100644 --- a/sdks/typescript-sdk/src/client/retry-policy.test.ts +++ b/sdks/typescript-sdk/src/client/retry-policy.test.ts @@ -51,6 +51,15 @@ describe('retry-policy', () => { const abort = Object.assign(new Error('aborted'), { name: 'AbortError' }); expect(isRetryableNetworkError(abort)).toBe(false); }); + + it('does not retry a programming-bug TypeError with a non-network message', () => { + // Masking real bugs by retrying them maxRetries times is worse than failing fast. + expect( + isRetryableNetworkError( + new TypeError("Cannot read properties of undefined (reading 'x')"), + ), + ).toBe(false); + }); }); describe('shouldRetryRequest', () => { @@ -94,6 +103,17 @@ describe('retry-policy', () => { const past = new Date(now - 5000).toUTCString(); expect(parseRetryAfterMs(past, now)).toBe(0); }); + + it('clamps an excessive delta-seconds value to the 60s cap', () => { + // A 24h Retry-After must not wedge the caller in a multi-hour sleep. + expect(parseRetryAfterMs('86400')).toBe(60_000); + }); + + it('clamps a far-future HTTP-date to the 60s cap', () => { + const now = Date.parse('2026-01-01T00:00:00Z'); + const farFuture = new Date(now + 86_400_000).toUTCString(); + expect(parseRetryAfterMs(farFuture, now)).toBe(60_000); + }); }); describe('computeBackoffDelay', () => { diff --git a/sdks/typescript-sdk/src/client/retry-policy.ts b/sdks/typescript-sdk/src/client/retry-policy.ts index 7863d377..72b8f093 100644 --- a/sdks/typescript-sdk/src/client/retry-policy.ts +++ b/sdks/typescript-sdk/src/client/retry-policy.ts @@ -49,9 +49,9 @@ export function isRetryableNetworkError(error: unknown): boolean { } // undici/whatwg surface generic connection failures as a TypeError whose - // message is "fetch failed" (often with a `cause`). - if (error instanceof TypeError) return true; - + // message is "fetch failed" (often with a `cause`). We require a network-ish + // message so a programming-bug TypeError ("Cannot read property 'x' ...") is + // NOT retried up to maxRetries and masked. const message = typeof err.message === 'string' ? err.message : ''; return /fetch failed|network|socket hang up|terminated/i.test(message); } @@ -73,10 +73,18 @@ export function shouldRetryRequest(ctx: RetryRequestContext): boolean { return ctx.hasIdempotencyKey === true; } +/** + * Upper bound (ms) on a honored `Retry-After` value. A misbehaving or adversarial + * upstream returning e.g. `Retry-After: 86400` must not be able to wedge the + * caller in a multi-hour `sleep` (the request `timeoutMs` guards individual + * `fetch` calls, not the backoff sleep), so the delay is clamped to this cap. + */ +const MAX_RETRY_AFTER_MS = 60_000; + /** * Parse a `Retry-After` header into milliseconds. Supports both delta-seconds - * (`"120"`) and an HTTP-date. Returns `undefined` when absent/unparseable, and - * never returns a negative delay. + * (`"120"`) and an HTTP-date. Returns `undefined` when absent/unparseable, never + * returns a negative delay, and clamps the result to {@link MAX_RETRY_AFTER_MS}. * * @param now - Reference time (ms since epoch) used for HTTP-date math; defaults * to `Date.now()` and is injectable for deterministic tests. @@ -90,12 +98,12 @@ export function parseRetryAfterMs( if (trimmed === '') return undefined; if (/^\d+$/.test(trimmed)) { - return Number(trimmed) * 1000; + return Math.min(Number(trimmed) * 1000, MAX_RETRY_AFTER_MS); } const dateMs = Date.parse(trimmed); if (Number.isNaN(dateMs)) return undefined; - return Math.max(0, dateMs - now); + return Math.min(Math.max(0, dateMs - now), MAX_RETRY_AFTER_MS); } /** Base unit (ms) for exponential backoff: attempt 0 -> 500ms, 1 -> 1000ms, ... */ diff --git a/sdks/typescript-sdk/src/client/transport.ts b/sdks/typescript-sdk/src/client/transport.ts index 3055638e..6be66318 100644 --- a/sdks/typescript-sdk/src/client/transport.ts +++ b/sdks/typescript-sdk/src/client/transport.ts @@ -119,11 +119,19 @@ export class Transport { } } - res = await retry.onResponse({ - request: retryableReq, - response: res, - id: retryContext.id, - }); + try { + res = await retry.onResponse({ + request: retryableReq, + response: res, + id: retryContext.id, + }); + } catch (error) { + // A retry kicked off inside onResponse (after an initial 5xx) can itself + // throw a fetch error; normalize it like the initial-fetch path so the + // caller always sees a NetworkError rather than a raw TypeError. + // (toNetworkError passes an already-normalized Terminal49Error through.) + throw toNetworkError(error); + } await errorMap.onResponse({ request: retryableReq, response: res, diff --git a/sdks/typescript-sdk/src/types/options.ts b/sdks/typescript-sdk/src/types/options.ts index e112c4d7..ba0469cc 100644 --- a/sdks/typescript-sdk/src/types/options.ts +++ b/sdks/typescript-sdk/src/types/options.ts @@ -13,6 +13,16 @@ export interface ListOptions extends CallOptions { page?: number; /** Number of records per page. */ pageSize?: number; + /** + * Maximum number of pages `iterate()` will fetch before stopping. Defaults to + * the manager's safety cap; raise it to walk past the default bound. + */ + maxPages?: number; + /** + * Maximum number of rows `iterate()` will yield before stopping. Defaults to + * the manager's safety cap; raise it to walk past the default bound. + */ + maxRows?: number; } export type IncludeParam = From dcfd05df632df89a9d7f9b1bc7ce260ddd9d9492 Mon Sep 17 00:00:00 2001 From: Akshay Dodeja Date: Fri, 26 Jun 2026 05:31:14 -0500 Subject: [PATCH 4/4] fix(sdk): normalize response-retry network errors and preserve replay state across onError->onResponse Two transport-resilience gaps in the typed-client retry path remained after the prior review pass: - A network error thrown by a response-triggered retry inside `RetryInterceptor.onResponse` propagated as a raw `TypeError`. openapi-fetch does not route an `onResponse` throw back through `onError`, so the caller saw an un-normalized error instead of a `NetworkError`. Wrap the retry `fetch` and normalize via `toNetworkError`, matching the initial-fetch path. - `onError` deleted the replay entry in its `finally` even when it returned a recovered Response. openapi-fetch then runs `onResponse` for that same request id, but with the replay state gone it could not retry a subsequent 429/5xx, so a transient network-failure -> 500 -> success sequence failed despite remaining retry budget. Only delete the replay entry on the terminal error paths; the `onResponse` chain deletes it once the response is settled. Added transport tests for both paths (verified red against the prior code). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/client.transport.test.ts | 77 ++++++++++++++++++- .../typescript-sdk/src/client/interceptors.ts | 53 ++++++++----- 2 files changed, 105 insertions(+), 25 deletions(-) diff --git a/sdks/typescript-sdk/src/client.transport.test.ts b/sdks/typescript-sdk/src/client.transport.test.ts index 6813ec93..2f0fab64 100644 --- a/sdks/typescript-sdk/src/client.transport.test.ts +++ b/sdks/typescript-sdk/src/client.transport.test.ts @@ -67,6 +67,74 @@ describe('Terminal49Client transport resilience', () => { } }); + it('normalizes a network error thrown by a response-triggered retry', async () => { + // First response is a retryable 5xx; the retry fetch then throws a raw + // network error. openapi-fetch does not route an onResponse throw back + // through onError, so without normalization the caller would see a bare + // TypeError instead of a Terminal49Error. + vi.useFakeTimers(); + try { + let attempt = 0; + const fetchImpl = vi.fn(async () => { + attempt += 1; + if (attempt === 1) { + return jsonResponse({ errors: [{ detail: 'server error' }] }, 503); + } + throw new TypeError('fetch failed'); + }); + + const client = new Terminal49Client({ + apiToken: 'token-123', + apiBaseUrl: baseUrl, + fetchImpl: fetchImpl as unknown as typeof fetch, + maxRetries: 1, + }); + + const resultPromise = client.getContainerRoute('abc'); + const assertion = + expect(resultPromise).rejects.toBeInstanceOf(Terminal49Error); + await vi.advanceTimersByTimeAsync(500); + await assertion; + expect(attempt).toBe(2); + } finally { + vi.useRealTimers(); + } + }); + + it('still retries a 5xx after recovering from an initial network error', async () => { + // A transient sequence of network failure -> 500 -> success must succeed: + // recovering from the thrown error in onError must not discard the replay + // state that onResponse needs to retry the subsequent 500. + vi.useFakeTimers(); + try { + let attempt = 0; + const fetchImpl = vi.fn(async () => { + attempt += 1; + if (attempt === 1) throw new TypeError('fetch failed'); + if (attempt === 2) { + return jsonResponse({ errors: [{ detail: 'server error' }] }, 500); + } + return jsonResponse({ data: { id: 'route-1' } }); + }); + + const client = new Terminal49Client({ + apiToken: 'token-123', + apiBaseUrl: baseUrl, + fetchImpl: fetchImpl as unknown as typeof fetch, + maxRetries: 2, + }); + + const resultPromise = client.getContainerRoute('abc'); + await vi.advanceTimersByTimeAsync(2000); + const result = await resultPromise; + + expect(result.data.id).toBe('route-1'); + expect(attempt).toBe(3); + } finally { + vi.useRealTimers(); + } + }); + it('waits for the Retry-After header before retrying a 429', async () => { vi.useFakeTimers(); try { @@ -187,10 +255,11 @@ describe('Terminal49Client transport resilience', () => { }); const items = []; - for await (const shipment of client.shipments.iterate({}, { - pageSize: 1, - maxPages: 3, - } as { pageSize?: number; maxPages?: number })) { + // `maxPages` is type-reachable via ListOptions, so no cast is needed. + for await (const shipment of client.shipments.iterate( + {}, + { pageSize: 1, maxPages: 3 }, + )) { items.push(shipment); } diff --git a/sdks/typescript-sdk/src/client/interceptors.ts b/sdks/typescript-sdk/src/client/interceptors.ts index 19494f67..067dd994 100644 --- a/sdks/typescript-sdk/src/client/interceptors.ts +++ b/sdks/typescript-sdk/src/client/interceptors.ts @@ -95,7 +95,16 @@ export class RetryInterceptor { ); await this.sleep(computeBackoffDelay(attempt, retryAfterMs)); - currentResponse = await this.fetchImpl(replayableRequest.clone()); + try { + currentResponse = await this.fetchImpl(replayableRequest.clone()); + } catch (error) { + // openapi-fetch does NOT route a throw from `onResponse` back through + // `onError`, so a network failure during a response-triggered retry + // would otherwise surface as a raw TypeError. Normalize it to a + // NetworkError so the caller sees the same error shape as the + // initial-fetch path. + throw toNetworkError(error); + } attempt++; } @@ -121,30 +130,32 @@ export class RetryInterceptor { const requestKey = this.requestKey(request, id); const replayableRequest = this.replayableRequests.get(requestKey); - try { - if ( - replayableRequest && - isRetryableNetworkError(error) && - this.isRetryable(replayableRequest) - ) { - let attempt = 0; - while (attempt < this.maxRetries) { - await this.sleep(computeBackoffDelay(attempt)); - try { - return await this.fetchImpl(replayableRequest.clone()); - } catch (retryError) { - attempt++; - if (attempt >= this.maxRetries) { - return toNetworkError(retryError); - } + if ( + replayableRequest && + isRetryableNetworkError(error) && + this.isRetryable(replayableRequest) + ) { + let attempt = 0; + while (attempt < this.maxRetries) { + await this.sleep(computeBackoffDelay(attempt)); + try { + // A recovered Response is handed back to openapi-fetch, which then + // runs `onResponse` for this same request id. Keep the replay entry + // so that path can still retry a subsequent 429/5xx — `onResponse` + // deletes it once the response chain finishes. + return await this.fetchImpl(replayableRequest.clone()); + } catch (retryError) { + attempt++; + if (attempt >= this.maxRetries) { + this.replayableRequests.delete(requestKey); + return toNetworkError(retryError); } } } - - return toNetworkError(error); - } finally { - this.replayableRequests.delete(requestKey); } + + this.replayableRequests.delete(requestKey); + return toNetworkError(error); } private isRetryable(request: Request): boolean {