fix: disable stream timeout for background commands to allow reconnect by PID#1229
Conversation
…t by PID When running background commands, the default 60s stream timeout caused the RPC connection to terminate with DeadlineExceeded. This made envd clean up process tracking, so subsequent connect(pid) calls would fail. For background commands where timeout is not explicitly set, override to 0 (no timeout) so the process remains reachable for reconnection. Fixes e2b-dev#1074 Signed-off-by: majiayu000 <1835304752@qq.com>
…default Replace `timeout == 60` check with `timeout: Optional[float] = None` sentinel to correctly distinguish 'user omitted timeout' from 'user explicitly set timeout=60'. This preserves the user's explicit timeout choice for background commands while defaulting to unlimited (0) when no timeout is specified. Signed-off-by: majiayu000 <1835304752@qq.com>
|
majiayu000
requested review from
ValentaTomas,
jakubno and
mishushakov
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bca54bd625
ℹ️ 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".
| if timeout is None: | ||
| effective_timeout = 0 if background else 60 |
There was a problem hiding this comment.
timeout=None for Python foreground runs
This branch now rewrites every timeout=None call to 60 when background is false, so commands.run(..., timeout=None) no longer disables the command-stream deadline. That is a public API regression, and it also changes existing internal callers such as packages/python-sdk/e2b/sandbox_sync/git.py:_run_git and packages/python-sdk/e2b/sandbox_async/git.py:_run_git, which default timeout to None and forward it directly. Slow git clone/fetch/push operations that previously had no command timeout will now start failing with TimeoutException after 60 seconds. If the goal is only to special-case omitted timeouts for background commands, this needs a separate sentinel instead of treating explicit None as “use default”.
Useful? React with 👍 / 👎.
1 participant
Fixes #1074
Summary
Commands.run()passes a default 60-second timeout to the internalstart()RPC call. This timeout sets a Connect-RPC deadline on the server-streaming call. For background commands, when the deadline expires the stream terminates withDeadlineExceeded, which signals envd to clean up its process tracking — making the PID unreachable for subsequentconnect(pid)calls.This fix changes the default timeout for background commands to
0(unlimited) while preserving the 60-second default for foreground commands:backgroundis true andtimeoutMsis not explicitly provided, override to0before callingstart().timeoutparameter default from60toNone(sentinel), then resolve to0for background commands or60for foreground commands at runtime.Users who explicitly pass a timeout value still get that exact timeout regardless of background mode.
Test plan
pnpm run typecheck,pnpm run lint, andpnpm run formatall passconnect(pid)succeeds