-
Notifications
You must be signed in to change notification settings - Fork 1k
feat(js-sdk)!: drop undici 7 and the in-flight cap, depend on undici@^8 only #1733
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
c19158a
389ee00
245f93b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| 'e2b': major | ||
| --- | ||
|
|
||
| Drop the dual `undici`/`undici8` dependency: the SDK now depends on `undici@^8` only and requires Node.js >= 22.19.0. undici 8 honors the server's `SETTINGS_MAX_CONCURRENT_STREAMS` per HTTP/2 connection and opens additional connections when a connection's streams are saturated, instead of undici 7's one-request-per-connection multiplexing. On Node.js older than 22.19.0 the SDK no longer loads undici and falls back to the global fetch. | ||
|
|
||
| Remove the SDK-level in-flight request cap: `limitConcurrency` and the `E2B_API_INFLIGHT_REQUESTS`, `E2B_ENVD_INFLIGHT_REQUESTS`, and `E2B_ENVD_RPC_INFLIGHT_REQUESTS` env vars are gone. Concurrency is now governed by undici's dispatcher (per-connection HTTP/2 stream limits from the server plus the `E2B_API_CONNECTIONS`/`E2B_ENVD_RPC_CONNECTIONS` connection pools). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| deno 2.8.1 | ||
| nodejs 22.18.0 | ||
| nodejs 22.19.0 | ||
| pnpm 10.34.5 | ||
| python 3.10 | ||
| uv 0.10.0 |
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,5 @@ | ||
| import { compareVersions } from 'compare-versions' | ||
|
|
||
| import { limitConcurrency } from './api/inflight' | ||
| import { isReadableStreamLike, isRequestLike } from './is' | ||
| import { dynamicImport, toDispatchableStream } from './utils' | ||
|
|
||
|
|
@@ -20,22 +19,22 @@ export type UndiciModule = { | |
| fetch: unknown | ||
| } | ||
|
|
||
| const UNDICI_8_MIN_NODE = '22.19.0' | ||
| const UNDICI_MIN_NODE = '22.19.0' | ||
|
|
||
| export function getUndiciPackageCandidates(nodeVersion: string): string[] { | ||
| if (compareVersions(nodeVersion, UNDICI_8_MIN_NODE) >= 0) { | ||
| return ['undici8', 'undici'] | ||
| if (compareVersions(nodeVersion, UNDICI_MIN_NODE) >= 0) { | ||
| return ['undici'] | ||
| } | ||
|
|
||
| return ['undici'] | ||
| return [] | ||
|
Comment on lines
+25
to
+29
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a TASTE rule, a behavior regression the diff makes easy to miss. Also, now that the list can hold at most one entry, the plural |
||
| } | ||
|
|
||
| export async function loadUndici(): Promise<UndiciModule | undefined> { | ||
| for (const packageName of getUndiciPackageCandidates(process.versions.node)) { | ||
| try { | ||
| return await dynamicImport<UndiciModule>(packageName) | ||
| } catch { | ||
| // Try the next package supported by this Node version. | ||
| // Fall back to the global fetch when undici cannot load. | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -88,20 +87,18 @@ export function createRuntimeFetch( | |
|
|
||
| /** | ||
| * Build a fetch bound to a bounded undici dispatcher (HTTP/2 enabled, | ||
| * `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. | ||
| * `connections` origin connections, optional proxy tunnel). Falls back to | ||
| * the global fetch when undici cannot be loaded. | ||
| */ | ||
| export async function buildDispatchedFetch(options: { | ||
| connections: number | ||
| inflightLimit: number | ||
| proxy?: string | ||
| loadUndici?: () => Promise<UndiciModule | undefined> | ||
| }): Promise<typeof fetch> { | ||
| const undici = await (options.loadUndici ?? loadUndici)() | ||
|
|
||
| if (!undici) { | ||
| return limitConcurrency(lateBoundGlobalFetch(), options.inflightLimit) | ||
| return lateBoundGlobalFetch() | ||
| } | ||
|
|
||
| const { Agent, ProxyAgent, fetch: undiciFetch } = undici | ||
|
|
@@ -121,16 +118,14 @@ export async function buildDispatchedFetch(options: { | |
| init?: UndiciRequestInit | ||
| ) => Promise<Response> | ||
|
|
||
| const wrapped: typeof fetch = ((input, init) => { | ||
| return ((input, init) => { | ||
| const request = toUndiciRequestInput(input, init) | ||
|
|
||
| return fetchWithDispatcher(request.input, { | ||
| ...request.init, | ||
| dispatcher, | ||
| }) | ||
| }) as typeof fetch | ||
|
|
||
| return limitConcurrency(wrapped, options.inflightLimit) | ||
| } | ||
|
|
||
| function toUndiciRequestInput( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the changesets release process updates
@e2b/cli'sworkspace:^dependency to this new SDK major, the CLI will still advertise Node>=20.18.1 <21 || >=22.9.0, while this dependency rejects every version below 22.19.0. Consequently, installing the current CLI on its documented Node 20 or Node 22.9–22.18 range fails for users with strict engine enforcement (including this repository's pnpm configuration), before the intended global-fetch fallback can run. Update the CLI engine range and changeset/tests as part of this breaking release, or retain a compatible SDK engine range for the fallback path.Useful? React with 👍 / 👎.