fix(python-sdk): raise SandboxException instead of bare Exception when command stream ends without exit event - #1727
Conversation
…n command stream ends without exit event
🦋 Changeset detectedLatest commit: 97b646b 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6aed678f7
ℹ️ 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.
TASTE.md review of the error-class change (checked T-1/T-1c parity, T-57/T-58 error hierarchy, T-62 actionable messages, T-69/T-71 docstrings).
The core of the change is right: a bare Exception here violated T-57 (sandbox failures extend SandboxException) and the fix matches the JS surface, which already throws SandboxError at the same spot. Keeping the base SandboxException rather than a specific subclass is also correct under T-58 here, since the cause is genuinely unknown at that point (transport failures are already mapped to TimeoutException upstream).
2 violations, both inline.
Not tied to a changed line: T-69/T-71 — wait()'s docstring in both files still documents only CommandExitException ("If the command exits with a non-zero exit code, it throws a CommandExitException"). T-62 requires documenting when an error is thrown, so the new SandboxException path belongs in the docstring, in reST field style (e.g. :raises SandboxException: the command stream ended before an exit event, e.g. the sandbox was killed or timed out). Same for the async variant.
|
/changeset |
|
This PR already has ---
"e2b": patch
+"@e2b/python-sdk": patch
---
-Raise `SandboxException` instead of bare `Exception` in `CommandHandle.wait()` when the command stream ends without an exit event, giving a clearer error message when the sandbox was killed, paused, or timed out mid-stream.
+Raise `SandboxException`/`SandboxError` instead of a bare `Exception` from `CommandHandle.wait()` when the command stream ends without an exit event, with a clearer message when the sandbox was killed, paused, or timed out mid-stream. |
|
PR #1727 already had a changeset, but it only bumped |
Summary
CommandHandle.wait()(sync) andAsyncCommandHandle.wait()(async) raised a bareException("Command ended without an end event")when the command stream closed without a ConnectRPC end event.Exceptioninstead of aSandboxException, breakingexcept SandboxExceptionhandlers and giving no indication that the sandbox lifecycle is the cause.Change
Replace the bare
ExceptionwithSandboxExceptionin bothsandbox_syncandsandbox_async:Context
This path is reached when the transport closes the stream before a ConnectRPC end envelope arrives — e.g. when a sandbox is killed or times out mid-command and the connection is dropped at the TCP level. The transport failure itself is already caught and mapped by
handle_rpc_exception_with_health(which raisesTimeoutExceptionwhen the sandbox health probe confirms it is gone). Thisresult is Nonebranch is the fallback for cases where the stream closed cleanly but no end event was received.Closes #1726 (Problem 3).