fix(sdk): disable timeout on background commands by default#1391
fix(sdk): disable timeout on background commands by default#1391mishushakov wants to merge 5 commits into
Conversation
🦋 Changeset detectedLatest commit: 12850f4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
PR SummaryMedium Risk Overview In JS, background starts pass Reviewed by Cursor Bugbot for commit 12850f4. Bugbot is set up for automated code reviews on this repo. Configure here. |
Package ArtifactsBuilt from 7a0477a. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.27.2-mishushakov-disable-background-cmd-timeout.0.tgzCLI ( npm install ./e2b-cli-2.10.4-mishushakov-disable-background-cmd-timeout.0.tgzPython SDK ( pip install ./e2b-2.25.1+mishushakov.disable.background.cmd.timeout-py3-none-any.whl |
005ac68 to
19deb22
Compare
Background commands (`commands.run(..., background=True)` / `{ background: true }`)
were killed after the default 60s command timeout. They now default to no
timeout, while foreground commands keep the 60s default. An explicit
`timeout`/`timeoutMs` still takes precedence.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
19deb22 to
d79fa59
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6647fb6cab
ℹ️ 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".
…x connect timeoutMs JSDoc Addresses PR review: - background no-timeout integration tests now sleep 70s (> the previous 60s default) so they actually guard the regression rather than passing under both old and new behavior. - `CommandConnectOpts` gets its own `timeoutMs` JSDoc so `connect()` tooltips no longer inherit the background-specific wording from `CommandStartOpts`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Distinguish an omitted `timeout` from an explicit `None` via a private sentinel: omitted resolves to the default (60s foreground, 0/no-timeout background), while an explicit `None` means no timeout (0). This restores the pre-change behavior where `run(..., timeout=None)` ran uncapped, addressing the PR review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ng the type Revert the `CommandConnectOpts` type split and the background note on the shared `CommandStartOpts.timeoutMs` field. The background no-timeout behavior is documented on the `run` background overload only, so `connect()`'s `timeoutMs` tooltip stays generic without any type churn. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Background commands (
commands.run(..., background=True)in Python /{ background: true }in JS) were killed once the default 60s command timeout elapsed. They now default to no timeout, while foreground commands keep the 60s default, and an explicittimeout/timeoutMsstill takes precedence.The "disable" value differs per transport: Python's
e2b_connecttreats0as no-limit, while@connectrpc/connecttreatstimeoutMs <= 0as an immediate abort and onlyundefinedas no-limit — so the JS side resolves toundefinedand the Python side to0. Both SDKs use their natural "not set" marker (undefined/None) to decide the default, and the changeset bumpse2band@e2b/python-sdkas patches.Usage
Tests
Integration tests (sync + async Python, JS) against live sandboxes assert that a background
sleep 20is capped when giventimeout=10(raisesTimeoutExceptionin under 20s) and completes successfully when no timeout is set.🤖 Generated with Claude Code