Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/metal-pandas-brake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'e2b': patch
---

Detect `Request` inputs by shape instead of `instanceof` in the Node fetch bridge. A `Request` minted by a different `Request` class — runtimes and tools replace `globalThis.Request` the same way they replace `globalThis.fetch` (test environments, instrumentation, server shims such as `@hono/node-server`, or two coexisting copies of one shim) — previously failed the brand check, was passed to undici's `fetch` verbatim, and crashed every API call with `Failed to parse URL from [object Request]`. Such Requests are now destructured into a plain `(url, init)` pair like any other, and their abort signal is honored while queued by the in-flight limiter.
7 changes: 6 additions & 1 deletion packages/js-sdk/src/api/inflight.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { isRequestLike } from '../utils'

/**
* Simple FIFO semaphore used to cap the number of in-flight requests sent
* through a fetch dispatcher.
Expand Down Expand Up @@ -66,8 +68,11 @@ export function limitConcurrency(
const sem = new Semaphore(max)

return (async (input, init) => {
// Shape check, not `instanceof` — a foreign-class Request (replaced or
// duplicated global Request) still carries the abort signal we must honor
// while queued (see isRequestLike).
const signal =
init?.signal ?? (input instanceof Request ? input.signal : undefined)
init?.signal ?? (isRequestLike(input) ? input.signal : undefined)
const release = await sem.acquire(signal)
try {
return await fetcher(input, init)
Expand Down
8 changes: 6 additions & 2 deletions packages/js-sdk/src/undici.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { compareVersions } from 'compare-versions'

import { limitConcurrency } from './api/inflight'
import { dynamicImport } from './utils'
import { dynamicImport, isRequestLike } from './utils'

type UndiciRequestInit = RequestInit & {
dispatcher?: unknown
Expand Down Expand Up @@ -136,7 +136,11 @@ function toUndiciRequestInput(
input: RequestInfo | URL,
init?: RequestInit
): { input: RequestInfo | URL; init?: RequestInit & { duplex?: 'half' } } {
if (!(input instanceof Request)) {
// Shape check, not `instanceof`: a Request minted by a replaced/duplicated
// global Request class must still be destructured here — passed through
// verbatim, undici's fetch would coerce it to a URL string and crash (see
// isRequestLike).
if (!isRequestLike(input)) {
return { input, init }
}

Expand Down
24 changes: 24 additions & 0 deletions packages/js-sdk/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,30 @@ export function timeoutToSeconds(timeout: number): number {
return Math.ceil(timeout / 1000)
}

/**
* Whether `input` should be treated as a `Request`, by shape rather than
* brand. A bare `input instanceof Request` misses Requests minted by a
* *different* `Request` class: runtimes and tools replace
* `globalThis.Request` just like they replace `globalThis.fetch` (test
* environments, instrumentation, server shims such as `@hono/node-server` —
* and two copies of one shim can even coexist in a process, each with its own
* class, so a Request built by one fails `instanceof` against the other).
*
* The miss is not benign where the SDK bridges into undici: a foreign Request
* passed to undici's `fetch` verbatim fails undici's own brand check too and
* gets coerced to a URL string, crashing with
* `Failed to parse URL from [object Request]`.
*/
export function isRequestLike(input: unknown): input is Request {
if (input instanceof Request) return true
return (
typeof input === 'object' &&
input !== null &&
typeof (input as Request).url === 'string' &&
typeof (input as Request).method === 'string'
)
}

/**
* Import an optional, runtime-resolved package (e.g. `undici`, `glob`, `tar`)
* without letting downstream bundlers resolve it at build time.
Expand Down
31 changes: 31 additions & 0 deletions packages/js-sdk/tests/api/inflight.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,34 @@ test('limitConcurrency aborts queued requests when their signal fires', async ()
const resp = await first
expect(await resp.text()).toBe('done')
})

test('limitConcurrency honors the abort signal of a Request minted by a foreign Request class', async () => {
// Same real-world shape as the undici.test.ts foreign-Request case:
// `globalThis.Request` gets replaced (or duplicated) by a shim, so a
// Request built from a sibling class fails `instanceof` — but its abort
// signal must still count while the request waits for a slot.
const NativeRequest = globalThis.Request
class MintingRequest extends NativeRequest {}
class GlobalShimRequest extends NativeRequest {}

const inner = vi.fn(async () => new Response('ok')) as unknown as typeof fetch
const limited = limitConcurrency(inner, 1)

const controller = new AbortController()
controller.abort()
const request = new MintingRequest('https://example.com/aborted', {
signal: controller.signal,
})

vi.stubGlobal('Request', GlobalShimRequest)
try {
expect(request instanceof globalThis.Request).toBe(false)
await expect(
limited(request as unknown as RequestInfo)
).rejects.toMatchObject({ name: 'AbortError' })
} finally {
vi.unstubAllGlobals()
}

expect(inner).not.toHaveBeenCalled()
})
51 changes: 51 additions & 0 deletions packages/js-sdk/tests/undici.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { expect, test, vi } from 'vitest'

import {
buildDispatchedFetch,
createRuntimeFetch,
getUndiciPackageCandidates,
loadUndici,
type UndiciModule,
} from '../src/undici'
import { runtime } from '../src/utils'

Expand Down Expand Up @@ -53,6 +55,55 @@ test('a stale awaiter of a failed build does not clobber a newer successful buil
expect(build).toHaveBeenCalledTimes(2)
})

// A Request minted by a different Request class than the one `instanceof`
// checks against. This happens in real processes: runtimes and tools replace
// `globalThis.Request` just like they replace `globalThis.fetch` (test
// environments, instrumentation, server shims such as @hono/node-server), and
// two copies of one shim can coexist — each with its own class. Sibling
// subclasses of the native Request reproduce exactly that: an instance of one
// is a fully functional Request but fails `instanceof` against the other.
test('destructures a Request minted by a foreign Request class instead of passing it to undici verbatim', async () => {
const NativeRequest = globalThis.Request
class MintingRequest extends NativeRequest {}
class GlobalShimRequest extends NativeRequest {}

const seen: Array<{ input: unknown; init?: RequestInit }> = []
const fakeUndici = {
Agent: class {},
ProxyAgent: class {},
fetch: async (input: unknown, init?: RequestInit) => {
seen.push({ input, init })
return new Response('ok')
},
} as unknown as UndiciModule

const request = new MintingRequest('https://api.example.test/sandboxes', {
method: 'POST',
})

vi.stubGlobal('Request', GlobalShimRequest)
try {
// The premise: a real Request that the current global class disowns.
expect(request instanceof globalThis.Request).toBe(false)

const fetcher = await buildDispatchedFetch({
connections: 1,
inflightLimit: 0,
loadUndici: async () => fakeUndici,
})
await fetcher(request as unknown as RequestInfo)
} finally {
vi.unstubAllGlobals()
}

// Undici's fetch cannot brand-check a foreign Request either — passed
// through verbatim it would be coerced to a URL string and crash with
// `Failed to parse URL from [object Request]`. It must arrive destructured.
expect(seen).toHaveLength(1)
expect(seen[0].input).toBe('https://api.example.test/sandboxes')
expect(seen[0].init?.method).toBe('POST')
})

// loadUndici is only reached in production when the runtime is Node; other
// runtimes (Bun, Deno) use their global fetch instead.
test.skipIf(runtime !== 'node')(
Expand Down