Skip to content

Commit 1ae3f92

Browse files
mishushakovclaude
andauthored
feat(js-sdk): run the full unit test suite in Cloudflare workerd (#1593)
## What Promotes `test:cf` from a single dist smoke test to the **full unit + connectionConfig suite running inside Cloudflare's workerd** (`@cloudflare/vitest-pool-workers`) — the same coverage `test:bun` and `test:deno` get. Locally: **74 files / 393 tests green** against prod sandboxes. The real-deploy suite (`test:cf:deploy`) is unchanged and keeps covering the built bundle on actual Cloudflare infrastructure (the pool can't reproduce bundling bugs like #1579). ## SDK fixes the suite surfaced 1. **Dropped-connection mapping for Workers** (`src/envd/rpc.ts`): workerd surfaces a sandbox connection drop as `Network connection lost`, which fell through to a cryptic `SandboxError`. It's now matched like the Node/Bun/Deno variants, so killing a sandbox mid-request surfaces as the health-checked `TimeoutError`: ```ts const cmd = await sandbox.commands.run('sleep 60', { background: true }) await sandbox.kill() await cmd.wait() // now rejects with TimeoutError('…sandbox was killed or reached its end of life…') on Workers too ``` 2. **Double connection release on stream cancel** (`src/connectionConfig.ts`): `wrapStreamWithConnectionCleanup` claimed its `release` was idempotent but had no guard — cancelling a streamed download while a read was in flight ran `cleanup()` twice (both the `cancel` callback and the pending `pull` resolving `done` fire). workerd's stream scheduling hits this deterministically; the pooled connection was double-released. Both are runtime-behavior fixes specific to the JS fetch/streams stack — no Python SDK equivalent applies. ## Test adjustments - **boot_id reads** in the two "filesystem-only pause" tests now use `commands.run('cat …')` instead of `files.read`: envd's non-gzip download path serves procfs files as an empty 200 (filed as e2b-dev/infra#3363 — Go `ServeContent` sizes them by stat, which is 0). Only clients that don't negotiate gzip (workerd's fetch) observe it; the command path sidesteps the bug while keeping the reboot assertion on all runtimes. - **runtime.test.ts** Node-host detection scenarios skip under workerd via the existing host guard (same treatment as Bun/Deno). - **Pool config filters expected unhandled-rejection shapes** via vitest's `onUnhandledError` (not the blanket `dangerouslyIgnoreUnhandledErrors`): workerd reports a rejection as unhandled unless a handler attaches within the same microtask drain — even inline `await expect(op()).rejects` trips it — and vitest never processes the `rejectionhandled` retraction on any runtime, so the suite's deliberate rejections false-positive ~60× per run. A diagnostic pairing `unhandledrejection` with `rejectionhandled` confirmed all of them are handled-late false positives (zero genuine leaks). The filter drops only the shapes the tests provoke (SDK error classes, `ConnectError`, `AbortError`, workerd's `Network connection lost.`, one test stub); unknown rejection shapes and uncaught exceptions still fail the run — verified with a planted never-handled `TypeError` (exit 1). ## CI Rebased onto #1588's per-runtime matrix: the `cloudflare` leg (already ubuntu-only there) now runs the full suite; no extra jobs added. The stale `tests/integration` exclude was dropped after #1591 removed that suite. ## Notes - Suite config needs `nodejs_compat_populate_process_env` + `E2B_API_KEY`/`E2B_DOMAIN` miniflare bindings so the SDK and tests read env like on Node. - The deleted `tests/runtimes/cloudflare/run.test.ts` (dist smoke) is fully subsumed: lifecycle coverage by the suite, bundle coverage by `test:cf:deploy` + `tests/bundle/edgeCompat.test.ts`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 3f46d56 commit 1ae3f92

11 files changed

Lines changed: 112 additions & 80 deletions

File tree

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+
Recognize Cloudflare Workers' `Network connection lost` as a dropped sandbox connection so a sandbox killed mid-request surfaces as the health-checked `TimeoutError` (matching Node/Bun/Deno), and fix streaming downloads releasing their pooled connection twice when cancelled while a read was in flight

.github/workflows/js_sdk_tests.yml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ jobs:
8686
path: ${{ matrix.os == 'windows-latest' && '~/AppData/Local/ms-playwright' || '~/.cache/ms-playwright' }}
8787
key: playwright-${{ runner.os }}-${{ steps.playwright-version.outputs.version }}
8888

89-
# Every suite needs dist/: the unit bundle test and both Cloudflare
90-
# configs fail in CI when the build output is missing.
89+
# The unit bundle test and the Cloudflare deploy config fail in CI when
90+
# the build output is missing.
9191
- name: Test build
9292
run: pnpm build
9393

@@ -122,7 +122,8 @@ jobs:
122122
E2B_API_KEY: ${{ secrets.E2B_API_KEY }}
123123
E2B_DOMAIN: ${{ inputs.E2B_DOMAIN }}
124124

125-
- name: Run Cloudflare Workers tests
125+
# Full unit + connectionConfig suite inside workerd (vitest-pool-workers).
126+
- name: Run test suite under Cloudflare workerd
126127
if: matrix.runtime == 'cloudflare'
127128
run: pnpm test:cf
128129
env:

packages/js-sdk/src/connectionConfig.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,8 +295,15 @@ export function wrapStreamWithConnectionCleanup(
295295
const reader = body.getReader()
296296
const idle = createIdleAbort(controller, idleTimeoutMs, 'Stream')
297297

298-
// Idempotent: safe to call from multiple stream callbacks.
298+
// Idempotent: safe to call from multiple stream callbacks — cancelling
299+
// while a pull is in flight settles both paths (the pending read resolves
300+
// `done` after `reader.cancel()`), which must not release twice.
301+
let released = false
299302
const release = () => {
303+
if (released) {
304+
return
305+
}
306+
released = true
300307
idle.clear()
301308
cleanup()
302309
}

packages/js-sdk/src/envd/rpc.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,16 @@ export type SandboxHealthCheck = () => Promise<boolean | undefined>
2525
* is dropped mid-request. The transport surfaces a dropped connection (e.g. an
2626
* HTTP/2 stream reset) with runtime- and version-specific wording, so we match
2727
* every known variant:
28-
* - Node (undici): `terminated`
29-
* - Bun: `The socket connection was closed unexpectedly`
30-
* - Deno: `error reading a body from connection`
28+
* - Node (undici): `terminated`
29+
* - Bun: `The socket connection was closed unexpectedly`
30+
* - Deno: `error reading a body from connection`
31+
* - Cloudflare Workers: `Network connection lost`
3132
*/
3233
const CONNECTION_TERMINATED_MESSAGES = [
3334
'terminated',
3435
'The socket connection was closed unexpectedly',
3536
'error reading a body from connection',
37+
'Network connection lost',
3638
]
3739

3840
/**

packages/js-sdk/tests/envd/handleEnvdApiError.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ describe('handleEnvdApiFetchError', () => {
125125
Node: new TypeError('terminated'),
126126
Bun: new Error('The socket connection was closed unexpectedly'),
127127
Deno: new TypeError('error reading a body from connection'),
128+
'Cloudflare Workers': new Error('Network connection lost.'),
128129
}
129130

130131
for (const [runtime, error] of Object.entries(runtimeTerminatedErrors)) {

packages/js-sdk/tests/envd/handleRpcError.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ describe('handleRpcErrorWithHealthCheck', () => {
6969
Node: 'terminated',
7070
Bun: 'The socket connection was closed unexpectedly',
7171
Deno: 'error reading a body from connection',
72+
'Cloudflare Workers': 'Network connection lost.',
7273
}
7374

7475
test('returns a TimeoutError when the health check says the sandbox is not running', async () => {

packages/js-sdk/tests/runtime.test.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import { afterEach, expect, test, vi } from 'vitest'
22

3-
// The unit project also runs under Bun (and prospectively Deno), where the
4-
// host runtime's own unstubbable marker (globalThis.Bun / globalThis.Deno)
5-
// correctly wins detection — these scenarios only exist on a Node host.
3+
// The unit project also runs under Bun, Deno, and Cloudflare's workerd, where
4+
// the host runtime's own unstubbable marker (globalThis.Bun / globalThis.Deno /
5+
// the Cloudflare-Workers user agent) correctly wins detection — these
6+
// scenarios only exist on a Node host.
67
const isNodeHost =
78
typeof (globalThis as any).Bun === 'undefined' &&
8-
typeof (globalThis as any).Deno === 'undefined'
9+
typeof (globalThis as any).Deno === 'undefined' &&
10+
(globalThis as any).navigator?.userAgent !== 'Cloudflare-Workers'
911

1012
afterEach(() => {
1113
vi.unstubAllGlobals()

packages/js-sdk/tests/runtimes/cloudflare/run.test.ts

Lines changed: 0 additions & 37 deletions
This file was deleted.
Lines changed: 65 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,43 @@
1-
import { existsSync } from 'node:fs'
2-
import path from 'node:path'
3-
import { fileURLToPath } from 'node:url'
4-
51
import { cloudflareTest } from '@cloudflare/vitest-pool-workers'
62
import { config } from 'dotenv'
73
import { defineConfig } from 'vitest/config'
84

95
const env = config()
106

11-
// The suite smoke-tests the built ESM bundle (what Workers users consume),
12-
// so dist must exist. Skip locally to keep plain `vitest` runs green, but
13-
// fail loudly in CI where the build step is expected to have run.
14-
const distEntry = path.resolve(
15-
path.dirname(fileURLToPath(import.meta.url)),
16-
'../../../dist/index.mjs'
17-
)
18-
const hasDist = existsSync(distEntry)
19-
20-
if (!hasDist) {
21-
const message =
22-
'dist/index.mjs not found — run `pnpm build` in packages/js-sdk before running the Cloudflare Workers tests'
23-
if (process.env.CI) {
24-
throw new Error(message)
25-
}
26-
console.warn(`Skipping Cloudflare Workers tests: ${message}`)
27-
}
7+
// Error names thrown by src/errors.ts (plus CommandExitError) — the shapes
8+
// this suite's rejection tests expect. Kept as a literal list so an unknown
9+
// error class still fails the run instead of being silently ignored.
10+
const SDK_ERROR_NAMES = new Set([
11+
'SandboxError',
12+
'TimeoutError',
13+
'InvalidArgumentError',
14+
'NotEnoughSpaceError',
15+
'NotFoundError',
16+
'FileNotFoundError',
17+
'SandboxNotFoundError',
18+
'AuthenticationError',
19+
'GitAuthError',
20+
'GitUpstreamError',
21+
'TemplateError',
22+
'RateLimitError',
23+
'BuildError',
24+
'FileUploadError',
25+
'VolumeError',
26+
'CommandExitError',
27+
])
2828

29+
// Runs the unit + connectionConfig projects (same coverage as test:bun /
30+
// test:deno) inside Cloudflare's workerd via vitest-pool-workers, against src.
31+
// The real-deploy suite (tests/runtimes/cloudflare-deploy) keeps covering the
32+
// built bundle on actual Cloudflare infrastructure.
2933
export default defineConfig({
3034
plugins: [
3135
cloudflareTest({
3236
miniflare: {
3337
compatibilityDate: '2026-03-01',
38+
// nodejs_compat is a hard requirement for the SDK on Workers. It also
39+
// mirrors the bindings below into process.env (default since compat
40+
// date 2025-04-01), which is how tests and the SDK read E2B_*.
3441
compatibilityFlags: ['nodejs_compat'],
3542
bindings: {
3643
E2B_API_KEY: process.env.E2B_API_KEY ?? env.parsed?.E2B_API_KEY ?? '',
@@ -41,8 +48,43 @@ export default defineConfig({
4148
],
4249
test: {
4350
name: 'cloudflare',
44-
include: hasDist ? ['tests/runtimes/cloudflare/**/*.test.ts'] : [],
45-
passWithNoTests: !hasDist,
51+
include: ['tests/**/*.test.ts'],
52+
exclude: [
53+
'tests/runtimes/**',
54+
'tests/template/**',
55+
// Inspects the host-built dist/index.mjs via node:fs, which workerd's
56+
// virtual filesystem can never see (and throws in CI when the file is
57+
// "missing"); the Node unit project keeps running it.
58+
'tests/bundle/**',
59+
],
60+
globals: false,
4661
testTimeout: 30_000,
62+
bail: 0,
63+
// workerd reports a rejection as unhandled unless a handler is attached
64+
// within the same microtask drain, and vitest never processes the
65+
// rejectionhandled retraction, so rejections the tests DO handle (the
66+
// `await expect(p).rejects` pattern, including inline ones — workerd also
67+
// flags intermediate promises of the async-function chain) false-positive
68+
// ~60 times per run. Instead of dangerouslyIgnoreUnhandledErrors, drop
69+
// only the rejection shapes these tests deliberately provoke; uncaught
70+
// exceptions and any unknown rejection shape still fail the run.
71+
onUnhandledError(error) {
72+
const message = String(error.message ?? '')
73+
const expectedRejection =
74+
error.type === 'Unhandled Rejection' &&
75+
// SDK public errors — what `expect(p).rejects` asserts on.
76+
(SDK_ERROR_NAMES.has(error.name) ||
77+
// The transport errors the SDK wraps: connect-rpc failures and
78+
// aborted requests surface on intermediate promises too.
79+
error.name === 'ConnectError' ||
80+
message.startsWith('ConnectError:') ||
81+
error.name === 'AbortError' ||
82+
// workerd's teardown error for in-flight streams when a test kills
83+
// the sandbox mid-request.
84+
message === 'Network connection lost.' ||
85+
// Stub rejection from tests/sandbox/git/validation.test.ts.
86+
message === 'commands.run should not be called')
87+
if (expectedRejection) return false
88+
},
4789
},
4890
})

