Skip to content

Commit 9ee4414

Browse files
mishushakovclaude
andauthored
feat(js-sdk): run the template test suite on Deno (#1595)
## What Extends the Deno vitest run (#1585) with the `template` project and fixes the real runtime bug the suite surfaced. Split out of #1594 (Bun counterpart: #1596). ```jsonc // packages/js-sdk/package.json "test:deno": "deno run -A npm:vitest run --project unit --project connectionConfig --project template", ``` ## Bug — Deno: template uploads used chunked transfer encoding Deno's native `fetch` ignores an explicit `Content-Length` header on stream bodies and falls back to `Transfer-Encoding: chunked` — exactly the failure #1243 fixed for Node, since S3-compatible presigned PUT URLs reject chunked uploads with 501. `uploadFile` now streams the spooled archive through **undici's `fetch`** (via the existing `loadUndici()` helper — undici 8 where it imports, undici 7 on Bun, global `fetch` where undici isn't resolvable, e.g. bundled apps), which honors the `Content-Length` header on stream bodies on every runtime. One upload path, no runtime sniffing. Approaches rejected along the way, all verified empirically with 1GB uploads + RSS sampling: - **File-backed `Blob` body (`fs.openAsBlob`)** — lazy on Node/Bun, but Deno's shim reads the whole file into memory eagerly (denoland/deno#32316), and Bun infers an unstrippable MIME type from the extension whose `Content-Type` breaks presigned signatures (403 against production storage). - **`node:http(s)` on Deno** — works (and is memory-bounded), but can't be unified: Bun's `node:http` ignores abort signals, and it's a second code path. Known caveat: Deno's `Readable.toWeb` shim has no backpressure, so the archive is buffered in memory during upload on Deno (Node and Bun stream in lockstep with the socket). Filed upstream as denoland/deno#36275 — accepted as Deno's to fix rather than worked around here. As part of this, `tarFileStream` became `spoolTarArchive`, returning `{ path, size, cleanup }` with caller-owned cleanup instead of a self-deleting read stream. `tests/template/uploadFile.test.ts` also asserts no `Content-Type` header is sent. ## Python SDK parity Intentionally none: `upload_file` already sends a sized file body via httpx. ## Testing - Real template builds (`tests/template/build.test.ts`, against prod S3 presigned URLs) green under **Node, Deno, and Bun** - `uploadFile` + `spoolTarArchive` suites green under Node, Deno, and Bun - `tests/template/abortSignal.test.ts` green under Deno - `pnpm build`, `lint`, `typecheck`, `prettier --check` clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 5417dd4 commit 9ee4414

7 files changed

Lines changed: 139 additions & 195 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+
Fix template file uploads under Deno. Deno's native `fetch` ignores a `Content-Length` header on stream bodies and fell back to `Transfer-Encoding: chunked`, which S3-compatible presigned upload URLs reject (see #1243). `Template.build` uploads now stream the spooled archive through undici's `fetch`, which honors the header on every runtime, falling back to the global `fetch` where undici isn't resolvable.

.github/workflows/js_sdk_tests.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ jobs:
2929
strategy:
3030
fail-fast: false
3131
# With node-only (set by staging callers) the matrix collapses to the
32-
# Node legs: Bun/Deno only run API-free unit suites and the Cloudflare
33-
# legs add sandbox load without extra backend signal, so the other
34-
# runtimes are exercised against production only.
32+
# Node legs: the Bun/Deno/Cloudflare legs re-run suites the Node legs
33+
# already cover and add sandbox/build load without extra backend
34+
# signal, so the other runtimes are exercised against production only.
3535
matrix:
3636
include: >-
3737
${{ inputs.node-only

packages/js-sdk/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
"test:bun": "bunx --bun vitest run --project unit --project connectionConfig",
4040
"test:cf": "vitest run --config tests/runtimes/cloudflare/vitest.config.mts",
4141
"test:cf:deploy": "vitest run --config tests/runtimes/cloudflare-deploy/vitest.config.mts",
42-
"test:deno": "deno run -A npm:vitest run --project unit --project connectionConfig",
42+
"test:deno": "deno run -A npm:vitest run --project unit --project connectionConfig --project template",
4343
"typecheck": "tsc --noEmit",
4444
"lint": "oxlint --config ../../.oxlintrc.json src tests",
4545
"format": "prettier --write src/ tests/ example.mts"

packages/js-sdk/src/template/buildApi.ts

Lines changed: 48 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
import type { Readable } from 'node:stream'
1+
import fs from 'node:fs'
2+
import stream from 'node:stream'
3+
24
import { ApiClient, handleApiError, components } from '../api'
35
import { buildRequestSignal } from '../connectionConfig'
4-
import { dynamicImport } from '../utils'
6+
import { loadUndici } from '../undici'
57
import { BuildError, FileUploadError, TemplateError } from '../errors'
68
import { FILE_UPLOAD_TIMEOUT_MS } from './consts'
79
import { LogEntry } from './logger'
8-
import { getBuildStepIndex, tarFileStream } from './utils'
10+
import { getBuildStepIndex, spoolTarArchive } from './utils'
911
import {
1012
BuildStatusReason,
1113
TemplateBuildStatus,
@@ -130,40 +132,27 @@ export async function uploadFile(
130132
gzip,
131133
} = options
132134
// Spool the archive to a temporary file and stream it from disk instead of
133-
// buffering in memory. S3 presigned PUT URLs reject Transfer-Encoding:
135+
// buffering it in memory. S3 presigned PUT URLs reject Transfer-Encoding:
134136
// chunked with 501 NotImplemented (see e2b-dev/e2b#1243), so the upload
135-
// sends an explicit Content-Length, which fetch honors for stream bodies.
136-
// The spooled file deletes itself once the stream is consumed, so there is
137-
// no cleanup to manage here. The Python SDK takes the same approach
138-
// (build_api.py:upload_file).
139-
let uploadStream: Readable | undefined
137+
// must carry an exact Content-Length. The Python SDK takes the same
138+
// approach (build_api.py:upload_file).
139+
let cleanup: (() => Promise<void>) | undefined
140140
try {
141-
// Dynamically import so the browser bundle doesn't pull in node:stream.
142-
const { Readable } =
143-
await dynamicImport<typeof import('node:stream')>('node:stream')
144-
145-
const tar = await tarFileStream(
141+
const tar = await spoolTarArchive(
146142
fileName,
147143
fileContextPath,
148144
ignorePatterns,
149145
resolveSymlinks,
150146
gzip
151147
)
152-
uploadStream = tar.stream
148+
cleanup = tar.cleanup
153149

154-
const res = await fetch(url, {
155-
method: 'PUT',
156-
body: Readable.toWeb(uploadStream) as ReadableStream<Uint8Array>,
157-
headers: {
158-
'Content-Length': tar.size.toString(),
159-
},
160-
// Streaming request bodies require half-duplex mode.
161-
duplex: 'half',
162-
signal: buildRequestSignal(
163-
abortOpts?.requestTimeoutMs ?? FILE_UPLOAD_TIMEOUT_MS,
164-
abortOpts?.signal
165-
),
166-
} as RequestInit)
150+
const signal = buildRequestSignal(
151+
abortOpts?.requestTimeoutMs ?? FILE_UPLOAD_TIMEOUT_MS,
152+
abortOpts?.signal
153+
)
154+
155+
const res = await putFileStream(url, tar.path, tar.size, signal)
167156

168157
if (!res.ok) {
169158
throw new FileUploadError(
@@ -172,17 +161,44 @@ export async function uploadFile(
172161
)
173162
}
174163
} catch (error) {
175-
// Ensure the spooled archive is removed even if fetch never consumed the
176-
// stream (e.g. it threw before reading the body). Destroying the stream
177-
// fires its `close` handler, which deletes the temp file.
178-
uploadStream?.destroy()
179164
if (error instanceof FileUploadError) {
180165
throw error
181166
}
182167
throw new FileUploadError(`Failed to upload file: ${error}`, stackTrace)
168+
} finally {
169+
await cleanup?.()
183170
}
184171
}
185172

173+
async function putFileStream(
174+
url: string,
175+
filePath: string,
176+
size: number,
177+
signal: AbortSignal | undefined
178+
): Promise<{ ok: boolean; statusText: string }> {
179+
// Prefer undici's fetch: it honors the explicit Content-Length on stream
180+
// bodies on every runtime, while Deno's native fetch ignores the header and
181+
// falls back to chunked. Where undici isn't resolvable (e.g. bundled apps),
182+
// fall back to the global fetch — Node's and Bun's native fetch also honor
183+
// Content-Length on stream bodies. loadUndici tries undici 8 first and
184+
// falls back to undici 7 where 8 doesn't import (Bun).
185+
const undici = await loadUndici()
186+
const fetchImpl = (undici?.fetch as typeof fetch | undefined) ?? fetch
187+
188+
return await fetchImpl(url, {
189+
method: 'PUT',
190+
body: stream.Readable.toWeb(
191+
fs.createReadStream(filePath)
192+
) as ReadableStream,
193+
headers: {
194+
'Content-Length': size.toString(),
195+
},
196+
// Streaming request bodies require half-duplex mode.
197+
duplex: 'half',
198+
signal,
199+
} as RequestInit)
200+
}
201+
186202
export async function triggerBuild(
187203
client: ApiClient,
188204
{ templateID, buildID, template }: TriggerBuildInput,

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

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import url from 'node:url'
66
import { dynamicImport } from '../utils'
77
import { TemplateError } from '../errors'
88
import { BASE_STEP_NAME, FINALIZE_STEP_NAME } from './consts'
9-
import type { Readable } from 'node:stream'
109
import type { Path } from 'glob'
1110
import type { BuildOptions } from './types'
1211

@@ -353,35 +352,31 @@ export function padOctal(mode: number): string {
353352
}
354353

355354
/**
356-
* Create a gzipped tar archive of files matching a pattern and return a
357-
* readable stream over it.
355+
* Create a gzipped tar archive of files matching a pattern, spooled to a
356+
* temporary file on disk.
358357
*
359-
* The archive is spooled to a temporary file on disk so it can be uploaded as
360-
* a stream with a known `Content-Length` instead of being buffered in memory.
361-
* The temporary file is removed automatically once the returned stream is
362-
* closed — whether it was fully read, errored, or destroyed. This mirrors the
363-
* Python SDK's `tar_file_stream`, where closing the temp-file handle deletes
364-
* it (Node has no portable delete-on-close, so cleanup is tied to the stream's
365-
* `close` event instead).
358+
* Spooling instead of buffering keeps memory bounded and gives the archive a
359+
* known size, so the upload can send an exact `Content-Length`. The caller
360+
* owns the archive's lifetime and must invoke `cleanup` once done with it.
361+
* This mirrors the Python SDK's `tar_file_stream`.
366362
*
367363
* @param fileName Glob pattern for files to include
368364
* @param fileContextPath Base directory for resolving file paths
369365
* @param ignorePatterns Ignore patterns to exclude from the archive
370366
* @param resolveSymlinks Whether to follow symbolic links
371367
* @param gzip Whether to gzip the archive
372-
* @returns The readable stream and the archive size in bytes
368+
* @returns The archive path, its size in bytes, and a cleanup callback that
369+
* removes the spooled archive. Cleanup is best-effort so it can never mask
370+
* the upload result — a leaked temp dir is non-fatal, the OS reclaims it.
373371
*/
374-
export async function tarFileStream(
372+
export async function spoolTarArchive(
375373
fileName: string,
376374
fileContextPath: string,
377375
ignorePatterns: string[],
378376
resolveSymlinks: boolean,
379377
gzip: boolean
380-
): Promise<{ stream: Readable; size: number }> {
378+
): Promise<{ path: string; size: number; cleanup: () => Promise<void> }> {
381379
const { create } = await dynamicImport<typeof import('tar')>('tar')
382-
// Dynamically import so the browser bundle doesn't pull in node:fs.
383-
const { createReadStream } =
384-
await dynamicImport<typeof import('node:fs')>('node:fs')
385380

386381
const allFiles = await getAllFilesInPath(
387382
fileName,
@@ -396,9 +391,7 @@ export async function tarFileStream(
396391
path.join(os.tmpdir(), 'e2b-template-')
397392
)
398393
const tarPath = path.join(tmpDir, 'context.tar.gz')
399-
// Best-effort removal so it can never mask the archive-creation error or the
400-
// upload result. A leaked temp dir is non-fatal — the OS reclaims it.
401-
const removeTmpDir = () =>
394+
const cleanup = () =>
402395
fs.promises.rm(tmpDir, { recursive: true, force: true }).catch(() => {})
403396

404397
try {
@@ -414,14 +407,9 @@ export async function tarFileStream(
414407
)
415408

416409
const { size } = await fs.promises.stat(tarPath)
417-
418-
const stream = createReadStream(tarPath)
419-
// Remove the spooled archive once the stream is done — `close` fires after
420-
// the fd is released on every path (end, error, or destroy).
421-
stream.once('close', removeTmpDir)
422-
return { stream, size }
410+
return { path: tarPath, size, cleanup }
423411
} catch (err) {
424-
await removeTmpDir()
412+
await cleanup()
425413
throw err
426414
}
427415
}

packages/js-sdk/tests/template/uploadFile.test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ import { uploadFile } from '../../src/template/buildApi'
1010
// Readable directly to fetch, which made undici fall back to
1111
// Transfer-Encoding: chunked. S3 presigned PUT URLs reject that with 501
1212
// NotImplemented. The fix spools the archive to a temporary file and streams
13-
// it from disk with an explicit Content-Length.
13+
// it from disk with an explicit Content-Length via undici's fetch, which
14+
// honors the header on stream bodies on every runtime (Deno's native fetch
15+
// ignores it and chunks). This suite runs under both Node and Deno.
1416
describe('uploadFile transfer encoding', () => {
1517
let testDir: string
1618
let server: Server
@@ -66,5 +68,10 @@ describe('uploadFile transfer encoding', () => {
6668
if (transferEncoding !== undefined) {
6769
expect(transferEncoding.toLowerCase()).not.toContain('chunked')
6870
}
71+
72+
// Presigned upload URLs sign the request headers, so an implicit
73+
// Content-Type (e.g. inferred from the archive's file extension) makes
74+
// the storage backend reject the upload with 403 Forbidden.
75+
expect(capturedHeaders['content-type']).toBeUndefined()
6976
})
7077
})

0 commit comments

Comments
 (0)