diff --git a/.changeset/connect-timeout-ts.md b/.changeset/connect-timeout-ts.md new file mode 100644 index 0000000..2493dde --- /dev/null +++ b/.changeset/connect-timeout-ts.md @@ -0,0 +1,5 @@ +--- +'@smooai/fetch': minor +--- + +Add an optional connect timeout (`connectTimeoutMs` / `FetchBuilder.withConnectTimeout`) that bounds only the connection-establishment phase, so a black-holed connect (e.g. a SYN to a dead pod IP still lingering in a ClusterIP's iptables) fails fast and retry can land on a live endpoint, instead of stalling until the whole-request timeout. Node only (via a lazily-loaded undici `Agent` dispatcher); ignored in browser/worker builds. Default-off — unset preserves the previous behavior exactly. Parity with the Rust `with_connect_timeout` (SmooAI/fetch#88, SMOODEV-2513). diff --git a/package.json b/package.json index 8a3d973..383b41e 100644 --- a/package.json +++ b/package.json @@ -127,7 +127,8 @@ "@standard-schema/spec": "^1.0.0", "@standard-schema/utils": "^0.3.0", "lodash.merge": "^4.6.2", - "mollitia": "^0.1.1" + "mollitia": "^0.1.1", + "undici": "^6" }, "devDependencies": { "@changesets/cli": "^2.28.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c7feb9f..4ea7b65 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -30,6 +30,9 @@ importers: mollitia: specifier: ^0.1.1 version: 0.1.1 + undici: + specifier: ^6 + version: 6.27.0 devDependencies: '@changesets/cli': specifier: ^2.28.1 @@ -2282,6 +2285,10 @@ packages: undici-types@6.20.0: resolution: {integrity: sha512-Ny6QZ2Nju20vw1SRHe3d9jVu6gJ+4e3+MMpqu7pqE5HT6WsTSlce++GQmK5UXS8mzV8DSYHrQH+Xrf2jVcuKNg==} + undici@6.27.0: + resolution: {integrity: sha512-YmfV3YnEDzXRC5lZ2jWtWWHKGUm1zIt8AhesR1tens+HTNv+YZlN/dp6G727LOvMJ8xjP9Be7Y2Sdr96LDm+pg==} + engines: {node: '>=18.17'} + unicorn-magic@0.1.0: resolution: {integrity: sha512-lRfVq8fE8gz6QMBuDM6a+LO3IAzTi05H6gCVaUpir2E1Rwpo4ZUog45KpNXKC/Mn3Yb9UDuHumeFTo9iV/D9FQ==} engines: {node: '>=18'} @@ -4860,6 +4867,8 @@ snapshots: undici-types@6.20.0: {} + undici@6.27.0: {} + unicorn-magic@0.1.0: {} universalify@0.1.2: {} diff --git a/src/fetch.connect-timeout.spec.ts b/src/fetch.connect-timeout.spec.ts new file mode 100644 index 0000000..16a5b2b --- /dev/null +++ b/src/fetch.connect-timeout.spec.ts @@ -0,0 +1,61 @@ +import { TimeoutError } from 'mollitia'; +import { describe, expect, test } from 'vitest'; +import fetch, { FetchBuilder } from './fetch'; + +/** + * A bounded connect timeout must fail fast on a black-holed connect instead of + * stalling until the (much larger) whole-request timeout. + * + * Regression coverage mirroring the Rust port (SMOODEV-2513 / SMOODEV-2498): + * api-prime's ~16s stalls were fresh SYNs to dead pod IPs still lingering in a + * ClusterIP's iptables. Without a connect timeout, undici waits the full + * whole-request timeout; with one, the connect fails in ~the configured window + * and retry can land on a live pod. + * + * 10.255.255.1 is a non-routable RFC1918 address with (almost certainly) no host + * answering: the SYN is dropped/black-holed so the connect never establishes. + */ +const BLACK_HOLE_URL = 'http://10.255.255.1:80/anything'; + +describe('connect timeout', () => { + test('fails fast on a black-holed connect, well under the whole-request timeout', async () => { + const start = Date.now(); + // Whole-request timeout is 10x the connect timeout — if the connect timeout + // is NOT honored, this would run ~5s and blow the elapsed assertion. + // Retry disabled so the connect window isn't multiplied across attempts. + const promise = fetch(BLACK_HOLE_URL, { + options: { + connectTimeoutMs: 500, + timeout: { timeoutMs: 5000 }, + retry: { attempts: 0, initialIntervalMs: 0 }, + }, + }); + + await expect(promise).rejects.toThrow(); + // A connect timeout surfaces as an undici request error, NOT the mollitia + // whole-request TimeoutError (which would mean the connect timeout never fired). + await expect(promise).rejects.not.toBeInstanceOf(TimeoutError); + + const elapsed = Date.now() - start; + expect(elapsed).toBeLessThan(3000); + }, 10000); + + test('FetchBuilder.withConnectTimeout wires the same fast-fail path', async () => { + const client = new FetchBuilder().withConnectTimeout(500).withTimeout(5000).withRetry({ attempts: 0, initialIntervalMs: 0 }).build(); + + const start = Date.now(); + await expect(client(BLACK_HOLE_URL)).rejects.toThrow(); + expect(Date.now() - start).toBeLessThan(3000); + }, 10000); + + test('unset connect timeout is behavior-identical (still reaches the black hole and fails)', async () => { + // Without a connect timeout the connect still fails on a black hole, just + // bounded by the whole-request timeout instead. Proves the default path + // does not attach a dispatcher / change behavior. + await expect( + fetch(BLACK_HOLE_URL, { + options: { timeout: { timeoutMs: 800 }, retry: { attempts: 0, initialIntervalMs: 0 } }, + }), + ).rejects.toThrow(); + }, 10000); +}); diff --git a/src/fetch.ts b/src/fetch.ts index a87c0b9..50d78cf 100644 --- a/src/fetch.ts +++ b/src/fetch.ts @@ -52,6 +52,32 @@ const globalFetch = (): typeof global.fetch => { throw new Error('No fetch implementation found. ' + 'Please ensure fetch is available in your environment.'); }; +/** + * Cache of undici `Agent` dispatchers keyed by connect-timeout ms, so we build + * one Agent per distinct timeout instead of per request. + */ +const connectTimeoutDispatchers = new Map(); + +/** + * Resolve an undici `Agent` dispatcher that bounds the connect phase to + * `connectTimeoutMs`. Node only: browsers/Web Workers have no connect-timeout + * knob, so this returns `undefined` there (and no `dispatcher` is passed, making + * behavior byte-identical to today). undici is imported lazily so it is never + * loaded unless a connect timeout is actually requested. + */ +async function getConnectTimeoutDispatcher(connectTimeoutMs: number): Promise { + // Only Node's fetch (undici) honors a `dispatcher`. Bail everywhere else. + if (typeof window !== 'undefined' || typeof process === 'undefined' || !process.versions?.node) { + return undefined; + } + const cached = connectTimeoutDispatchers.get(connectTimeoutMs); + if (cached) return cached; + const { Agent } = await import('undici'); + const dispatcher = new Agent({ connect: { timeout: connectTimeoutMs } }); + connectTimeoutDispatchers.set(connectTimeoutMs, dispatcher); + return dispatcher; +} + /** * Interface for browser-compatible logging functionality. * Provides methods for different log levels with context support. @@ -291,6 +317,17 @@ export interface RequestOptions { /** Timeout duration in milliseconds */ timeoutMs: number; }; + /** + * Connect timeout in milliseconds. Bounds only the connection-establishment + * phase (Node only, via an undici `Agent` dispatcher). When set, a connect + * that never completes (e.g. a black-holed SYN to a dead pod IP still + * lingering in a ClusterIP's iptables) fails in ~this window and the + * configured retry can land on a live endpoint — instead of stalling until + * the whole-request {@link timeout}. Slow-but-alive handlers are unaffected. + * Leaving it unset preserves the previous no-connect-timeout behavior. + * Ignored in browser/worker environments (no connect-timeout knob there). + */ + connectTimeoutMs?: number; /** Retry configuration */ retry?: RetryOptions; /** Schema for response validation. Must be a StandardSchemaV1 compatible schema (e.g., Zod schema) */ @@ -506,6 +543,15 @@ async function doGlobalFetch( useInit.body = JSON.stringify(useInit.body); } + // Bound the connect phase via an undici dispatcher (Node only). Only attach + // when explicitly requested so the default path stays byte-identical. + if (options?.connectTimeoutMs) { + const dispatcher = await getConnectTimeoutDispatcher(options.connectTimeoutMs); + if (dispatcher) { + (useInit as Record).dispatcher = dispatcher; + } + } + const response = await globalFetch()(url, useInit); let isJson = false; let data: ResponseType | undefined; @@ -805,6 +851,24 @@ export class FetchBuilder { return this; } + /** + * Sets the connect timeout (Node only). Bounds only the + * connection-establishment phase so a black-holed connect fails fast and + * retry can land on a live endpoint, instead of stalling until the + * whole-request {@link withTimeout} elapses. Leaving it unset preserves the + * previous no-connect-timeout behavior; ignored in browser/worker builds. + * Mirrors the Rust `FetchBuilder::with_connect_timeout`. + * @param connectTimeoutMs - Connect timeout duration in milliseconds + * @returns The builder instance for method chaining + */ + withConnectTimeout(connectTimeoutMs: number): FetchBuilder { + this._requestOptions = { + ...this._requestOptions, + connectTimeoutMs, + }; + return this; + } + /** * Configures retry behavior for failed requests. * If not specified, uses DEFAULT_RETRY_OPTIONS.