fix(js-sdk): pass connection config to pause() and betaPause()#1230
fix(js-sdk): pass connection config to pause() and betaPause()#1230goingforstudying-ctrl wants to merge 2 commits intoe2b-dev:mainfrom
Conversation
The pause() and betaPause() methods were not using the connection config passed to Sandbox.connect(), unlike kill() which correctly uses it. This caused authentication failures when calling pause() after connecting to a sandbox with explicit credentials. Fixes e2b-dev#1215
🦋 Changeset detectedLatest commit: bd90328 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: 65245eefd2
ℹ️ 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".
| */ | ||
| async pause(opts?: ConnectionOpts): Promise<boolean> { | ||
| return await SandboxApi.pause(this.sandboxId, opts) | ||
| return await SandboxApi.pause(this.sandboxId, { ...this.connectionConfig, ...opts }) |
There was a problem hiding this comment.
Avoid freezing pause() to the original apiUrl
Because ConnectionConfig stores a resolved apiUrl, the spread here also forwards that derived value into SandboxApi.pause(). In packages/js-sdk/src/connectionConfig.ts, the constructor prefers opts.apiUrl over recomputing from domain or debug, so calls like sandbox.pause({ domain: 'staging.e2b.app' }) or sandbox.pause({ debug: true }) will still hit the old control-plane URL after this change unless the caller also overrides apiUrl. Before this patch, pause(opts) still honored those per-call ConnectionOpts, so this regresses non-default domain/debug setups; the same issue is repeated in betaPause() below.
Useful? React with 👍 / 👎.
|
Looks like the upstream This PR is effectively redundant since the fix has been addressed in the codebase. Feel free to close this if you agree. Thanks! |
Description
Fixes #1215
The
pause()andbetaPause()methods were not using the connection config passed toSandbox.connect(), unlikekill()which correctly uses it.This caused authentication failures when calling
pause()after connecting to a sandbox with explicit credentials (e.g., viaapiKeyoption).Changes
pause()to pass{ ...this.connectionConfig, ...opts }toSandboxApi.pause()betaPause()to pass{ ...this.connectionConfig, ...opts }toSandboxApi.betaPause()This aligns the behavior with
kill(), which already correctly uses the connection config.Before
After