Skip to content

fix(sdk): stop command event iterator at terminal end event#1392

Open
mishushakov wants to merge 4 commits into
mainfrom
mishushakov/command-iterator-stop-on-end
Open

fix(sdk): stop command event iterator at terminal end event#1392
mishushakov wants to merge 4 commits into
mainfrom
mishushakov/command-iterator-stop-on-end

Conversation

@mishushakov
Copy link
Copy Markdown
Member

@mishushakov mishushakov commented Jun 5, 2026

Problem

A background command handle reads the command's output stream in a loop. envd sends a terminal end event (carrying the exit code) and is then expected to close the HTTP stream. The iterator recorded the result on the end event but kept awaiting the next stream event, so CommandHandle.wait() only unblocked when envd actually closed the stream — and if that close was delayed, wait() hung past the point the result was already known. Separately, the underlying stream held its HTTP/TCP connection open until GC, which can exhaust the connection pool (max_connections=2000) in workloads creating many handles. This is the wait()-blocking / connection-release facet of #1034.

Changes

  • Stop processing the stream as soon as the terminal end event is received, regardless of stream-close timing.
  • Release the HTTP stream/connection deterministically in all paths (normal completion, exception, and disconnect()):
    • Python async/sync: the generator supports aclose()/close(), so _handle_events closes it in a finally. disconnect() (async) now cancels the task and lets that finally close the stream, avoiding the concurrent-access RuntimeError that previously disabled aclose() there.
    • JS: the connect-es response iterator deliberately omits return()/throw(), and the transport only clears its per-call deadline timer + connection on a settled read — never on an abandoned iterator. So instead of returning early, we cancel the request on the end event; the next read then settles immediately and the transport runs its cleanup. The resulting cancellation error is ignored once the result is known. (Returning early leaked a timer, which Deno's leak detector flagged as a CI failure.)
  • Added unit tests for all three SDKs, each verified to fail on the unpatched code; plus a changeset for e2b and @e2b/python-sdk.

Usage

No API change — existing code benefits automatically:

handle = await sandbox.commands.run("echo hi", background=True)
result = await handle.wait()  # returns at the end event, not at stream close

kill() semantics are unchanged; the child-process-survival facet of #1034 is handled separately.

🤖 Generated with Claude Code

Terminate the background command event iterator as soon as the terminal
`end` event is received instead of waiting for envd to close the HTTP
stream, so `CommandHandle.wait()` returns promptly once the result is
known. Release the underlying HTTP stream/connection deterministically in
all paths (normal completion, exception, disconnect) to avoid exhausting
the connection pool. Applied to JS, Python async, and Python sync SDKs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@cursor
Copy link
Copy Markdown

cursor Bot commented Jun 5, 2026

PR Summary

Medium Risk
Changes core command streaming and connection lifecycle in both SDKs; wrong cleanup could still leak connections or surface spurious errors, but behavior is covered by new unit tests and is a targeted bugfix.

Overview
Background command wait() could stay blocked after the terminal end event when envd delayed closing the HTTP stream, and command streams could keep connections open until GC and exhaust the pool.

JS CommandHandle cancels the request on end, ignores cancellation errors once the result is set, and still runs disconnect cleanup in finally. Python async and sync handles stop iterating at end, close the event stream in finally, and async disconnect() avoids closing the stream while the task is still iterating.

Unit tests were added for JS and Python async/sync; changeset bumps e2b and @e2b/python-sdk.

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

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jun 5, 2026

🦋 Changeset detected

Latest commit: 869f048

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

This PR includes changesets to release 2 packages
Name Type
e2b Patch
@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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

Package Artifacts

Built from 19ef195. Download artifacts from this workflow run.

JS SDK (e2b@2.27.2-mishushakov-command-iterator-stop-on-end.0):

npm install ./e2b-2.27.2-mishushakov-command-iterator-stop-on-end.0.tgz

CLI (@e2b/cli@2.10.4-mishushakov-command-iterator-stop-on-end.0):

npm install ./e2b-cli-2.10.4-mishushakov-command-iterator-stop-on-end.0.tgz

Python SDK (e2b==2.25.1+mishushakov-command-iterator-stop-on-end):

pip install ./e2b-2.25.1+mishushakov.command.iterator.stop.on.end-py3-none-any.whl

mishushakov and others added 3 commits June 5, 2026 21:04
The connect-es response iterator omits return()/throw(), so early-returning
from it on the end event left the transport's per-call deadline timer (and
the connection) dangling until GC — which Deno's leak detector flags as a
test failure. Instead, cancel the request on the end event so the next read
settles immediately and the transport runs its cleanup; the resulting
cancellation error is ignored once the result is known. Updated the unit
test to faithfully model the connect-es iterator (no return(), settles only
on cancel) so it actually exercises the leak.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Keep one "stops at end event" case per SDK; drop the disconnect/exception
variants that mostly restated the SDK-side control flow. Real envd behavior
is covered by the live command tests and the Deno leak test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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