fix(sdk): buffer template upload to set Content-Length, add regression tests#1294
fix(sdk): buffer template upload to set Content-Length, add regression tests#1294mishushakov merged 6 commits intomainfrom
Conversation
uploadFile passed a Node Readable directly to fetch, which undici turns into Transfer-Encoding: chunked when Content-Length is unknown. S3 presigned PUT URLs reject chunked requests with 501 NotImplemented, which breaks template uploads against S3-compatible storage in self-hosted deployments (issue #1243). Buffer the archive via stream/consumers.buffer() before PUT so fetch sets Content-Length automatically. Aligns with the Python SDK, which already materializes the tar into io.BytesIO and uploads bytes via httpx. Adds a regression test that spins up a local HTTP server, runs uploadFile against it, and asserts Content-Length is set and matches the received body length. Fixes #1243
Use mkdtemp for atomic unique temp dir and lift server/fixture lifecycle to beforeAll/afterAll since there's only one test. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The cli's typecheck (without skipLibCheck on the js-sdk source it imports via path mapping) didn't narrow tar's Pack to the AsyncIterable<Buffer> overload of node:stream/consumers' buffer(). Cast via unknown to satisfy both the cli and js-sdk typechecks. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Static import broke the browser build — vite externalized the module and threw at import time for browser test files that transitively pulled in buildApi. Defer the import inside uploadFile like tarFileStream already does for the tar package. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ngth Guards against future changes swapping the buffered bytes for a generator/stream, which would trigger Transfer-Encoding: chunked and break S3 presigned PUT uploads with 501 NotImplemented. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PR SummaryMedium Risk Overview Adds regression tests: a new JS test that spins up a local HTTP server to assert Reviewed by Cursor Bugbot for commit 1323a27. Bugbot is set up for automated code reviews on this repo. Configure here. |
🦋 Changeset detectedLatest commit: 1323a27 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Package ArtifactsBuilt from 0b91e8f. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.19.1-mishushakov-s3-upload-fix.0.tgzCLI ( npm install ./e2b-cli-2.9.1-mishushakov-s3-upload-fix.0.tgzPython SDK ( pip install ./e2b-2.20.0+mishushakov.s3.upload.fix-py3-none-any.whl |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1323a2767a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — small, well-scoped fix with solid regression coverage.
Extended reasoning...
Overview
The PR changes uploadFile in packages/js-sdk/src/template/buildApi.ts to buffer the tar archive via node:stream/consumers.buffer before calling fetch, so undici sets Content-Length instead of falling back to Transfer-Encoding: chunked (which S3 presigned PUT URLs reject with 501). It adds a JS regression test that spins up a local HTTP server and asserts the headers, plus sync+async Python regression tests guarding the existing Python behavior (no Python code change).
Security risks
None identified. This is a client-side HTTP behavior fix — no auth/crypto/permission surface touched. The dynamic import of node:stream/consumers is a hardcoded module string, not user-controlled.
Level of scrutiny
Low-to-medium. The change is ~10 lines of production code in a single SDK function with a clearly stated motivation (GitHub issue #1243) and an obvious matching fix pattern already used by the Python SDK. The tradeoff flagged by cursor[bot] — buffering large templates in memory — is real but already the status quo on the Python side, so this PR simply aligns the two SDKs rather than introducing a novel design decision.
Other factors
The cast uploadStream as unknown as AsyncIterable<Buffer> is explained in-code with a clear justification tied to preserveSymlinks in the CLI's tsconfig; tar's Pack is indeed async-iterable at runtime via Minipass. The dynamic import keeps the browser bundle clean. No outstanding human review comments, and the bug-hunting system found no issues.
3 participants
Summary
Consolidates the fix and tests from #1285 and #1293 into a single PR.
uploadFileused to pass a NodeReadabledirectly tofetch, causing undici to fall back toTransfer-Encoding: chunked. S3 presigned PUT URLs reject chunked with 501 NotImplemented. Fix buffers the archive first soContent-Lengthis set. Includes:Content-Lengthis set and matches the body, andTransfer-Encodingis not chunked.Pack→AsyncIterable<Buffer>).node:stream/consumersso the browser bundle doesn't pull it in.upload_filethat guard against the same class of bug (someone swappingtar_buffer.getvalue()for a stream/generator). No Python code change — the current implementation already passes bytes tohttpx.put(..., content=...).Authorship of the original JS fix commit preserved (truffle-dev).
Closes #1243.
Test plan
pnpm run test tests/template/uploadFile.test.ts— passespnpm run typecheck/lintclean across js-sdk and clipoetry run pytest tests/sync/template_sync/test_upload_file.py tests/async/template_async/test_upload_file.py -v— both passpoetry run make format/make lint/make typecheckclean🤖 Generated with Claude Code