Skip to content

fix(python-sdk): keep streamed uploads off the retrying transport - #1661

Draft
mishushakov wants to merge 2 commits into
python-sdk-unify-pyqwest-connection-pools-once-all-http-sdk-291from
fix-retry-body-buffering
Draft

fix(python-sdk): keep streamed uploads off the retrying transport#1661
mishushakov wants to merge 2 commits into
python-sdk-unify-pyqwest-connection-pools-once-all-http-sdk-291from
fix-retry-body-buffering

Conversation

@mishushakov

@mishushakov mishushakov commented Aug 11, 2026

Copy link
Copy Markdown
Member

Stacked on #1659.

pyqwest.middleware.retry makes a streamed request body replayable by growing a bytearray copy of it as the body is sent, for every body that isn't already bytes. So an upload was streamed to the wire and mirrored whole in RAM. Measured on main with volume.write_file(path, file_object) and a 64 MiB file: Content-Length: 67108864 with no chunked encoding and the server reading incrementally — genuinely streamed — while 67,108,864 bytes landed in RetryingRequestContent._buffer, for a traced peak of 70 MB. The copy is a tee rather than a replacement for streaming, which is why nothing in volume_sync.py or the filesystem write path looks wrong.

The copy is what makes the retries work, so it can't just be removed. reqwest reads ahead into its own body channel while the connection is being established, so a connect error surfaces with the iterator already started and nothing to rewind. Measured against a refused port on pyqwest 0.9.0, chunks pulled from the body before ConnectionError:

body sync async
8 × 1 KiB 2, 2, 2 8, 8, 8 (all of it)
8 × 1 MiB 2, 2, 2 1, 2, 3
1 × 200 B (unary-RPC shaped) 1, 1, 1 1, 1, 1

That last row is why the retry layer has to stay for RPC traffic: connectrpc hands pyqwest a generator even for unary calls, and without the copy every envd RPC would silently lose its connect retries.

So the uploads move instead of the copy. get_upload_transport is the httpx adapter over the same connection pool with the retry layer left out, and the three upload paths take it when their body is streamed:

  • envd files.write — a third sibling client next to the retrying and streaming ones (_envd_api_upload), used on both the octet-stream and multipart paths.
  • volume.write_fileget_upload_api_client, the volume content client on that transport.
  • template context uploads — which as a side effect stop building a reqwest pool per build, the workaround refactor(python-sdk): unify the pyqwest connection pools #1659 called out in its review notes.

Sharing the pool is the point: an upload still travels the sandbox's or the volume host's pooled connections, and a health probe after a failed RPC still lands on the same connection. What a streamed upload gives up is the connect retry, which fires before any of the body was written, so the caller sees the connection error intact and can retry the upload itself. Writes of in-memory data (files.write(path, "text"), bytes) stay on the retrying transport — a bytes body is replayed without a copy, so those retries cost nothing and are kept.

No user-facing API change, so there are no usage examples to add: same public surface, same timeouts, no new options. JS has no counterpart — undici streams request bodies without buffering and the retry middleware is pyqwest-only.

Closes SDK-332.

Verification

  • tests/test_upload_transport.py (new, 12 tests): a 32 MiB streamed upload arrives whole with Content-Length framing and stays under 8 MiB of traced peak allocations (measured 1.5 MB, against 38 MB on the retrying transport); the upload transport wraps the very same pool object with no retry layer and is cached per proxy and read bound; the envd filesystem's three clients sit on the three transports; and a write of in-memory data still goes to the retrying client while a file-like one goes to the upload client, on both the octet-stream and multipart paths.
  • tests/test_volume_client.py: the volume upload transport is the SDK-wide upload transport, distinct from the volume client's default one.
  • Against prod: the full files/ suites, sync and async (123 tests, in-memory, streamed octet-stream and multipart writes), and template_sync/template_async test_build.py with force_upload=True, which puts a real build context through the presigned S3 PUT on the shared pool.
  • python-sdk unit suite, ruff lint/format and ty typecheck are green. The JS checks were not run — no JS or TS files are touched.

Notes for review

  • get_pyqwest_transport (the retrying stack, used by the envd RPC clients) is unchanged in behavior; it's now built on top of get_pool, which is the pool cache split out so the retry layer can be skipped without losing the connections.
  • Alternative considered: bounding the middleware's copy instead (keep it up to ~1 MiB, drop it past that), which would have preserved retries for small streamed bodies. It needed a hand-rolled retry loop, since pyqwest's execute_sync is @final — more machinery than the problem deserves.
  • Worth raising a max_buffered_body_size upstream in pyqwest's middleware anyway, so other consumers don't pay an unbounded copy.

🤖 Generated with Claude Code

Every persistent HTTP stack in the SDK now draws its connection pool from
`e2b.api.client_sync`/`client_async`, keyed on (proxy, idle read bound),
instead of caching four of its own: the control-plane REST API, the envd
HTTP API, the envd RPC clients, and the volume content API. reqwest pools
per host internally, so one pool serves the API host and every per-sandbox
host without interference — and because envd RPC and the envd HTTP API hit
the same host, an active sandbox needs a single HTTP/2 connection instead
of one per stack.