packages/js-sdk/tests/sandbox/lifecyclePayload.test.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,13 @@ test.skipIf(isDebug)(
7979
try {
8080
const marker = 'auto-pause-fs-only'
8181
await sandbox.files.write('/home/user/auto-pause-marker.txt', marker)
82+
// Read via a command, not files.read: envd's non-gzip download path
83+
// serves procfs files as an empty 200 (it sizes them by stat, which is
84+
// 0), so clients that don't negotiate gzip — like workerd's fetch —
85+
// silently get '' (infra#3363).
8286
const bootBefore = (
83-
await sandbox.files.read('/proc/sys/kernel/random/boot_id')
84-
).trim()
87+
await sandbox.commands.run('cat /proc/sys/kernel/random/boot_id')
88+
).stdout.trim()
8589

8690
await wait(5_000)
8791

@@ -97,8 +101,8 @@ test.skipIf(isDebug)(
97101
assert.equal(persisted, marker)
98102

99103
const bootAfter = (
100-
await sandbox.files.read('/proc/sys/kernel/random/boot_id')
101-
).trim()
104+
await sandbox.commands.run('cat /proc/sys/kernel/random/boot_id')
105+
).stdout.trim()
102106
assert.notEqual(bootAfter, bootBefore)
103107
} finally {
104108
await sandbox.kill().catch(() => {})

0 commit comments

Comments
 (0)