Turn on automatic retries for sandbox requests #1022
Conversation
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
| const maxRetries = 3 | ||
| for (let retry = 0; ; retry++) { | ||
| try { | ||
| await sandbox.pty.sendInput(terminalSession.pid, combined) | ||
| break |
There was a problem hiding this comment.
Avoid retrying PTY input without idempotency
Retrying sendInput can duplicate keystrokes/commands when a transient error is raised after the server already applied the input (e.g., timeout/connection reset after the PTY received data). Because the retry resends the same combined buffer without any sequence/dedup mechanism, users can see repeated characters or repeated command execution under exactly those network conditions. If sendInput is not explicitly idempotent, this introduces at-least-once semantics for terminal input.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Interesting point. @ValentaTomas wdyt about adding a "request number", idempotency key, or something similar so that the backend avoids duplicate processing?
There was a problem hiding this comment.
Might make sense here! Alternatively, the client long running connection that we don't have implemented in the SDK might be also a solution.
|
|
||
| function isRetryableError(err: unknown): boolean { | ||
| // Retry on SDK TimeoutError | ||
| if (err instanceof (e2b as any).TimeoutError) return true |
There was a problem hiding this comment.
(e2b as any) unnecessary - just import TimeoutError from e2b
| if (err instanceof (e2b as any).TimeoutError) return true | ||
|
|
||
| // Some environments throw AbortError for aborted/timeout fetches | ||
| if (err && typeof err === 'object' && (err as any).name === 'AbortError') |
There was a problem hiding this comment.
you can infer the type as err as Error (in below occurrences also)
| ]) | ||
| if (typeof code === 'string' && retryableCodes.has(code)) return true | ||
|
|
||
| // Undici/Fetch may surface as TypeError: fetch failed with nested cause |
| await sandbox.pty.sendInput(terminalSession.pid, combined) | ||
|
|
||
| const maxRetries = 3 | ||
| for (let retry = 0; ; retry++) { |
Note
Implements automatic retries for terminal input delivery to the sandbox PTY to improve resilience against transient failures.
isRetryableErrorto detect transient conditions (SDKTimeoutError,AbortError, common network error codes, and UndiciTypeError: fetch failed)sandbox.pty.sendInputwith up to 3 retries, skipping retries for non-retryable errorsWritten by Cursor Bugbot for commit 503bd4b. This will update automatically on new commits. Configure here.