diff --git a/.changeset/envd-connect-retry.md b/.changeset/envd-connect-retry.md new file mode 100644 index 0000000000..2252408283 --- /dev/null +++ b/.changeset/envd-connect-retry.md @@ -0,0 +1,5 @@ +--- +'e2b': patch +--- + +Retry connections to the E2B API and to the sandbox (envd) that cannot be established (`ECONNREFUSED`, DNS failure, unreachable host, connect timeout) in Node, `E2B_CONNECTION_RETRIES` times (default 3) with the control-plane client's backoff — the same variable the Python SDK's `ConnectionRetryTransport` reads; `retries` keeps controlling control-plane request retries only. The retry happens at undici's connector level: only the TCP/TLS connect is repeated and the request is dispatched once, so it is safe for every request, unary or streaming. On Node the control-plane request retries (`retries`) no longer retry a connection failure a second time — the connector already did, as in Python. The health probe the SDK runs after a request failed uses no connection retries, so a sandbox that cannot be reached is not tried all over again. diff --git a/packages/js-sdk/src/api/http2.ts b/packages/js-sdk/src/api/http2.ts index 41aaba410e..ee99a385ef 100644 --- a/packages/js-sdk/src/api/http2.ts +++ b/packages/js-sdk/src/api/http2.ts @@ -1,5 +1,9 @@ import { runtime } from '../utils' -import { DEFAULT_HTTP_VERSION, type HttpVersion } from '../connectionConfig' +import { + DEFAULT_HTTP_VERSION, + resolveConnectionRetries, + type HttpVersion, +} from '../connectionConfig' import { parseInflightLimitEnv, parsePositiveIntEnv } from './metadata' import { buildDispatchedFetch, @@ -12,27 +16,48 @@ const DEFAULT_API_CONNECTION_LIMIT = 100 // Override via env if your workload needs different. const DEFAULT_API_INFLIGHT_LIMIT = 1000 -// Fetchers are cached per proxy and HTTP version so requests without a proxy -// keep sharing a single dispatcher while each distinct proxy URL gets its own. +// Fetchers are cached per proxy, HTTP version and connection retry count so +// requests without a proxy keep sharing a single dispatcher while each +// distinct proxy URL gets its own. const apiFetchers = new Map() +/** + * Options of the shared fetchers: `connectionRetries` is the number of + * connection attempts after the first for sockets that cannot be established + * (Node only; see `buildDispatchedFetch`), `ConnectionConfig.connectionRetries` + * i.e. `E2B_CONNECTION_RETRIES`. + */ export interface FetchOpts { proxy?: string httpVersion?: HttpVersion + connectionRetries?: number +} + +function fetcherKey( + proxy: string | undefined, + httpVersion: HttpVersion, + retries: number +): string { + return `${httpVersion}:${retries}:${proxy ?? ''}` } export function createApiFetch({ proxy, httpVersion = DEFAULT_HTTP_VERSION, + connectionRetries = resolveConnectionRetries(), }: FetchOpts = {}): typeof fetch { - const key = `${httpVersion}:${proxy ?? ''}` + const key = fetcherKey(proxy, httpVersion, connectionRetries) const cached = apiFetchers.get(key) if (cached) { return cached } - const apiFetch = createApiFetchForRuntime(runtime, { proxy, httpVersion }) + const apiFetch = createApiFetchForRuntime(runtime, { + proxy, + httpVersion, + connectRetries: connectionRetries, + }) apiFetchers.set(key, apiFetch) return apiFetch @@ -45,6 +70,7 @@ export function createApiFetchForRuntime( inflightLimit?: number proxy?: string httpVersion?: HttpVersion + connectRetries?: number loadUndici?: () => Promise } = {} ): typeof fetch { @@ -56,6 +82,7 @@ export function createApiFetchForRuntime( inflightLimit: options.inflightLimit ?? getApiInflightLimit(), proxy: options.proxy, httpVersion: options.httpVersion, + connectRetries: options.connectRetries, loadUndici: options.loadUndici, }) ) diff --git a/packages/js-sdk/src/api/inflight.ts b/packages/js-sdk/src/api/inflight.ts index 16401467af..596d6173ca 100644 --- a/packages/js-sdk/src/api/inflight.ts +++ b/packages/js-sdk/src/api/inflight.ts @@ -61,22 +61,35 @@ export function limitConcurrency( fetcher: typeof fetch, max: number ): typeof fetch { + return createConcurrencyLimiter(max)(fetcher) +} + +/** + * A reusable {@link limitConcurrency}: every fetcher passed through the + * returned function shares one semaphore of `max` slots, so a cap can span + * several fetchers (e.g. the envd REST fetcher and its no-retry health-probe + * variant). Identity when `max` is not a positive number. + */ +export function createConcurrencyLimiter( + max: number +): (fetcher: typeof fetch) => typeof fetch { if (!Number.isFinite(max) || max <= 0) { - return fetcher + return (fetcher) => fetcher } const sem = new Semaphore(max) - return (async (input, init) => { - // A Request the current global class disowns still carries the signal we - // have to honor while it waits for a slot. - const signal = - init?.signal ?? (isRequestLike(input) ? input.signal : undefined) - const release = await sem.acquire(signal) - try { - return await fetcher(input, init) - } finally { - release() - } - }) as typeof fetch + return (fetcher) => + (async (input, init) => { + // A Request the current global class disowns still carries the signal we + // have to honor while it waits for a slot. + const signal = + init?.signal ?? (isRequestLike(input) ? input.signal : undefined) + const release = await sem.acquire(signal) + try { + return await fetcher(input, init) + } finally { + release() + } + }) as typeof fetch } diff --git a/packages/js-sdk/src/api/metadata.ts b/packages/js-sdk/src/api/metadata.ts index 4eba2e0986..b304e66a26 100644 --- a/packages/js-sdk/src/api/metadata.ts +++ b/packages/js-sdk/src/api/metadata.ts @@ -33,12 +33,16 @@ export function getEnvVar(name: string) { * the env var is unset. Throws on non-integer input rather than silently * falling back so misconfiguration is surfaced loudly. */ -function parseIntEnv(name: string, defaultValue: number): number { +export function parseIntEnv(name: string, defaultValue: number): number { const raw = getEnvVar(name) if (!raw) return defaultValue - const parsed = Number.parseInt(raw, 10) - if (!Number.isFinite(parsed)) { + // `Number.parseInt` would accept `3foo` or `0.5` as 3 / 0, silently + // changing the setting; require the whole value to be a decimal integer. + const parsed = /^[+-]?\d+$/.test(raw.trim()) + ? Number.parseInt(raw, 10) + : Number.NaN + if (!Number.isSafeInteger(parsed)) { throw new Error( `Invalid ${name}=${JSON.stringify(raw)}: expected an integer.` ) diff --git a/packages/js-sdk/src/connectionConfig.ts b/packages/js-sdk/src/connectionConfig.ts index b946cc69b4..92a297b4de 100644 --- a/packages/js-sdk/src/connectionConfig.ts +++ b/packages/js-sdk/src/connectionConfig.ts @@ -1,5 +1,5 @@ import { Logger } from './logs' -import { getEnvVar, version } from './api/metadata' +import { getEnvVar, parseIntEnv, version } from './api/metadata' import { runtime } from './utils' import { resolveRetries } from './retry' import { InvalidArgumentError } from './errors' @@ -40,6 +40,23 @@ export function resolveHttpVersion(httpVersion?: string): HttpVersion { /** * Connection options for requests to the API. */ +/** + * Number of attempts, after the first, to establish a connection to the E2B + * API or to the sandbox (envd) when it cannot be established: + * `E2B_CONNECTION_RETRIES`, else 3 — the same variable the Python SDK reads. + * Node only; not a connection option, so that `retries` keeps meaning + * control-plane request retries in both SDKs. + */ +export function resolveConnectionRetries(): number { + const retries = parseIntEnv('E2B_CONNECTION_RETRIES', DEFAULT_RETRIES) + if (retries < 0) { + throw new InvalidArgumentError( + `Invalid E2B_CONNECTION_RETRIES=${retries}: expected a non-negative integer.` + ) + } + return retries +} + export interface ConnectionOpts { /** * E2B API key to use for authentication. @@ -449,6 +466,15 @@ export class ConnectionConfig { readonly requestTimeoutMs: number readonly retries: number + /** + * Number of attempts, after the first, to establish a connection to the E2B + * API or to the sandbox (envd) when it cannot be established (connection + * refused, DNS failure, unreachable host, connect timeout): + * `E2B_CONNECTION_RETRIES`, default 3, as in the Python SDK. Node only — the + * retry happens at the socket level and the request itself is sent once; + * other runtimes do not retry connections. + */ + readonly connectionRetries: number readonly apiKey?: string /** @@ -469,6 +495,7 @@ export class ConnectionConfig { this.domain = opts?.domain || ConnectionConfig.domain this.requestTimeoutMs = opts?.requestTimeoutMs ?? REQUEST_TIMEOUT_MS this.retries = resolveRetries(opts?.retries ?? DEFAULT_RETRIES) + this.connectionRetries = resolveConnectionRetries() this.logger = opts?.logger this.headers = { ...(opts?.headers ?? {}), ...(opts?.apiHeaders ?? {}) } ConnectionConfig.applyUserAgent(this.headers) diff --git a/packages/js-sdk/src/envd/api.ts b/packages/js-sdk/src/envd/api.ts index f92223c940..da03f5f9ed 100644 --- a/packages/js-sdk/src/envd/api.ts +++ b/packages/js-sdk/src/envd/api.ts @@ -39,14 +39,14 @@ const HEALTH_CHECK_TIMEOUT_MS = 5_000 /** * Probes the sandbox's envd health endpoint. * - * @param envdApi - The envd API client of the sandbox. + * @param envdApi - The envd API client of the sandbox; the probe goes through its `health` client, which runs without connection retries so a sandbox that cannot be reached is not tried all over again. * @returns `true` if the sandbox is running, `false` if it is not, `undefined` if it answered but its state could not be determined. * @throws The `fetch` failure when the sandbox could not be reached (no answer within the probe timeout), or the `SandboxUnreachableError` a request would get when the proxy reports the sandbox running but envd's port not open. */ export async function checkSandboxHealth( envdApi: EnvdApiClient ): Promise { - const res = await envdApi.api.GET('/health', { + const res = await envdApi.health.GET('/health', { signal: AbortSignal.timeout(HEALTH_CHECK_TIMEOUT_MS), }) @@ -189,6 +189,11 @@ export async function handleWatchDirStartEvent( class EnvdApiClient { readonly api: ReturnType> + /** + * Client for the health probe run after a request failed: the same API over + * `healthFetch` (no connection retries), or `api` itself when not given. + */ + readonly health: ReturnType> readonly version: string constructor( @@ -198,28 +203,40 @@ class EnvdApiClient { */ envdAccessToken?: string fetch?: (request: Request) => ReturnType + /** + * `fetch` for the health probe, without connection retries. + */ + healthFetch?: (request: Request) => ReturnType headers?: Record }, metadata: { version: string } ) { - this.api = createClient({ - baseUrl: config.apiUrl, - fetch: config?.fetch, - headers: { - ...config?.headers, - ...(config.envdAccessToken && { - 'X-Access-Token': config.envdAccessToken, - }), - }, - // In HTTP 1.1, all connections are considered persistent unless declared otherwise - // keepalive: true, - }) + const build = ( + fetchImpl?: (request: Request) => ReturnType + ) => + createClient({ + baseUrl: config.apiUrl, + fetch: fetchImpl, + headers: { + ...config?.headers, + ...(config.envdAccessToken && { + 'X-Access-Token': config.envdAccessToken, + }), + }, + // In HTTP 1.1, all connections are considered persistent unless declared otherwise + // keepalive: true, + }) + this.api = build(config?.fetch) + this.health = config.healthFetch ? build(config.healthFetch) : this.api this.version = metadata.version if (config.logger) { this.api.use(createApiLogger(config.logger)) + if (this.health !== this.api) { + this.health.use(createApiLogger(config.logger)) + } } } } diff --git a/packages/js-sdk/src/envd/http2.ts b/packages/js-sdk/src/envd/http2.ts index ccf31d24ad..8ca1c8d326 100644 --- a/packages/js-sdk/src/envd/http2.ts +++ b/packages/js-sdk/src/envd/http2.ts @@ -1,7 +1,12 @@ import { runtime } from '../utils' -import { DEFAULT_HTTP_VERSION, type HttpVersion } from '../connectionConfig' +import { + DEFAULT_HTTP_VERSION, + resolveConnectionRetries, + type HttpVersion, +} from '../connectionConfig' import type { FetchOpts } from '../api/http2' import { parseInflightLimitEnv, parsePositiveIntEnv } from '../api/metadata' +import { createConcurrencyLimiter } from '../api/inflight' import { buildDispatchedFetch, createRuntimeFetch, @@ -13,6 +18,7 @@ type EnvdFetchOptions = { inflightLimit?: number proxy?: string httpVersion?: HttpVersion + connectRetries?: number loadUndici?: () => Promise } @@ -20,6 +26,11 @@ type EnvdFetchOptions = { // keep sharing a single dispatcher while each distinct proxy URL gets its own. const envdFetchers = new Map() const envdRpcFetchers = new Map() +// One in-flight cap per family across every fetcher variant (proxy, HTTP +// version, connection retries — the health probe uses 0), so the limits +// documented as process-wide stay process-wide. +let envdLimiter: ((fetcher: typeof fetch) => typeof fetch) | undefined +let envdRpcLimiter: ((fetcher: typeof fetch) => typeof fetch) | undefined const DEFAULT_ENVD_CONNECTION_LIMIT = 10 const DEFAULT_ENVD_RPC_CONNECTION_LIMIT = 200 const DEFAULT_ENVD_INFLIGHT_LIMIT = 2000 @@ -35,23 +46,30 @@ export function createEnvdFetchForRuntime( inflightLimit: options.inflightLimit ?? 0, proxy: options.proxy, httpVersion: options.httpVersion, + connectRetries: options.connectRetries, loadUndici: options.loadUndici, }) ) } +export type EnvdFetchOpts = FetchOpts + +// Kept local (same format as the API fetchers' key) so that tests mocking +// `../api/http2` for `createApiFetch` do not have to provide it. function fetcherKey( proxy: string | undefined, - httpVersion: HttpVersion + httpVersion: HttpVersion, + retries: number ): string { - return `${httpVersion}:${proxy ?? ''}` + return `${httpVersion}:${retries}:${proxy ?? ''}` } export function createEnvdFetch({ proxy, httpVersion = DEFAULT_HTTP_VERSION, -}: FetchOpts = {}): typeof fetch { - const key = fetcherKey(proxy, httpVersion) + connectionRetries = resolveConnectionRetries(), +}: EnvdFetchOpts = {}): typeof fetch { + const key = fetcherKey(proxy, httpVersion, connectionRetries) const cached = envdFetchers.get(key) if (cached) { @@ -60,11 +78,15 @@ export function createEnvdFetch({ // Keep one origin connection for short envd REST calls. If ALPN falls back // to h1, this favors connection pressure over per-sandbox throughput. - const envdFetch = createEnvdFetchForRuntime(runtime, { - inflightLimit: getEnvdInflightLimit(), - proxy, - httpVersion, - }) + envdLimiter ??= createConcurrencyLimiter(getEnvdInflightLimit()) + const envdFetch = envdLimiter( + createEnvdFetchForRuntime(runtime, { + inflightLimit: 0, + proxy, + httpVersion, + connectRetries: connectionRetries, + }) + ) envdFetchers.set(key, envdFetch) return envdFetch @@ -73,20 +95,25 @@ export function createEnvdFetch({ export function createEnvdRpcFetch({ proxy, httpVersion = DEFAULT_HTTP_VERSION, -}: FetchOpts = {}): typeof fetch { - const key = fetcherKey(proxy, httpVersion) + connectionRetries = resolveConnectionRetries(), +}: EnvdFetchOpts = {}): typeof fetch { + const key = fetcherKey(proxy, httpVersion, connectionRetries) const cached = envdRpcFetchers.get(key) if (cached) { return cached } - const envdRpcFetch = createEnvdFetchForRuntime(runtime, { - connectionLimit: getEnvdRpcConnectionLimit(), - inflightLimit: getEnvdRpcInflightLimit(), - proxy, - httpVersion, - }) + envdRpcLimiter ??= createConcurrencyLimiter(getEnvdRpcInflightLimit()) + const envdRpcFetch = envdRpcLimiter( + createEnvdFetchForRuntime(runtime, { + connectionLimit: getEnvdRpcConnectionLimit(), + inflightLimit: 0, + proxy, + httpVersion, + connectRetries: connectionRetries, + }) + ) envdRpcFetchers.set(key, envdRpcFetch) return envdRpcFetch diff --git a/packages/js-sdk/src/retry.ts b/packages/js-sdk/src/retry.ts index 903edce8bb..dbf81cbd88 100644 --- a/packages/js-sdk/src/retry.ts +++ b/packages/js-sdk/src/retry.ts @@ -36,8 +36,9 @@ const OPAQUE_NETWORK_ERROR = // client-supplied idempotency key (sandbox/fork/snapshot/API-key/volume/secret/ // webhook creation) or append to one (secret update) must stay off the list — // a replay could create a duplicate — and so does any new POST until it is -// reviewed. Connection-establishment failures -// are retried for every operation regardless: the request never left. +// reviewed. Connection-establishment failures are retried for every operation +// regardless: the request never left — unless the connector already retried +// them ({@link markConnectionRetried}), as the Python SDK's connection layer does. const REPLAYABLE_METHODS = new Set(['GET', 'PUT', 'PATCH', 'DELETE']) const REPLAYABLE_OPERATIONS: [method: string, path: RegExp][] = [ ['POST', /^\/sandboxes\/[^/]+\/(pause|resume|timeout|refreshes)$/], @@ -115,19 +116,49 @@ export function isReplayable(request: Request): boolean { ) } +const CONNECTION_RETRIED = Symbol.for('e2b.connectionRetried') + +/** + * Tag a connection error that the connector already retried + * (`withConnectRetries` in `undici.ts`), so that request-level retries do not + * repeat the exhausted connection attempts. undici surfaces the connector's + * error as the `cause` of its `TypeError('fetch failed')`, which is where + * {@link isConnectionRetried} finds the tag. + */ +export function markConnectionRetried(error: T): T { + if (error instanceof Error) { + Object.defineProperty(error, CONNECTION_RETRIED, { value: true }) + } + return error +} + +export function isConnectionRetried(error: unknown, depth = 0): boolean { + if (!(error instanceof Error) || depth > 4) return false + if ((error as { [CONNECTION_RETRIED]?: boolean })[CONNECTION_RETRIED]) { + return true + } + if (error instanceof AggregateError) { + return error.errors.some((member) => isConnectionRetried(member, depth + 1)) + } + return isConnectionRetried(error.cause, depth + 1) +} + /** * Whether a `fetch` rejection may be retried. Connection-establishment - * failures are always retried: the request never left. For a replayable - * request any other network error is retried as well — a connection dropped - * mid-request, or the opaque `TypeError` browsers and Cloudflare Workers raise - * for every network failure. Aborts are never retried. + * failures are retried for any request — the request never left — unless the + * connector already retried the connection itself ({@link isConnectionRetried}): + * the failure is then final, as in the Python SDK whose request-level retry + * excludes `ConnectError`. For a replayable request any other network error + * is retried as well — a connection dropped mid-request, or the opaque + * `TypeError` browsers and Cloudflare Workers raise for every network failure. + * Aborts are never retried. */ export function isRetryableFetchError( error: unknown, replayable: boolean ): boolean { if (error instanceof DOMException) return false - if (isConnectionError(error)) return true + if (isConnectionError(error)) return !isConnectionRetried(error) return replayable && error instanceof Error } @@ -153,7 +184,7 @@ function wait(delayMs: number, signal: AbortSignal): Promise { }) } -function backoffMs(attempt: number, random: () => number): number { +export function backoffMs(attempt: number, random: () => number): number { const backoff = Math.min(BACKOFF_BASE_MS * 2 ** attempt, BACKOFF_MAX_MS) return Math.floor( backoff * (BACKOFF_JITTER_MIN + random() * (1 - BACKOFF_JITTER_MIN)) @@ -187,7 +218,8 @@ function retryDelayMs( * Retry requests after a 429 carrying `Retry-After`, a 503 or — for replayable * requests ({@link isReplayable}) — a 502 (using `Retry-After` when present, * exponential backoff otherwise), or a network failure (exponential backoff; - * see {@link isRetryableFetchError}). + * see {@link isRetryableFetchError}); a connection failure the connector + * already retried is not retried again. */ export function withRetry( fetchImpl: typeof fetch, diff --git a/packages/js-sdk/src/sandbox/index.ts b/packages/js-sdk/src/sandbox/index.ts index 778c9859fd..0d6741e2a2 100644 --- a/packages/js-sdk/src/sandbox/index.ts +++ b/packages/js-sdk/src/sandbox/index.ts @@ -164,6 +164,13 @@ export class Sandbox extends SandboxApi { } const envdFetch = createEnvdFetch(this.connectionConfig) const envdRpcFetch = createEnvdRpcFetch(this.connectionConfig) + // The health probe run after a request failed goes without the connection + // retries, so a sandbox that cannot be reached is not tried all over again. + const envdHealthFetch = createEnvdFetch({ + proxy: this.connectionConfig.proxy, + httpVersion: this.connectionConfig.httpVersion, + connectionRetries: 0, + }) const rpcTransport = createConnectTransport({ baseUrl: this.envdApiUrl, @@ -209,6 +216,7 @@ export class Sandbox extends SandboxApi { ...sandboxHeaders, }, fetch: (request) => envdFetch(request), + healthFetch: (request) => envdHealthFetch(request), }, { version: opts.envdVersion, diff --git a/packages/js-sdk/src/undici.ts b/packages/js-sdk/src/undici.ts index 86f4b9f34f..55deac3e41 100644 --- a/packages/js-sdk/src/undici.ts +++ b/packages/js-sdk/src/undici.ts @@ -3,6 +3,7 @@ import { compareVersions } from 'compare-versions' import { limitConcurrency } from './api/inflight' import { isReadableStreamLike, isRequestLike } from './is' import { dynamicImport, toDispatchableStream } from './utils' +import { backoffMs, isConnectionError, markConnectionRetried } from './retry' import { DEFAULT_HTTP_VERSION, type HttpVersion } from './connectionConfig' type UndiciRequestInit = RequestInit & { @@ -10,13 +11,40 @@ type UndiciRequestInit = RequestInit & { duplex?: 'half' } +/** + * undici connector: establishes the TCP/TLS socket for a new origin connection. + */ +export type UndiciConnector = ( + options: Record, + callback: (err: Error | null, socket?: unknown) => void +) => void + +/** + * `ProxyAgent`'s factory for the client that talks to the proxy; undici hands + * it the connector it built for the proxy. + */ +export type UndiciProxyClientFactory = ( + origin: string | URL, + options: { connect: UndiciConnector } +) => unknown + export type UndiciModule = { - Agent: new (options: { allowH2: boolean; connections?: number }) => unknown + Agent: new (options: { + allowH2: boolean + connections?: number + connect?: UndiciConnector + }) => unknown + buildConnector?: (options: { allowH2: boolean }) => UndiciConnector + Pool?: new ( + origin: string | URL, + options: { connect?: UndiciConnector } + ) => unknown ProxyAgent: new (options: { uri: string allowH2: boolean connections?: number proxyTunnel: true + clientFactory?: UndiciProxyClientFactory }) => unknown fetch: unknown } @@ -87,18 +115,74 @@ export function createRuntimeFetch( }) as typeof fetch } +export type ConnectRetryDependencies = { + sleep?: (delayMs: number) => Promise + random?: () => number +} + +/** + * Wrap an undici connector so that a socket that cannot be established + * ({@link isConnectionError}: connection refused, DNS failure, unreachable + * host, connect timeout) is attempted again up to `retries` times with + * exponential backoff. Only the TCP/TLS connect is repeated — undici + * dispatches the request once, after a socket exists — so this is safe for + * every request, unary or streaming. Matches the Python SDK's + * `ConnectionRetryTransport`. + */ +export function withConnectRetries( + connect: UndiciConnector, + retries: number, + dependencies: ConnectRetryDependencies = {} +): UndiciConnector { + if (retries === 0) { + return connect + } + + const sleep = + dependencies.sleep ?? + ((delayMs: number) => + new Promise((resolve) => setTimeout(resolve, delayMs))) + const random = dependencies.random ?? Math.random + + return (options, callback) => { + let attempt = 0 + + const attemptConnect = () => + connect(options, (err, socket) => { + if (err && isConnectionError(err)) { + if (attempt < retries) { + sleep(backoffMs(attempt++, random)).then(attemptConnect) + return + } + markConnectionRetried(err) + } + + callback(err, socket) + }) + + attemptConnect() + } +} + /** * Build a fetch bound to a bounded undici dispatcher (HTTP/2 enabled unless * `httpVersion` is `'1.1'`, `connections` origin connections, optional proxy * tunnel), capped at `inflightLimit` in-flight requests (`0` disables the * cap). Falls back to the global fetch — still capped — when undici cannot be * loaded. + * + * With `connectRetries > 0`, sockets that cannot be established are retried + * at the connector level ({@link withConnectRetries}). Through a proxy it is + * the connect to the proxy that is retried, via `ProxyAgent`'s + * `clientFactory`; the tunnel to the sandbox is a CONNECT request over that + * client, not a socket. */ export async function buildDispatchedFetch(options: { connections: number inflightLimit: number proxy?: string httpVersion?: HttpVersion + connectRetries?: number loadUndici?: () => Promise }): Promise { const undici = await (options.loadUndici ?? loadUndici)() @@ -107,18 +191,33 @@ export async function buildDispatchedFetch(options: { return limitConcurrency(lateBoundGlobalFetch(), options.inflightLimit) } - const { Agent, ProxyAgent, fetch: undiciFetch } = undici + const { Agent, Pool, ProxyAgent, buildConnector, fetch: undiciFetch } = undici const allowH2 = (options.httpVersion ?? DEFAULT_HTTP_VERSION) === '2' + const connectRetries = options.connectRetries ?? 0 + const connect = + connectRetries > 0 && buildConnector + ? withConnectRetries(buildConnector({ allowH2 }), connectRetries) + : undefined + const clientFactory: UndiciProxyClientFactory | undefined = + connectRetries > 0 && Pool + ? (origin, opts) => + new Pool(origin, { + ...opts, + connect: withConnectRetries(opts.connect, connectRetries), + }) + : undefined const dispatcher = options.proxy ? new ProxyAgent({ uri: options.proxy, allowH2, connections: options.connections, proxyTunnel: true, + ...(clientFactory ? { clientFactory } : {}), }) : new Agent({ allowH2, connections: options.connections, + ...(connect ? { connect } : {}), }) const fetchWithDispatcher = undiciFetch as unknown as ( input: RequestInfo | URL, diff --git a/packages/js-sdk/tests/api/http2.test.ts b/packages/js-sdk/tests/api/http2.test.ts index a1cc6fe18c..f5e34452de 100644 --- a/packages/js-sdk/tests/api/http2.test.ts +++ b/packages/js-sdk/tests/api/http2.test.ts @@ -167,7 +167,42 @@ test('pins the API dispatcher to HTTP/1.1 when httpVersion is http1', async () = expect(agents).toEqual([{ allowH2: false, connections: 3 }]) }) -test('caches API fetchers per proxy and HTTP version', async () => { +test('retries API connections at the connector level when connectRetries > 0', async () => { + const agents: Array<{ connect?: unknown }> = [] + const connector = vi.fn() + + class Agent { + constructor(options: { connect?: unknown }) { + agents.push(options) + } + } + + const undiciFetch = vi.fn(() => Promise.resolve(new Response('ok'))) + const buildConnector = vi.fn(() => connector) + const loadUndici = () => + Promise.resolve({ Agent, buildConnector, fetch: undiciFetch }) + + const { createApiFetchForRuntime } = await import('../../src/api/http2') + + const withRetries = createApiFetchForRuntime('node', { + connectRetries: 3, + loadUndici, + }) + await withRetries('https://example.com/sandboxes') + + const withoutRetries = createApiFetchForRuntime('node', { + connectRetries: 0, + loadUndici, + }) + await withoutRetries('https://example.com/sandboxes') + + expect(buildConnector).toHaveBeenCalledWith({ allowH2: true }) + expect(typeof agents[0].connect).toBe('function') + expect(agents[0].connect).not.toBe(connector) + expect(agents[1].connect).toBeUndefined() +}) + +test('caches API fetchers per proxy, HTTP version and connection retries', async () => { const { createApiFetch } = await import('../../src/api/http2') const noProxy = createApiFetch() @@ -179,6 +214,8 @@ test('caches API fetchers per proxy and HTTP version', async () => { expect(createApiFetch({ httpVersion: '2' })).toBe(noProxy) expect(createApiFetch({ proxy: 'http://127.0.0.1:8080' })).toBe(proxyA) expect(createApiFetch({ httpVersion: '1.1' })).toBe(noProxyH1) + expect(createApiFetch({ connectionRetries: 3 })).toBe(noProxy) + expect(createApiFetch({ connectionRetries: 0 })).not.toBe(noProxy) expect(proxyA).not.toBe(noProxy) expect(proxyA).not.toBe(proxyB) expect(noProxyH1).not.toBe(noProxy) diff --git a/packages/js-sdk/tests/api/inflight.test.ts b/packages/js-sdk/tests/api/inflight.test.ts index bd23eb34ca..53d26fdb6b 100644 --- a/packages/js-sdk/tests/api/inflight.test.ts +++ b/packages/js-sdk/tests/api/inflight.test.ts @@ -1,6 +1,9 @@ import { expect, test, vi } from 'vitest' -import { limitConcurrency } from '../../src/api/inflight' +import { + createConcurrencyLimiter, + limitConcurrency, +} from '../../src/api/inflight' import { foreignRequestClasses } from '../foreignPlatformObjects' function deferred() { @@ -99,3 +102,33 @@ test('limitConcurrency honors the signal of a Request the global class disowns', expect(inner).not.toHaveBeenCalled() }) + +test('createConcurrencyLimiter shares one cap across several fetchers', async () => { + let release!: () => void + const first = vi.fn( + () => + new Promise((resolve) => { + release = () => resolve(new Response('first')) + }) + ) as unknown as typeof fetch + const second = vi.fn(async () => new Response('second')) as typeof fetch + const limit = createConcurrencyLimiter(1) + const limitedFirst = limit(first) + const limitedSecond = limit(second) + + const firstCall = limitedFirst('https://sandbox.test/files') + const secondCall = limitedSecond('https://sandbox.test/health') + await Promise.resolve() + expect(first).toHaveBeenCalledTimes(1) + expect(second).not.toHaveBeenCalled() + + release() + await firstCall + await secondCall + expect(second).toHaveBeenCalledTimes(1) +}) + +test('createConcurrencyLimiter is the identity when the cap is disabled', () => { + const fetcher = vi.fn() as unknown as typeof fetch + expect(createConcurrencyLimiter(0)(fetcher)).toBe(fetcher) +}) diff --git a/packages/js-sdk/tests/connectionConfig.test.ts b/packages/js-sdk/tests/connectionConfig.test.ts index 5c7a2065c6..b717424fe3 100644 --- a/packages/js-sdk/tests/connectionConfig.test.ts +++ b/packages/js-sdk/tests/connectionConfig.test.ts @@ -17,6 +17,7 @@ beforeEach(() => { E2B_DOMAIN: process.env.E2B_DOMAIN, E2B_SANDBOX_URL: process.env.E2B_SANDBOX_URL, E2B_HTTP_VERSION: process.env.E2B_HTTP_VERSION, + E2B_CONNECTION_RETRIES: process.env.E2B_CONNECTION_RETRIES, E2B_DEBUG: process.env.E2B_DEBUG, } }) @@ -182,6 +183,33 @@ test('httpVersion defaults to http2 and reads E2B_HTTP_VERSION', () => { assert.equal(new ConnectionConfig({ httpVersion: '1.1' }).httpVersion, '1.1') }) +test('connectionRetries defaults to 3 and reads E2B_CONNECTION_RETRIES', () => { + delete process.env.E2B_CONNECTION_RETRIES + assert.equal(new ConnectionConfig().connectionRetries, DEFAULT_RETRIES) + // `retries` is the control-plane count and does not drive it, as in Python. + assert.equal( + new ConnectionConfig({ retries: 0 }).connectionRetries, + DEFAULT_RETRIES + ) + + process.env.E2B_CONNECTION_RETRIES = '0' + assert.equal(new ConnectionConfig().connectionRetries, 0) + process.env.E2B_CONNECTION_RETRIES = '5' + assert.equal(new ConnectionConfig().connectionRetries, 5) + + process.env.E2B_CONNECTION_RETRIES = '-1' + assert.throws(() => new ConnectionConfig(), InvalidArgumentError) + assert.throws(() => new ConnectionConfig(), /E2B_CONNECTION_RETRIES/) + for (const value of ['many', '0.5', '3foo', '1e3', ' ']) { + process.env.E2B_CONNECTION_RETRIES = value + assert.throws( + () => new ConnectionConfig(), + /E2B_CONNECTION_RETRIES/, + `E2B_CONNECTION_RETRIES=${JSON.stringify(value)}` + ) + } +}) + test('httpVersion rejects values other than 1.1 and 2', () => { delete process.env.E2B_HTTP_VERSION for (const value of ['http1', 'http2', '1', '2.0', 'HTTP/1.1', '']) { diff --git a/packages/js-sdk/tests/envd/handleEnvdApiError.test.ts b/packages/js-sdk/tests/envd/handleEnvdApiError.test.ts index 10bfa0a7ea..f57bae0667 100644 --- a/packages/js-sdk/tests/envd/handleEnvdApiError.test.ts +++ b/packages/js-sdk/tests/envd/handleEnvdApiError.test.ts @@ -30,7 +30,7 @@ const PORT_CLOSED = 'The sandbox is running but port is not open' function healthApi(status: number, error?: { message?: string } | string) { return { - api: { GET: async () => createMockResponse(status, error) }, + health: { GET: async () => createMockResponse(status, error) }, } as unknown as Parameters[0] } diff --git a/packages/js-sdk/tests/envd/http2.test.ts b/packages/js-sdk/tests/envd/http2.test.ts index 907c88cf15..c7c7f524f5 100644 --- a/packages/js-sdk/tests/envd/http2.test.ts +++ b/packages/js-sdk/tests/envd/http2.test.ts @@ -151,6 +151,8 @@ test('caches envd fetchers per proxy and HTTP version', async () => { expect(createEnvdFetch({ httpVersion: '1.1' })).toBe(noProxyH1) expect(proxyA).not.toBe(noProxy) expect(noProxyH1).not.toBe(noProxy) + expect(createEnvdFetch({ connectionRetries: 3 })).toBe(noProxy) + expect(createEnvdFetch({ connectionRetries: 0 })).not.toBe(noProxy) const rpcNoProxy = createEnvdRpcFetch() const rpcProxyA = createEnvdRpcFetch({ proxy: 'http://127.0.0.1:8080' }) diff --git a/packages/js-sdk/tests/retry.test.ts b/packages/js-sdk/tests/retry.test.ts index 7f43e9f9e7..9e31b956a3 100644 --- a/packages/js-sdk/tests/retry.test.ts +++ b/packages/js-sdk/tests/retry.test.ts @@ -8,8 +8,10 @@ import { resolveRetries, withRetry, isNetworkError, + isConnectionRetried, + markConnectionRetried, } from '../src/retry' -import { EnvdApiClient } from '../src/envd/api' +import { checkSandboxHealth, EnvdApiClient } from '../src/envd/api' import { InvalidArgumentError } from '../src/errors' describe('resolveRetries', () => { @@ -414,6 +416,46 @@ test('envd clients do not retry rate-limited requests', async () => { expect(fetchImpl).toHaveBeenCalledOnce() }) +test('the health probe goes through healthFetch, not the retrying fetch', async () => { + const fetchImpl = vi.fn(async () => { + throw new Error('retrying fetch must not be used by the probe') + }) as typeof fetch + const healthFetch = vi.fn( + async () => new Response('{}', { status: 200 }) + ) as typeof fetch + const client = new EnvdApiClient( + { + apiUrl: 'https://envd.e2b.test', + logger: undefined, + fetch: fetchImpl, + healthFetch, + }, + { version: '0.0.0' } + ) + + expect(await checkSandboxHealth(client)).toBe(true) + expect(healthFetch).toHaveBeenCalledOnce() + expect(fetchImpl).not.toHaveBeenCalled() +}) + +test('the health probe falls back to fetch without healthFetch', async () => { + const fetchImpl = vi.fn( + async () => new Response('{}', { status: 200 }) + ) as typeof fetch + const client = new EnvdApiClient( + { + apiUrl: 'https://envd.e2b.test', + logger: undefined, + fetch: fetchImpl, + }, + { version: '0.0.0' } + ) + + expect(client.health).toBe(client.api) + expect(await checkSandboxHealth(client)).toBe(true) + expect(fetchImpl).toHaveBeenCalledOnce() +}) + function connectError(code: string, syscall = 'connect'): TypeError { return new TypeError('fetch failed', { cause: Object.assign(new Error(`${syscall} ${code}`), { code, syscall }), @@ -547,6 +589,39 @@ describe('isRetryableFetchError', () => { expect(isRetryableFetchError(error, false)).toBe(false) } ) + + test.each(Object.entries(connectFailures))( + 'does not retry %s again once the connector retried it', + (_, error) => { + // undici tags the socket error, which surfaces as the fetch error's cause + const retried = markConnectionRetried( + (error.cause as Error | undefined) ?? error + ) + expect(isConnectionRetried(error)).toBe(true) + expect(isRetryableFetchError(error, true)).toBe(false) + expect(isRetryableFetchError(error, false)).toBe(false) + expect(retried).toBe(error.cause ?? error) + } + ) + + test('finds the connector tag inside a happy-eyeballs AggregateError', () => { + const member = connectError('ECONNREFUSED').cause as Error + const error = Object.assign(new Error('fetch failed'), { + cause: new AggregateError([markConnectionRetried(member)]), + }) + expect(isConnectionRetried(error)).toBe(true) + expect(isRetryableFetchError(error, true)).toBe(false) + }) + + test.each([ + ['non-error', 'ECONNREFUSED'], + ['no cause', new Error('x')], + ])('isConnectionRetried is false for %s', (_, error) => { + expect(isConnectionRetried(markConnectionRetried(error) && error)).toBe( + error instanceof Error + ) + expect(isConnectionRetried(new TypeError('fetch failed'))).toBe(false) + }) }) const nonReplayable = [ @@ -646,6 +721,25 @@ test('rethrows the connection failure after exhausting retries', async () => { expect(fetchImpl).toHaveBeenCalledTimes(3) }) +test('does not retry a connection failure the connector already retried', async () => { + const error = connectError('ECONNREFUSED') + markConnectionRetried(error.cause) + const fetchImpl = vi.fn(async () => { + throw error + }) as typeof fetch + const sleep = vi.fn(async () => {}) + const fetchWithRetry = withRetry(fetchImpl, 3, 10_000, { + monotonic: () => 0, + sleep, + }) + + await expect(fetchWithRetry('https://api.e2b.test/resource')).rejects.toBe( + error + ) + expect(fetchImpl).toHaveBeenCalledTimes(1) + expect(sleep).not.toHaveBeenCalled() +}) + test('rethrows a connection failure when the backoff would exceed the request timeout', async () => { const error = connectError('ECONNREFUSED') const fetchImpl = vi.fn(async () => { diff --git a/packages/js-sdk/tests/undici.test.ts b/packages/js-sdk/tests/undici.test.ts index 1d32d9c039..c6ac2c1051 100644 --- a/packages/js-sdk/tests/undici.test.ts +++ b/packages/js-sdk/tests/undici.test.ts @@ -1,11 +1,15 @@ -import { expect, test, vi } from 'vitest' +import { isConnectionRetried } from '../src/retry' +import { describe, expect, test, vi } from 'vitest' import { buildDispatchedFetch, createRuntimeFetch, getUndiciPackageCandidates, loadUndici, + withConnectRetries, + type UndiciConnector, type UndiciModule, + type UndiciProxyClientFactory, } from '../src/undici' import { runtime } from '../src/utils' import { @@ -217,3 +221,176 @@ test.skipIf(runtime !== 'node')( } } ) + +function connectError(code: string): Error { + return Object.assign(new Error(code), { code }) +} + +function connectorFailingTimes( + failures: number, + code: string +): UndiciConnector & { attempts: number } { + const connector = ((_options, callback) => { + connector.attempts++ + if (connector.attempts <= failures) { + callback(connectError(code)) + return + } + callback(null, 'socket') + }) as UndiciConnector & { attempts: number } + connector.attempts = 0 + return connector +} + +function connectOnce(connector: UndiciConnector) { + return new Promise<{ err: Error | null; socket?: unknown }>((resolve) => + connector({}, (err, socket) => resolve({ err, socket })) + ) +} + +describe('withConnectRetries', () => { + const noSleep = { sleep: () => Promise.resolve(), random: () => 1 } + + test('retries a socket that cannot be established and passes the socket through', async () => { + const connector = connectorFailingTimes(2, 'ECONNREFUSED') + const sleeps: number[] = [] + const { err, socket } = await connectOnce( + withConnectRetries(connector, 3, { + sleep: (ms) => { + sleeps.push(ms) + return Promise.resolve() + }, + random: () => 1, + }) + ) + + expect(err).toBeNull() + expect(socket).toBe('socket') + expect(connector.attempts).toBe(3) + expect(sleeps).toEqual([100, 200]) + }) + + test('gives up after the configured number of retries', async () => { + const connector = connectorFailingTimes(10, 'ENOTFOUND') + const { err } = await connectOnce(withConnectRetries(connector, 2, noSleep)) + + expect((err as { code?: string } | null)?.code).toBe('ENOTFOUND') + expect(connector.attempts).toBe(3) + // request-level retries must not repeat the exhausted connection attempts + expect(isConnectionRetried(err)).toBe(true) + }) + + test('does not retry errors that are not connection failures', async () => { + const connector = connectorFailingTimes(10, 'ECONNRESET') + const { err } = await connectOnce(withConnectRetries(connector, 3, noSleep)) + + expect((err as { code?: string } | null)?.code).toBe('ECONNRESET') + expect(connector.attempts).toBe(1) + expect(isConnectionRetried(err)).toBe(false) + }) + + test('returns the connector untouched when retries is 0', () => { + const connector = connectorFailingTimes(0, 'ECONNREFUSED') + expect(withConnectRetries(connector, 0)).toBe(connector) + }) +}) + +describe('buildDispatchedFetch connect retries', () => { + function fakeUndici(withBuildConnector: boolean) { + const agents: Array<{ connect?: UndiciConnector }> = [] + const proxyAgents: Array<{ clientFactory?: UndiciProxyClientFactory }> = [] + const pools: Array<{ origin: string | URL; connect?: UndiciConnector }> = [] + const buildConnector = vi.fn(() => connectorFailingTimes(0, 'ECONNREFUSED')) + const undici = { + Agent: class { + constructor(options: { connect?: UndiciConnector }) { + agents.push(options) + } + }, + Pool: class { + constructor( + origin: string | URL, + options: { connect?: UndiciConnector } + ) { + pools.push({ origin, ...options }) + } + }, + ProxyAgent: class { + constructor(options: { clientFactory?: UndiciProxyClientFactory }) { + proxyAgents.push(options) + } + }, + ...(withBuildConnector ? { buildConnector } : {}), + fetch: async () => new Response('ok'), + } as unknown as UndiciModule + return { undici, agents, proxyAgents, pools, buildConnector } + } + + test('wraps the Agent connector when connectRetries > 0', async () => { + const { undici, agents, buildConnector } = fakeUndici(true) + await buildDispatchedFetch({ + connections: 1, + inflightLimit: 0, + connectRetries: 3, + loadUndici: () => Promise.resolve(undici), + }) + + expect(buildConnector).toHaveBeenCalledWith({ allowH2: true }) + expect(typeof agents[0].connect).toBe('function') + }) + + test('retries the connect to the proxy through the ProxyAgent client', async () => { + const { undici, proxyAgents, pools } = fakeUndici(true) + await buildDispatchedFetch({ + connections: 1, + inflightLimit: 0, + proxy: 'http://127.0.0.1:8080', + connectRetries: 3, + loadUndici: () => Promise.resolve(undici), + }) + + const proxyConnector = connectorFailingTimes(1, 'ECONNREFUSED') + proxyAgents[0].clientFactory!('http://127.0.0.1:8080', { + connect: proxyConnector, + }) + expect(pools[0].origin).toBe('http://127.0.0.1:8080') + expect(pools[0].connect).not.toBe(proxyConnector) + + const { err, socket } = await connectOnce(pools[0].connect!) + expect(err).toBeNull() + expect(socket).toBe('socket') + expect(proxyConnector.attempts).toBe(2) + }) + + test('leaves the proxy client alone when connectRetries is 0', async () => { + const { undici, proxyAgents } = fakeUndici(true) + await buildDispatchedFetch({ + connections: 1, + inflightLimit: 0, + proxy: 'http://127.0.0.1:8080', + connectRetries: 0, + loadUndici: () => Promise.resolve(undici), + }) + expect(proxyAgents[0].clientFactory).toBeUndefined() + }) + + test('leaves the Agent connector alone when connectRetries is 0 or buildConnector is missing', async () => { + const withoutRetries = fakeUndici(true) + await buildDispatchedFetch({ + connections: 1, + inflightLimit: 0, + connectRetries: 0, + loadUndici: () => Promise.resolve(withoutRetries.undici), + }) + expect(withoutRetries.agents[0].connect).toBeUndefined() + + const withoutBuildConnector = fakeUndici(false) + await buildDispatchedFetch({ + connections: 1, + inflightLimit: 0, + connectRetries: 3, + loadUndici: () => Promise.resolve(withoutBuildConnector.undici), + }) + expect(withoutBuildConnector.agents[0].connect).toBeUndefined() + }) +})