Two accessors expose it: `get_pyqwest_transport` hands connectrpc the
pool behind the connect-only retries, and `get_httpx_transport` hands the
generated httpx clients the `PyqwestTransport` adapter over that same
pool. Layers above stay per-consumer, as the design calls for:
`PlainHTTPErrorTransport` is now a stateless per-client wrapper rather
than a cached transport, so Connect-error normalization stays RPC-only.
Streamed downloads keep a pool of their own — the only one carrying the
idle `read_timeout`, since reqwest's read timer runs during body send and
TTFB and would otherwise cut off long uploads.

Sharing puts the sandbox health probe on the connection the failed RPC was
using, so `tests/test_shared_transport_pool.py` pins that at the frame
level with a new multi-connection HTTP/2 server serving both routes on one
pool: an RST_STREAM kills only the stream and the probe reuses the same
connection, while a dropped TCP connection makes reqwest redial. Both
paths still answer, so `handle_rpc_exception_with_health` keeps telling a
wedged connection apart from a dead sandbox.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@linear-code

linear-code Bot commented Aug 11, 2026

Copy link
Copy Markdown

SDK-332

@cla-bot cla-bot Bot added the cla-signed label Aug 11, 2026
@cursor

cursor Bot commented Aug 11, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches the shared HTTP transport stack and every large upload path; streamed uploads intentionally lose automatic connect retries (callers must retry), while envd RPC and in-memory writes keep the retrying client.

Overview
Fixes SDK-332 by stopping pyqwest’s connect-retry middleware from buffering entire streamed upload bodies in RAM (large files.write, volume.write_file, and template context uploads could peak near file size).

Transport layering is refactored in sync/async client_*: shared pools are created via get_pool(), connect retries wrap the pool for normal traffic, and a new get_upload_transport() / get_transport(..., for_upload=True) exposes an httpx adapter on the same pool without the retry layer. In-memory bodies still use the retrying stack because bytes replay without copying.

Call sites are wired to the upload path: sandbox filesystem gets _envd_api_upload and uses it only for streamed octet-stream and multipart writes; volume write_file uses get_upload_api_client; template upload_file drops per-build HTTPTransport in favor of the shared upload transport. Tests cover transport caching, client selection, routing, and tracemalloc peaks on ~32 MiB uploads.

Reviewed by Cursor Bugbot for commit 0d6033d. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Package Artifacts

Built from 93f6c21. Download artifacts from this workflow run.

JS SDK (e2b@2.38.4-fix-retry-body-buffering.0):

npm install ./e2b-2.38.4-fix-retry-body-buffering.0.tgz

CLI (@e2b/cli@2.16.2-fix-retry-body-buffering.0):

npm install ./e2b-cli-2.16.2-fix-retry-body-buffering.0.tgz

Python SDK (e2b==2.38.0+fix.retry.body.buffering):

pip install ./e2b-2.38.0+fix.retry.body.buffering-py3-none-any.whl

@mishushakov
mishushakov force-pushed the fix-retry-body-buffering branch from 8c57676 to 3754b57 Compare August 11, 2026 11:49
@changeset-bot

changeset-bot Bot commented Aug 11, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 0d6033d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@e2b/python-sdk Patch

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

@mishushakov mishushakov changed the title fix(python-sdk): stream request bodies instead of copying them fix(python-sdk): bound the memory the connect retries hold Aug 11, 2026
To be able to replay a request, pyqwest's retry middleware copies a streamed
request body in full as it is sent, so uploads were streamed to the wire *and*
held whole in RAM (64 MiB file -> 70 MB peak). The copy is what makes the
retries work — reqwest reads ahead into the body while connecting, so a
connect error leaves the iterator already started and unrewindable — so
uploads skip the layer rather than the layer skipping the copy.

`get_upload_transport` is the httpx adapter over the same pool without the
retries, and envd `files.write`, `volume.write_file` and template context
uploads take it whenever their body is streamed. Sharing the pool keeps the
connection reuse (and lets template uploads stop building a pool per build);
what a streamed upload gives up is the connect retry, which fires before any
of the body was written. Writes of in-memory data stay on the retrying
transport, where a `bytes` body is replayed without a copy.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mishushakov
mishushakov force-pushed the fix-retry-body-buffering branch from 3754b57 to 0d6033d Compare August 11, 2026 12:55
@mishushakov mishushakov changed the title fix(python-sdk): bound the memory the connect retries hold fix(python-sdk): keep streamed uploads off the retrying transport Aug 11, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 0d6033d. Configure here.

# An upload streams its body, and the connect retries would make that
# body replayable by copying it in full — the whole file in memory
# (SDK-332) — so it goes out on the non-retrying client instead.
api_client = get_upload_volume_api_client(config)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Volume writes drop in-memory retries

Medium Severity

write_file always uses get_upload_volume_api_client, including for str/bytes bodies that reach the transport as bytes and need no replay copy. Those calls lose connect retries, unlike files.write and contrary to the changeset note that in-memory writes keep them.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0d6033d. Configure here.

@mishushakov
mishushakov marked this pull request as draft August 11, 2026 15:30
@mishushakov
mishushakov force-pushed the python-sdk-unify-pyqwest-connection-pools-once-all-http-sdk-291 branch from ad9eb51 to 3141555 Compare August 13, 2026 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant