feat(js): retry rate-limited control-plane requests - #1848
Conversation
🦋 Changeset detectedLatest commit: acb9c72 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 aa13215. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.49.1-feat-retry-after-rate-limits.0.tgzCLI ( npm install ./e2b-cli-2.19.1-feat-retry-after-rate-limits.0.tgzCode Interpreter JS SDK ( npm install ./e2b-code-interpreter-2.8.1-feat-retry-after-rate-limits.0.tgzDesktop JS SDK ( npm install ./e2b-desktop-2.4.1-feat-retry-after-rate-limits.0.tgzPython SDK ( pip install ./e2b-2.49.0+feat.retry.after.rate.limits-py3-none-any.whlCode Interpreter Python SDK ( pip install ./e2b_code_interpreter-2.10.0+feat.retry.after.rate.limits-py3-none-any.whlDesktop Python SDK ( pip install ./e2b_desktop-2.5.0+feat.retry.after.rate.limits-py3-none-any.whl |
There was a problem hiding this comment.
TASTE.md review of the retries option (JS ConnectionOpts / VolumeApiOpts, Python ApiParams / VolumeApiParams, retry.ts, _retry.py, per-call plumbing in filesystem / isRunning / volume).
Checked: T-1/T-2/T-10 cross-SDK parity, T-3/T-3a positional vs. keyword options, T-6 static vs. instance option surfaces, T-12 naming, T-46 signal threading, T-47 named defaults, T-49 config precedence, T-51 immutability, T-57/T-59/T-62/T-64 error classes and messages, T-69/T-70/T-71 docs.
5 violations, 7 inline comments (T-59 and T-1 each surface at two sites):
- T-59 — argument validation for
retriesthrows bareError(JS) /ValueError(Python) instead ofInvalidArgumentError/InvalidArgumentException. - T-1 / T-10 — JS grows a per-call
retriesonFilesystemRequestOptsandsandbox.isRunning(opts); the sync/async Pythonfiles.*andis_running()methods only takerequest_timeout, so the per-call knob has no Python mirror. - T-3 —
Volumeconstructors (JS + both Python variants) extend the chain of optional positionals with one more (retries). Follows the existing pattern, but the rule says never extend that chain — a keyword-only / options-object slot is the compliant form for a new parameter. - T-64 — the retry transport's
httpx.TimeoutException("Request timeout exhausted while retrying")does not name the knob (request_timeout/retries). - T-47 —
EnvdApiClientdefaultsretrieswith a literal0at the call site rather than a named constant shared withresolveRetries.
Not tied to a line: the Retry-After parser accepts only delta-seconds and silently ignores HTTP-date values (Retry-After allows both per RFC 9110); this is a behavior choice rather than a TASTE rule, but the retries docstrings should say so since T-69 asks for failure modes to be documented. Also worth deciding under T-49 whether retries should get an E2B_RETRIES env var — it reads as a per-caller knob rather than deployment-level, so the current omission is defensible.
|
Scope clarification: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 211b42a03a
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 211b42a03a
ℹ️ 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".
|
Review follow-up: all seven inline threads are addressed and resolved. The public option docs now state that only valid non-negative integer delta-seconds |
| def _copy_request( | ||
| request: httpx.Request, remaining_timeout: Optional[float] = None | ||
| ) -> httpx.Request: | ||
| extensions = dict(request.extensions) | ||
| timeout = extensions.get("timeout") | ||
| if remaining_timeout is not None and isinstance(timeout, dict): | ||
| adjusted_timeout = { | ||
| key: remaining_timeout if value is None else min(value, remaining_timeout) | ||
| for key, value in timeout.items() | ||
| } | ||
| extensions["timeout"] = adjusted_timeout | ||
|
|
||
| return httpx.Request( | ||
| request.method, | ||
| request.url, | ||
| headers=request.headers, | ||
| content=request.content, | ||
| extensions=extensions, | ||
| ) |
There was a problem hiding this comment.
this could be potentially dangerous on larger request bodies, I'd suggest probably to make any request that body exceeds n to be non-retryable instead and throw early
There was a problem hiding this comment.
What do you mean by dangerous? I'm not sure the content is actually copied over if you mean the memory usage.
There was a problem hiding this comment.
yes I meant if it's a large request like upload that is being retried (1GB)
| monotonic() + (requestTimeoutMs || MAX_RETRY_WAIT_WITHOUT_TIMEOUT_MS) | ||
|
|
||
| for (let attempt = 0; ; attempt++) { | ||
| const response = await fetchImpl(request.clone()) |
There was a problem hiding this comment.
same large request body point as in the Python version
|
and also could we rename RateLimitTransport to RetryableTransport or something similar that makes it clear it can be retried? |
|
@mishushakov I've renamed |
Mirror the Python transport's ByteStream check: mark whether the body is a stream at RetryableRequest construction using isReadableStreamLike and send streaming bodies once, replaying everything else with clone() instead of capturing serialized bodies in a WeakMap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The retry wrapper is only installed on the control-plane client, where openapi-fetch serializes every body to a string before constructing the Request, so Request-form inputs are always safe to clone. The streamability gate on init bodies stays for direct callers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
0970e82
into
feat/python-retry-after-rate-limits
## Stack 1. **This PR: Python SDK**, based on `main`. 2. **JavaScript SDK: #1848**, stacked on this branch. Merge Python first, then retarget the JS PR to `main`. Split from #1848; this PR contains the Python implementation and its package-specific changeset. ## Summary - Add configurable `Retry-After` handling to both sync and async Python control-plane API clients. - Default to 3 retries; set `retries=0` to disable them. - Retry only `429` responses with a valid non-negative integer delta-seconds `Retry-After` header. - Respect the original timeout budget, with a 60-second aggregate retry-wait bound when request timeouts are disabled. - Close intermediate responses and reuse immutable buffered body bytes; streaming bodies are sent once without buffering for replay. - Preserve existing rate-limit errors when retries are disabled, invalid, or exhausted. ## Scope Retries apply to E2B control-plane REST calls, including volume management. Direct sandbox/envd traffic (filesystem and commands) and volume-content traffic are outside this retry layer. Existing connect-only transport retries remain unchanged. ## Usage ```python from e2b import Sandbox, AsyncSandbox # Sync: control-plane calls retry up to three times by default. sandbox = Sandbox.create() sandboxes = Sandbox.list(retries=0).next_items() # Async equivalent (inside an async function). sandbox = await AsyncSandbox.create() sandboxes = await AsyncSandbox.list(retries=0).next_items() ``` ## Validation - Repository-wide `pnpm run format`, `pnpm run lint`, and `pnpm run typecheck` passed. - 131 focused Python tests passed on this Python-only branch. - `git diff --check` passed. - Full production/staging integration validation runs in CI. From `packages/python-sdk`: ```sh uv run pytest tests/test_rate_limit_retry_transport.py tests/test_api_client_transport.py tests/test_connection_config.py tests/test_volume_client.py ``` <details> <summary>Python sync/async memory verification</summary> Run from `packages/python-sdk`. No network requests or API credentials are needed. The 32 MiB payload is allocated before tracing; three distinct requests must reference the exact same bytes object. The assertion bounds additional traced Python allocations, not native transport buffers or whole-process RSS. ```sh uv run python <<'PY' import asyncio import gc import tracemalloc import httpx from e2b.retry import RetryableTransport, AsyncRetryableTransport payload = b"x" * (32 * 1024 * 1024) async def check(async_mode): requests = [] def handle(request): requests.append(request) assert request.content is payload return httpx.Response( 429 if len(requests) < 3 else 200, headers={"Retry-After": "0"}, ) request = httpx.Request("POST", "https://api.test", content=payload) inner = httpx.MockTransport(handle) gc.collect() tracemalloc.start() try: if async_mode: response = await AsyncRetryableTransport(inner, retries=2).handle_async_request(request) else: response = RetryableTransport(inner, retries=2).handle_request(request) _, peak = tracemalloc.get_traced_memory() finally: tracemalloc.stop() assert response.status_code == 200 and len(requests) == 3 assert len({id(r) for r in requests}) == 3 assert all(r.content is payload for r in requests) assert peak < 4 * 1024 * 1024, peak print(f"{'async' if async_mode else 'sync'}: {len(requests)} attempts share the original bytes; peak extra allocation {peak / 1024:.1f} KiB") async def main(): await check(False) await check(True) asyncio.run(main()) PY ``` Expected: both paths share the original payload across three attempts, with peak additional allocations below 4 MiB. Previously measured locally: about 14.5 KiB sync and 13.0 KiB async. </details> --------- Co-authored-by: Mish Ushakov <10400064+mishushakov@users.noreply.github.com> Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Stack
main.feat/python-retry-after-rate-limits.Merge #1862 first, then retarget this PR to
main. This PR's diff contains only the JS implementation, tests, and JS changeset.Summary
Retry-Afterhandling to the JavaScript SDK control-plane clientretries: 0to disable them429responses carrying a valid non-negative delta-secondsRetry-AfterheaderisReadableStreamLike) are sent exactly once, everything else is replayed withRequest.clone()— safe because openapi-fetch serializes every control-plane body to a string before constructing the Request, and all streaming uploads (envd filesystem, volume content, template files) use separate, unwrapped clientsScope
The retry layer is installed only on E2B control-plane REST clients. Direct sandbox/envd traffic, including filesystem and command operations, is not retried. Volume-content traffic is also not retried; volume management calls made through the E2B control-plane API follow the control-plane policy. Existing connect-only transport retries remain unchanged.
Usage
JavaScript
Validation
pnpm run formatpnpm run lintpnpm run typechecktests/retry.test.ts,tests/client.test.ts,tests/connectionConfig.test.ts,tests/volume/file.test.ts,tests/sandbox/configPropagation.test.ts)git diff --checkpnpm run testwas also attempted. The recursive run reached live integration suites but could not complete in this local environment because credentials and external sandbox fixtures were unavailable.