Skip to content

feat: add composite upload option for large file writes#1254

Draft
mishushakov wants to merge 14 commits into
mainfrom
mishushakov/composite-upload
Draft

feat: add composite upload option for large file writes#1254
mishushakov wants to merge 14 commits into
mainfrom
mishushakov/composite-upload

fix: avoid consuming IO streams twice when data fits in single chunk

9de5bc1
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Apr 3, 2026 in 12m 53s

Code review found 2 important issues

Found 5 candidates, confirmed 3. See review comments for details.

Details

Severity Count
🔴 Important 2
🟡 Nit 0
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important packages/python-sdk/e2b/sandbox_async/filesystem/filesystem.py:418-437 Async chunk uploads block event loop and leak on gather failure
🔴 Important packages/js-sdk/src/sandbox/filesystem/index.ts:855-884 Orphaned temp chunk files not cleaned up on upload failure

Annotations

Check failure on line 437 in packages/python-sdk/e2b/sandbox_async/filesystem/filesystem.py

See this annotation in the file changed.

@claude claude / Claude Code Review

Async chunk uploads block event loop and leak on gather failure

The new `_composite_write` method introduces two related bugs in its `_upload_chunk` coroutines run via `asyncio.gather`: (1) when `use_gzip=True`, `to_upload_body(chunk_data, use_gzip)` calls `gzip.compress(raw)` synchronously—a CPU-bound operation that can take seconds on a 64MB chunk—fully blocking the asyncio event loop before the first `await`, freezing all other async operations during every chunk's compression; (2) `asyncio.gather` without `return_exceptions=True` or `TaskGroup` does not 

Check failure on line 884 in packages/js-sdk/src/sandbox/filesystem/index.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Orphaned temp chunk files not cleaned up on upload failure

The composite upload implementations in `compositeWrite` (JS) and `_composite_write` (Python sync/async) have no cleanup path on failure: if any chunk upload throws or the `/files/compose` call fails, all already-uploaded temp files at `/tmp/.e2b-upload-{uuid}-{n}` are permanently orphaned in the sandbox. The spec explicitly states source files are deleted only "after successful composition", so the server never removes them on error. Add a `try/finally` (or equivalent) block in each implementat