Stream template build-context upload from disk instead of buffering in memory#1435
Stream template build-context upload from disk instead of buffering in memory#1435mishushakov wants to merge 8 commits into
Conversation
…uffering in memory Spool the build-context tar archive to a temporary file and stream it from disk during upload (JS and Python, sync and async), keeping the explicit Content-Length required by S3 presigned PUT URLs, which reject Transfer-Encoding: chunked with 501 NotImplemented (#1243). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: e353629 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
PR SummaryMedium Risk Overview JS: Python: Tests cover Reviewed by Cursor Bugbot for commit e353629. Bugbot is set up for automated code reviews on this repo. Configure here. |
Package ArtifactsBuilt from 534aa69. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.30.6-mishushakov-stream-template-build-upload.0.tgzCLI ( npm install ./e2b-cli-2.12.3-mishushakov-stream-template-build-upload.0.tgzPython SDK ( pip install ./e2b-2.29.5+mishushakov.stream.template.build.upload-py3-none-any.whl |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4c78c2b. Configure here.
After S3 accepts the archive, the temp-file cleanup running in the upload's `finally` block could throw (e.g. removal fails while a read stream still holds the file). That error escaped into the outer catch and was wrapped as a FileUploadError/FileUploadException, reporting a false failure for an upload that already succeeded — and on a failed upload it would overwrite the real error. Make cleanup best-effort in all three implementations (JS, Python sync and async) so it can never alter the upload result. Add regression tests covering a cleanup/close failure after a 200 response. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes the Lint and Generated-files CI checks, which run `pnpm run format` and fail on any resulting diff. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address Claude bot review on PR #1435: - In tarFileToTempFile, the failure-path `catch` did `await cleanup(); throw err`. If cleanup() rejected (e.g. EBUSY/EPERM on Windows while the tar process still holds a handle), it short-circuited and the original archive-creation error was lost. Use the same best-effort `await cleanup().catch(() => {})` pattern as uploadFile. - Update the stale uploadFile test header comment that still described the old in-memory buffering instead of the spool-and-stream approach. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirror the JS tarFileToTempFile fix in the shared tar_file_stream util (used by both sync and async). On the archive-creation failure path, `tar_file.close()` running before `raise` could itself throw (e.g. an OSError flushing the partially-written archive) and propagate as the primary exception instead of the genuine archive-creation error. Wrap the close in best-effort try/except so the real error is preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… Python)
Replace tarFileToTempFile ({ path, size, cleanup }) with tarFileToStream
({ stream, size }), mirroring the Python SDK's tar_file_stream: the
spooled archive deletes itself once the returned stream is closed
(consumed, errored, or destroyed) via its `close` event.
uploadFile no longer manages cleanup — there is no cleanup callback or
`finally` in the upload path, so a cleanup failure can no longer mask the
upload result. To preserve guaranteed removal when fetch never consumes
the body (e.g. it throws before reading), uploadFile destroys the stream
in its catch, which triggers the same `close`-based cleanup.
Node has no portable delete-on-close (Python's TemporaryFile uses
O_TEMPORARY on Windows / immediate unlink on POSIX), so cleanup is tied to
the stream lifecycle instead. Tests rewritten to consume the stream and
assert the spooled archive is removed on both consume and destroy paths.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rename tarFileToStream -> tarFileStream to match the prior naming and the Python SDK's tar_file_stream, and the local upload variable stream -> uploadStream for clarity. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Summary
Template builds previously buffered the entire gzipped build-context tar archive in memory before uploading it. This PR spools the archive to a temporary file and streams it from disk during upload — in the JS SDK and both sync and async Python SDKs — so memory usage no longer scales with the size of the build context.
The upload keeps an explicit
Content-Lengthheader (taken from the spooled file's size), which S3 presigned PUT URLs require — they rejectTransfer-Encoding: chunkedwith501 NotImplemented(#1243).Changes
packages/js-sdk/src/template/):tarFileStream/tarFileStreamUploadare replaced bytarFileToStream, which writes the archive to a temp file and returns a self-cleaning read stream plus itssize. The spooled temp file deletes itself once the stream is closed (consumed, errored, or destroyed) via the stream'scloseevent — mirroring the Python SDK'star_file_stream.buildApistreams this body withduplex: 'half'and an explicitContent-Lengthfromsize; iffetchthrows before consuming the body, it destroys the stream to trigger the same cleanup. There is no separate cleanup callback, so a cleanup failure can no longer mask the upload result.packages/python-sdk/e2b/template/utils.py,template_async/build_api.py,template_sync/build_api.py):tar_file_streamnow writes to atempfile.TemporaryFileinstead ofio.BytesIOand returns the file object positioned at the start; the upload streams from it with an explicitContent-Lengthand closes it (deleting the temp file) when done.tarFileToStream.test.ts,uploadFile.test.ts; Python upload/tar tests), including assertions that the spooled archive is removed on both the consume and destroy paths.Usage
No API changes —
Template.build()/ template builds behave the same, just without holding the build context in memory:Split out of #1433, which covers streaming for sandbox/volume file uploads and downloads.
🤖 Generated with Claude Code