Skip to content

fix(sdk): fail wait() after disconnect() with a SandboxError, not a TimeoutError - #1972

Open
TanishGudise wants to merge 2 commits into
e2b-dev:mainfrom
TanishGudise:fix/command-handle-disconnect-wait
Open

TanishGudise wants to merge 2 commits into
e2b-dev:mainfrom
TanishGudise:fix/command-handle-disconnect-wait

Conversation

@TanishGudise

@TanishGudise TanishGudise commented Oct 10, 2026 •

Copy link
Copy Markdown

Summary

CommandHandle.disconnect() aborts the command's output stream. In the JS SDK, a wait() that is pending at that point, or called afterwards, reports the abort as a request timeout:

TimeoutError: [canceled] This operation was aborted: This error is likely due to exceeding 'requestTimeoutMs'. You can pass the request timeout value as an option when making the request.

This is the CommandHandle / PTY twin of the WatchHandle.stop() bug fixed in #1923 (#1895), and the fix copies that pattern: the handle already records the disconnect, and a stream error that arrives after it is now a clean end. wait() then rejects with a SandboxError saying the handle was disconnected before the command finished. Before, the error could also depend on timing: if an output event was already in flight when disconnect() was called, the disconnected check in handleEvents broke out of the loop and wait() rejected with SandboxError('Process exited without a result') instead. Both cases now get the same message.

The Python SDK had the same gap with different symptoms, so per the repo's parity rule it gets the matching change:

  • sync: wait() raised a bare Exception("Command ended without an end event"), which except SandboxException does not catch.
  • async: wait() raised asyncio.CancelledError into a task nobody cancelled. disconnect() cancels the handle's internal event task, and wait() re-raised that cancellation. except Exception does not catch it.

Both now raise SandboxException with the same message as JS.

