Skip to content

Commit b511953

Browse files
mishushakovclaude
mishushakov
and
claude
authored
fix(js-sdk): share lazy fetcher loading and drop the new Function import trick (#1607)
Extracts the duplicated api/envd fetcher-loading logic into shared `createRuntimeFetch` + `buildDispatchedFetch` helpers in `undici.ts`, fixing two behaviors along the way: a failed fetcher build is no longer cached forever (the next request retries, with a guarded compare-and-clear so a stale awaiter can't clobber a newer in-flight build — covered by a regression test reproducing the microtask interleaving), and the no-undici fallback now late-binds `globalThis.fetch` so fetch replacements installed after the first request (msw, instrumentation) are picked up. `loadUndici` now uses the shared `dynamicImport` helper, whose import is kept opaque to downstream bundlers via `webpackIgnore`/`@vite-ignore` annotations instead of the `new Function('return import(...)')` trick — so environments that disallow code generation from strings (CSP, `--disallow-code-generation-from-strings`) now load undici normally instead of silently degrading to the global fetch. The now-internal `toUndiciRequestInput`/`UndiciRequestInit` are no longer exported. No user-facing API changes; verified with the unit suites (lint/typecheck clean) plus real API integration tests through the new dispatcher path. ### Test-suite fallout from the import fix Dropping the `new Function` trick exposed a hidden test dependency: that trick throws under vitest's vm evaluation, so every vitest run had silently fallen back to the msw-patchable global fetch. With module loading un-broken, the Node test runs dispatched through real undici, bypassing msw's `globalThis.fetch` patch — mocked requests escaped to the real API (real 404s in the tags suite, 3-minute hangs in the abortSignal suites waiting for msw's `request:start`, and 28 real template builds per run from the stacktrace suite). Fixed centrally: `tests/globalFetchFallback.setup.ts`, registered via `setupFiles` for the unit and template projects, mocks `buildDispatchedFetch` to run the SDK's real undici-unavailable fallback (late-bound `globalThis.fetch`), so msw suites need no per-file mock and future msw suites are covered automatically. Suites that inject their own `loadUndici` (the api/envd transport tests) keep it, so the dispatcher wiring itself stays covered. Verified under Node, Bun, and Cloudflare workerd. Closes SDK-290 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 59c6996 commit b511953

10 files changed

Lines changed: 274 additions & 140 deletions

File tree

‎.changeset/olive-pugs-refuse.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'e2b': patch
3+
---
4+
5+
Remove the `new Function('return import(...)')` trick from undici loading. `loadUndici` now uses the shared `dynamicImport` helper, whose dynamic import is kept opaque to downstream bundlers with `webpackIgnore`/`@vite-ignore` annotations instead of runtime code generation. Environments that disallow code generation from strings (CSP, `--disallow-code-generation-from-strings`) now load undici normally instead of silently falling back to the global fetch.

‎.changeset/tidy-moons-shout.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'e2b': patch
3+
---
4+
5+
Fix the lazy fetcher loading in `api/http2.ts` and `envd/http2.ts`: a failed fetcher build is no longer cached forever (the next request retries instead of replaying the stale rejection), and the no-undici fallback now late-binds `globalThis.fetch` so fetch replacements installed after the first request (msw, instrumentation) are picked up. The previously duplicated loading logic is shared in `undici.ts`.

‎packages/js-sdk/src/api/http2.ts‎

Lines changed: 12 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
import { runtime } from '../utils'
22
import { parseInflightLimitEnv, parsePositiveIntEnv } from './metadata'
3-
import { limitConcurrency } from './inflight'
43
import {
5-
loadUndici,
6-
toUndiciRequestInput,
4+
buildDispatchedFetch,
5+
createRuntimeFetch,
76
type UndiciModule,
8-
type UndiciRequestInit,
97
} from '../undici'
108

119
const DEFAULT_API_CONNECTION_LIMIT = 100
@@ -40,64 +38,16 @@ export function createApiFetchForRuntime(
4038
loadUndici?: () => Promise<UndiciModule | undefined>
4139
} = {}
4240
): typeof fetch {
43-
if (currentRuntime !== 'node') {
44-
// Late-bind the global fetch: runtimes and tools (msw, instrumentation)
45-
// may replace `globalThis.fetch` after this factory runs, and a fresh
46-
// closure keeps the per-proxy cache entries distinct.
47-
return ((input, init) => globalThis.fetch(input, init)) as typeof fetch
48-
}
49-
50-
let fetcherPromise: Promise<typeof fetch> | undefined
51-
52-
return (async (input, init) => {
53-
fetcherPromise ??= buildApiFetcher(options)
54-
const fetcher = await fetcherPromise
55-
56-
return fetcher(input, init)
57-
}) as typeof fetch
58-
}
59-
60-
async function buildApiFetcher(options: {
61-
connectionLimit?: number
62-
inflightLimit?: number
63-
proxy?: string
64-
loadUndici?: () => Promise<UndiciModule | undefined>
65-
}): Promise<typeof fetch> {
66-
const undici = await (options.loadUndici ?? loadUndici)()
67-
const inflightLimit = options.inflightLimit ?? getApiInflightLimit()
68-
69-
if (!undici) {
70-
return limitConcurrency(fetch, inflightLimit)
71-
}
72-
73-
const { Agent, ProxyAgent, fetch: undiciFetch } = undici
74-
const connections = options.connectionLimit ?? getApiConnectionLimit()
75-
const dispatcher = options.proxy
76-
? new ProxyAgent({
77-
uri: options.proxy,
78-
allowH2: true,
79-
connections,
80-
proxyTunnel: true,
81-
})
82-
: new Agent({
83-
allowH2: true,
84-
connections,
85-
})
86-
const fetchWithDispatcher = undiciFetch as unknown as (
87-
input: RequestInfo | URL,
88-
init?: UndiciRequestInit
89-
) => Promise<Response>
90-
91-
const wrapped: typeof fetch = ((input, init) => {
92-
const request = toUndiciRequestInput(input, init)
93-
94-
return fetchWithDispatcher(request.input, {
95-
...request.init,
96-
dispatcher,
41+
// Defaults resolve inside the thunk so env vars are read lazily, at the
42+
// first request rather than at factory creation.
43+
return createRuntimeFetch(currentRuntime, () =>
44+
buildDispatchedFetch({
45+
connections: options.connectionLimit ?? getApiConnectionLimit(),
46+
inflightLimit: options.inflightLimit ?? getApiInflightLimit(),
47+
proxy: options.proxy,
48+
loadUndici: options.loadUndici,
9749
})
98-
}) as typeof fetch
99-
100-
return limitConcurrency(wrapped, inflightLimit)
50+
)
10151
}
10252

10353
export function getApiConnectionLimit(): number {
@@ -111,7 +61,7 @@ export function getApiConnectionLimit(): number {
11161
* Returns the configured max number of API requests that can be in flight at
11262
* once, or `0` to disable the cap.
11363
*
114-
* Defaults to {@link DEFAULT_API_INFLIGHT_LIMIT} ({@link 1000}). Override via
64+
* Defaults to `1000` ({@link DEFAULT_API_INFLIGHT_LIMIT}). Override via
11565
* `E2B_API_INFLIGHT_REQUESTS` env var; set to `0` to disable the cap entirely.
11666
*/
11767
export function getApiInflightLimit(): number {

‎packages/js-sdk/src/envd/http2.ts‎

Lines changed: 11 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
import { runtime } from '../utils'
22
import { parseInflightLimitEnv, parsePositiveIntEnv } from '../api/metadata'
3-
import { limitConcurrency } from '../api/inflight'
43
import {
5-
loadUndici,
6-
toUndiciRequestInput,
4+
buildDispatchedFetch,
5+
createRuntimeFetch,
76
type UndiciModule,
8-
type UndiciRequestInit,
97
} from '../undici'
108

119
type EnvdFetchOptions = {
@@ -28,62 +26,14 @@ export function createEnvdFetchForRuntime(
2826
currentRuntime = runtime,
2927
options: EnvdFetchOptions = {}
3028
): typeof fetch {
31-
if (currentRuntime !== 'node') {
32-
// Late-bind the global fetch: runtimes and tools (msw, instrumentation)
33-
// may replace `globalThis.fetch` after this factory runs, and a fresh
34-
// closure keeps the per-proxy cache entries distinct.
35-
return ((input, init) => globalThis.fetch(input, init)) as typeof fetch
36-
}
37-
38-
let fetcherPromise: Promise<typeof fetch> | undefined
39-
40-
return (async (input, init) => {
41-
fetcherPromise ??= buildEnvdFetcher(options)
42-
const fetcher = await fetcherPromise
43-
44-
return fetcher(input, init)
45-
}) as typeof fetch
46-
}
47-
48-
async function buildEnvdFetcher(
49-
options: EnvdFetchOptions
50-
): Promise<typeof fetch> {
51-
const undici = await (options.loadUndici ?? loadUndici)()
52-
const inflightLimit = options.inflightLimit ?? 0
53-
54-
if (!undici) {
55-
return limitConcurrency(fetch, inflightLimit)
56-
}
57-
58-
const { Agent, ProxyAgent, fetch: undiciFetch } = undici
59-
const connections = options.connectionLimit ?? DEFAULT_ENVD_CONNECTION_LIMIT
60-
61-
const dispatcher = options.proxy
62-
? new ProxyAgent({
63-
uri: options.proxy,
64-
allowH2: true,
65-
connections,
66-
proxyTunnel: true,
67-
})
68-
: new Agent({
69-
allowH2: true,
70-
connections,
71-
})
72-
const fetchWithDispatcher = undiciFetch as unknown as (
73-
input: RequestInfo | URL,
74-
init?: UndiciRequestInit
75-
) => Promise<Response>
76-
77-
const wrapped: typeof fetch = ((input, init) => {
78-
const request = toUndiciRequestInput(input, init)
79-
80-
return fetchWithDispatcher(request.input, {
81-
...request.init,
82-
dispatcher,
29+
return createRuntimeFetch(currentRuntime, () =>
30+
buildDispatchedFetch({
31+
connections: options.connectionLimit ?? DEFAULT_ENVD_CONNECTION_LIMIT,
32+
inflightLimit: options.inflightLimit ?? 0,
33+
proxy: options.proxy,
34+
loadUndici: options.loadUndici,
8335
})
84-
}) as typeof fetch
85-
86-
return limitConcurrency(wrapped, inflightLimit)
36+
)
8737
}
8838

8939
export function createEnvdFetch(proxy?: string): typeof fetch {
@@ -135,7 +85,7 @@ export function getEnvdRpcConnectionLimit(): number {
13585
* `files.read`/`files.write`) that can be in flight at once across all
13686
* sandboxes in this SDK process, or `0` to disable the cap.
13787
*
138-
* Defaults to {@link DEFAULT_ENVD_INFLIGHT_LIMIT} ({@link 2000}). Override
88+
* Defaults to `2000` ({@link DEFAULT_ENVD_INFLIGHT_LIMIT}). Override
13989
* via `E2B_ENVD_INFLIGHT_REQUESTS` env var; set to `0` to disable the cap
14090
* entirely.
14191
*/
@@ -151,7 +101,7 @@ export function getEnvdInflightLimit(): number {
151101
* can be in flight at once across all sandboxes in this SDK process,
152102
* or `0` to disable the cap.
153103
*
154-
* Defaults to {@link DEFAULT_ENVD_RPC_INFLIGHT_LIMIT} ({@link 2000}). Override
104+
* Defaults to `2000` ({@link DEFAULT_ENVD_RPC_INFLIGHT_LIMIT}). Override
155105
* via `E2B_ENVD_RPC_INFLIGHT_REQUESTS` env var; set to `0` to disable the cap
156106
* entirely.
157107
*/

‎packages/js-sdk/src/undici.ts‎

Lines changed: 97 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import { compareVersions } from 'compare-versions'
22

3-
export type UndiciRequestInit = RequestInit & {
3+
import { limitConcurrency } from './api/inflight'
4+
import { dynamicImport } from './utils'
5+
6+
type UndiciRequestInit = RequestInit & {
47
dispatcher?: unknown
58
duplex?: 'half'
69
}
@@ -27,20 +30,9 @@ export function getUndiciPackageCandidates(nodeVersion: string): string[] {
2730
}
2831

2932
export async function loadUndici(): Promise<UndiciModule | undefined> {
30-
let importModule: (moduleName: string) => Promise<UndiciModule>
31-
try {
32-
// Keep package imports opaque so downstream bundlers resolve them at runtime.
33-
// eslint-disable-next-line no-new-func
34-
importModule = new Function('moduleName', 'return import(moduleName)') as (
35-
moduleName: string
36-
) => Promise<UndiciModule>
37-
} catch {
38-
return undefined
39-
}
40-
4133
for (const packageName of getUndiciPackageCandidates(process.versions.node)) {
4234
try {
43-
return await importModule(packageName)
35+
return await dynamicImport<UndiciModule>(packageName)
4436
} catch {
4537
// Try the next package supported by this Node version.
4638
}
@@ -49,7 +41,98 @@ export async function loadUndici(): Promise<UndiciModule | undefined> {
4941
return undefined
5042
}
5143

52-
export function toUndiciRequestInput(
44+
/**
45+
* Late-bind the global fetch: runtimes and tools (msw, instrumentation) may
46+
* replace `globalThis.fetch` after the SDK builds a fetcher. A factory rather
47+
* than a shared const so per-proxy cache entries stay distinct closures.
48+
*/
49+
function lateBoundGlobalFetch(): typeof fetch {
50+
return ((input, init) => globalThis.fetch(input, init)) as typeof fetch
51+
}
52+
53+
/**
54+
* Create a fetch for the given runtime. Outside Node it late-binds the global
55+
* fetch. On Node it lazily runs `build` on the first request and caches the
56+
* built fetcher; a failed build is not cached, so the next request retries
57+
* instead of replaying the same stale rejection forever.
58+
*/
59+
export function createRuntimeFetch(
60+
currentRuntime: string,
61+
build: () => Promise<typeof fetch>
62+
): typeof fetch {
63+
if (currentRuntime !== 'node') {
64+
return lateBoundGlobalFetch()
65+
}
66+
67+
let fetcherPromise: Promise<typeof fetch> | undefined
68+
69+
return (async (input, init) => {
70+
const promise = (fetcherPromise ??= build())
71+
72+
let fetcher: typeof fetch
73+
try {
74+
fetcher = await promise
75+
} catch (err) {
76+
// Clear only our own failed build: a stale awaiter of an already-
77+
// rejected promise must not clobber a newer in-flight build.
78+
if (fetcherPromise === promise) {
79+
fetcherPromise = undefined
80+
}
81+
throw err
82+
}
83+
84+
return fetcher(input, init)
85+
}) as typeof fetch
86+
}
87+
88+
/**
89+
* Build a fetch bound to a bounded undici dispatcher (HTTP/2 enabled,
90+
* `connections` origin connections, optional proxy tunnel), capped at
91+
* `inflightLimit` in-flight requests (`0` disables the cap). Falls back to
92+
* the global fetch — still capped — when undici cannot be loaded.
93+
*/
94+
export async function buildDispatchedFetch(options: {
95+
connections: number
96+
inflightLimit: number
97+
proxy?: string
98+
loadUndici?: () => Promise<UndiciModule | undefined>
99+
}): Promise<typeof fetch> {
100+
const undici = await (options.loadUndici ?? loadUndici)()
101+
102+
if (!undici) {
103+
return limitConcurrency(lateBoundGlobalFetch(), options.inflightLimit)
104+
}
105+
106+
const { Agent, ProxyAgent, fetch: undiciFetch } = undici
107+
const dispatcher = options.proxy
108+
? new ProxyAgent({
109+
uri: options.proxy,
110+
allowH2: true,
111+
connections: options.connections,
112+
proxyTunnel: true,
113+
})
114+
: new Agent({
115+
allowH2: true,
116+
connections: options.connections,
117+
})
118+
const fetchWithDispatcher = undiciFetch as unknown as (
119+
input: RequestInfo | URL,
120+
init?: UndiciRequestInit
121+
) => Promise<Response>
122+
123+
const wrapped: typeof fetch = ((input, init) => {
124+
const request = toUndiciRequestInput(input, init)
125+
126+
return fetchWithDispatcher(request.input, {
127+
...request.init,
128+
dispatcher,
129+
})
130+
}) as typeof fetch
131+
132+
return limitConcurrency(wrapped, options.inflightLimit)
133+
}
134+
135+
function toUndiciRequestInput(
53136
input: RequestInfo | URL,
54137
init?: RequestInit
55138
): { input: RequestInfo | URL; init?: RequestInit & { duplex?: 'half' } } {

‎packages/js-sdk/src/utils.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,22 @@ export function timeoutToSeconds(timeout: number): number {
6262
return Math.ceil(timeout / 1000)
6363
}
6464

65+
/**
66+
* Import an optional, runtime-resolved package (e.g. `undici`, `glob`, `tar`)
67+
* without letting downstream bundlers resolve it at build time.
68+
*
69+
* The variable specifier plus the `webpackIgnore`/`@vite-ignore` annotations
70+
* keep the import opaque to bundlers, so browser/edge builds don't try to
71+
* pull node-only packages into the bundle, while plain Node resolves it
72+
* natively at runtime.
73+
*/
6574
export async function dynamicImport<T>(module: string): Promise<T> {
6675
if (runtime === 'browser') {
6776
throw new Error('Browser runtime is not supported for dynamic import')
6877
}
6978

7079
// @ts-ignore
71-
return await import(module)
80+
return await import(/* webpackIgnore: true */ /* @vite-ignore */ module)
7281
}
7382

7483
// Source: https://github.com/chalk/ansi-regex/blob/main/index.js

0 commit comments

Comments
 (0)