feat(js-sdk): support AbortSignal for request cancellation#1328
feat(js-sdk): support AbortSignal for request cancellation#1328mishushakov wants to merge 9 commits into
Conversation
Add an optional `signal: AbortSignal` to SDK request options across `Sandbox`, `Commands`, `Pty`, `Filesystem`, and `Volume` methods. The user signal is combined with the existing request-timeout signal via `AbortSignal.any()`, and is wired into the manual AbortControllers used by streaming RPCs (commands.run/connect, pty.create/connect, watchDir). Fixes #1312
🦋 Changeset detectedLatest commit: 09196c7 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 |
PR SummaryMedium Risk Overview It refactors streaming RPC startup to use a shared controller with a handshake timeout that aborts with a It changes paginator cancellation to be per-call ( Reviewed by Cursor Bugbot for commit 09196c7. Bugbot is set up for automated code reviews on this repo. Configure here. |
![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: ba388391b1
ℹ️ 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".
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
The CLI typechecks the js-sdk source via path mapping with \`lib: [es2022, dom, dom.iterable]\` and TypeScript 5.2, which does not expose \`AbortSignal.any\`. Replace it with a small \`combineAbortSignals\` helper so the type is portable across consumer tsconfigs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Package ArtifactsBuilt from 315fa55. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.20.2-mishushakov-js-sdk-abort-signal.0.tgzCLI ( npm install ./e2b-cli-2.10.2-mishushakov-js-sdk-abort-signal.0.tgzPython SDK ( pip install ./e2b-2.21.1+mishushakov.js.sdk.abort.signal-py3-none-any.whl |
This reverts commit 51ef43e.
CLI was pinned to TypeScript 5.2.2 whose dom lib does not declare \`AbortSignal.any\`, causing typecheck failures when the CLI imports the js-sdk source via path mapping. Align with the js-sdk's TS range. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Propagate signal through SandboxPaginator/SnapshotPaginator so
Sandbox.list({signal}) / listSnapshots({signal}) actually cancel.
- Extract setupRequestController() in connectionConfig to centralize
user-signal + timeout wiring with an idempotent cleanup() that
detaches the listener, clears the timer, and aborts the controller.
- Use it in Commands.start/connect, Pty.create/connect, and
Filesystem.watchDir, so the listener is always cleaned up — including
the stdin version-check error path (now thrown before setup) and the
initial startup catch.
- Have CommandHandle.handleEvents and WatchHandle.handleEvents call
handleDisconnect/handleStop in a finally block so the listener is
also released on natural stream completion (e.g. commands.run + wait)
or after kill().
- Add tests for paginator cancellation and setupRequestController.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7a4e700. Configure here.
The previous refactor kept the requestTimeoutMs timer running for the entire stream lifetime, which would prematurely abort any command, PTY session, or watchDir running longer than the request timeout (default 60s). The timer is only meant to bound the initial handshake — restore the old behaviour by splitting cleanup into clearStartTimeout (called once handleProcessStartEvent / handleWatchDirStartEvent resolves) and cleanup (called at stream end or on startup failure). The user-signal listener stays attached for the full stream lifetime so callers can still abort long-running streams. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Extract shared buildRequestSignal() from ConnectionConfig.getSignal
and VolumeConnectionConfig.getSignal so the two implementations
can't drift.
- setupRequestController now aborts with
DOMException('Request handshake timed out ...', 'TimeoutError') so
callers can distinguish handshake timeouts from user aborts (mirrors
AbortSignal.timeout() semantics).
- Document on SandboxListOpts.signal and SnapshotListOpts.signal that
the signal is stored on the paginator and applies to every
subsequent nextItems() call (construct a new paginator for a fresh
signal).
- Drop the defensive try/catch around handleDisconnect/handleStop in
CommandHandle and WatchHandle — both wrap an idempotent cleanup, so
the silent catch hides nothing today and would mask real bugs if
those methods ever grow side effects.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace the constructor-time signal on SandboxPaginator/SnapshotPaginator
with a per-call AbortSignal accepted by nextItems({ signal }). Storing
the signal on the paginator was footgunny: one abort would poison every
subsequent page. Per-call signals make cancellation explicit and let the
caller mix cancellable and non-cancellable pages on the same paginator.
Also Omit 'signal' from SandboxListOpts/SnapshotListOpts since the
paginator constructor performs no I/O and storing it had no effect.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…flake The tests previously used `setTimeout(() => controller.abort(), 25)` to fire the abort after the request had (presumably) started. On Windows CI that 25ms guess sometimes landed before MSW invoked the handler, so the handler attached its `abort` listener to an already-aborted signal and hung until the 30s vitest timeout. Wait on MSW's `request:start` event instead, and keep the handler race-safe by checking `signal.aborted` before subscribing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 participant
Summary
signal: AbortSignalto SDK request options acrossSandbox,Commands,Pty,Filesystem, andVolumemethods.ConnectionConfig.getSignal()(andVolumeConnectionConfig.getSignal()) delegate to a sharedbuildRequestSignal()helper that combines the user signal with the existing request-timeout signal viaAbortSignal.any().commands.run/connect,pty.create/connect,watchDir) route through a sharedsetupRequestController()that bridges the user signal into the internalAbortControllerand exposesclearStartTimeout+ idempotentcleanup. Handshake timer is cleared after start succeeds (so long-running streams aren't aborted atrequestTimeoutMs), andCommandHandle/WatchHandlecall cleanup in afinallyso the user-signal listener is always detached.DOMException('Request handshake timed out …', 'TimeoutError')so callers can distinguish from user aborts.SandboxPaginator.nextItems/SnapshotPaginator.nextItemsaccept a per-callsignal(nextItems({ signal })); the constructor opts no longer takesignalsince paginator construction is sync and the per-call API is the natural fit.^5.4.5so its dom lib includesAbortSignal.any.Fixes #1312
Usage:
Test plan
pnpm run typecheck(js-sdk + cli)pnpm run lintpnpm run formatpnpm exec vitest run tests/sandbox/abortSignal.test.ts tests/connectionConfig.test.ts(19/19 pass — coversSandbox.create/kill, paginator per-call signal,getSignal,setupRequestControllerlifecycle,TimeoutErrorreason)🤖 Generated with Claude Code