Scope:

  • Only stream errors are suppressed after a disconnect. Callback errors (onStdout / on_stdout and friends) are still reported, as in fix(js-sdk): treat WatchHandle.stop() as a clean watch end #1923.
  • Bytes still buffered in the UTF-8 decoders at the disconnect: async Python and JS still add them to stdout/stderr. Async Python no longer passes them to the callbacks after the disconnect (JS already didn't); sync Python is unchanged here.
  • Sync Python marks the handle disconnected only once closing the stream succeeded. A disconnect() that fails (for example from another thread during a read, which raises ValueError: generator already executing, as on main) leaves the handle as it was.
  • A wait() on a command that finished before the disconnect still returns its result.
  • Handles that were never disconnected behave as before, including the bare Exception when a stream ends without an end event (fix(python-sdk): raise SandboxException instead of bare Exception when command stream ends without exit event #1727 addresses that separately).
  • Async Python: cancelling the task that awaits wait() still cancels it. On Python 3.11+ that also holds when the cancel and disconnect() land in the same event-loop tick (checked with Task.cancelling(), counting only cancellations requested during the wait() call, as asyncio.timeout() does). Python 3.10 has no Task.cancelling(), so in that same-tick case the waiter gets the SandboxException instead, and CI (3.10) skips the test for it. If you'd rather close that gap on every version, wait() could wait with asyncio.wait([self._wait]), which never passes the caller's cancellation into the event task, so a cancelled caller always sees its own CancelledError and the cancelling() checks go away. The cost is that cancelling wait() would no longer stop the handle's event task, which changes behavior, so I left that to you.

Usage

const handle = await sandbox.commands.run('sleep 300', { background: true })
const done = handle.wait()

await handle.disconnect()

try {
  await done
} catch (err) {
  // before: TimeoutError blaming requestTimeoutMs
  // now:    SandboxError: Disconnected before the command finished; reconnect
  //         with `commands.connect()` or `pty.connect()` to wait for its result.
}

const reconnected = await sandbox.commands.connect(handle.pid)
handle = await sandbox.commands.run("sleep 300", background=True)
waiter = asyncio.create_task(handle.wait())

await handle.disconnect()

try:
    await waiter
except SandboxException as e:  # before: CancelledError (async) / bare Exception (sync)
    print(e)

reconnected = await sandbox.commands.connect(handle.pid)

Tests

  • packages/js-sdk/tests/sandbox/commands/commandHandle.test.ts: a pending wait() (stdout and PTY handles) and a later wait() reject with the SandboxError; a stream error on a handle that was not disconnected still maps to TimeoutError; a callback error raised after disconnect() is still reported; decoder bytes buffered at the disconnect end up in stdout but not in the callback. The stream fake fails the pending read with the Canceled ConnectError that connect-es raises for an aborted request.
  • packages/python-sdk/tests/test_command_handle.py, sync and async, mostly on the repo's HTTP/2 envd frame server (envd_frame_server) with the real connectrpc client: disconnect while idle, async disconnect while a callback is running, sync disconnect from inside a callback, two guards that cancelling the waiter still cancels it (one is 3.11+ only), a cancellation the calling task handled before wait() is not taken as a new one (3.11+, both disconnect paths), a sync disconnect() that fails leaves the handle connected, and async decoder bytes buffered at the disconnect end up in stdout but not in the callback.
  • The tests that cover the change fail on main (3 JS, 7 Python) and pass with the fix; the guard tests pass on both. 20 repeated runs were clean.
  • Against a real envd, built from e2b-dev/runtime at the commit pinned in spec/runtime-ref and run in Docker with -isnotfc: on main, 20/20 command and 20/20 PTY wait()s after disconnect() fail with TimeoutError in JS, CancelledError in async Python and a bare Exception in sync Python. With the fix, all of them raise the new SandboxError / SandboxException. The command and PTY test folders in E2B_DEBUG mode against that envd pass. JS (tests/sandbox/commands, tests/sandbox/pty): 45 passed / 2 skipped, against main's 39 plus the 6 new tests. Python (tests/{sync,async}/sandbox_*/{commands,pty}): 52 passed / 5 skipped on both.
  • Full JS and Python suites: same failure set as main. Without an E2B_API_KEY (I don't have one), the live tests fail, and so do three JS Secret/Volume tests that also fail on main. Python tests also ran on 3.10 (CI's version) and 3.14. pnpm run lint, pnpm run format (no diff) and pnpm run typecheck pass.

Includes an e2b / @e2b/python-sdk patch changeset.

🤖 Generated with Claude Code

…imeoutError

disconnect() aborts the command/PTY output stream. In JS the abort reached
wait() as a TimeoutError blaming requestTimeoutMs; async Python raised
CancelledError into a task nobody cancelled, and sync Python a bare
Exception. A stream error after disconnect() is now a clean end, as for
WatchHandle.stop() (e2b-dev#1923), and wait() reports the disconnect with a
SandboxError / SandboxException.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 08f820f

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

This PR includes changesets to release 3 packages
Name Type
e2b Patch
@e2b/python-sdk Patch
@e2b/code-interpreter-python 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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-10T19:46:47.629322Z 2ed1d3b PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2ed1d3b841

ℹ️ 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".

Comment on lines +273 to +274
if not self._disconnected or _current_task_cancelling():
raise

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Distinguish prior cancellations from this wait

On Python 3.11+, a task's cancelling() count remains nonzero after it has caught a prior CancelledError unless it explicitly calls uncancel(). If that task later calls wait() after disconnect(), the CancelledError from the handle's internally canceled _wait takes this branch and escapes as though the caller canceled the current wait, instead of producing the promised SandboxException like JS and sync Python; compare against a count captured on entry to wait() (or otherwise distinguish a new cancellation) rather than treating any historical count as pending.

AGENTS.md reference: AGENTS.md:L3-L3

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Confirmed: a task that had handled an earlier cancel kept cancelling() > 0, so wait() after disconnect() raised CancelledError instead of SandboxException. Fixed in 08f820f: wait() records cancelling() on entry and only treats an increase as the caller being cancelled (as asyncio.timeout() does), with a regression test for both disconnect paths.

A task that handled an earlier cancellation without uncancel() keeps a
nonzero Task.cancelling(), so wait() after disconnect() raised
CancelledError instead of SandboxException. Compare against the count
captured on entry, as asyncio.timeout() does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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