Skip to content

fix(sdk): clean up sandbox when MCP gateway startup fails #1548

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mishushakov wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(sdk): clean up sandbox when MCP gateway startup fails #1548

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16390cf
fix(sdk): clean up sandbox when MCP gateway startup fails (#1547)
hxaxd Jul 13, 2026
89c902b
test(sdk): force a real MCP gateway startup failure in cleanup tests
mishushakov Jul 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/quiet-sandboxes-clean.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"e2b": patch
"@e2b/python-sdk": patch
---

Kill newly created sandboxes when MCP gateway startup fails.
25 changes: 15 additions & 10 deletions packages/js-sdk/src/sandbox/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,17 +320,22 @@ export class Sandbox extends SandboxApi {

if (sandboxOpts?.mcp) {
sandbox.mcpToken = crypto.randomUUID()
const res = await sandbox.commands.run(
`mcp-gateway --config ${shellQuote(JSON.stringify(sandboxOpts.mcp))}`,
{
user: 'root',
envs: {
GATEWAY_ACCESS_TOKEN: sandbox.mcpToken ?? '',
},
try {
const res = await sandbox.commands.run(
`mcp-gateway --config ${shellQuote(JSON.stringify(sandboxOpts.mcp))}`,
{
user: 'root',
envs: {
GATEWAY_ACCESS_TOKEN: sandbox.mcpToken ?? '',
},
}
)
if (res.exitCode !== 0) {
throw new Error(`Failed to start MCP gateway: ${res.stderr}`)
}
)
if (res.exitCode !== 0) {
throw new Error(`Failed to start MCP gateway: ${res.stderr}`)
} catch (error) {
await sandbox.kill().catch(() => undefined)
throw error
}
}
Comment on lines 320 to 340

@claude claude Bot Jul 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The manual if (res.exitCode !== 0) throw new Error('Failed to start MCP gateway: ...') check is unreachable dead code: sandbox.commands.run() without background: true resolves via CommandHandle.wait(), which already throws CommandExitError whenever the exit code is non-zero, before ever returning a result object (same pattern in both Python SDKs with CommandExitException).

Extended reasoning...

The bug. Commands.run() in foreground mode (packages/js-sdk/src/sandbox/commands/index.ts:423) returns proc.wait(), not a raw result. CommandHandle.wait() (commandHandle.ts:166-181) checks if (this.result.exitCode !== 0) throw new CommandExitError(this.result) and only returns this.result when the exit code is zero. So by the time res is assigned in Sandbox.create() (packages/js-sdk/src/sandbox/index.ts:325-334), res.exitCode can only ever be 0 — the subsequent if (res.exitCode !== 0) throw new Error(\Failed to start MCP gateway: ${res.stderr}`)can never execute. The same structure exists in both Python SDKs:sandbox_sync/main.pyandsandbox_async/main.pycallcommands.run()withoutbackground=True, which resolves through CommandHandle.wait() (command_handle.py:151-194), and that method raises CommandExitExceptionon a non-zero exit code (line 188-194) before it can return a result with a non-zeroexit_codefor the manual check to catch.\n\n**Why this matters now.** This exact block is pre-existing — it was just re-indented into the newtry { ... } catch (error) { await sandbox.kill()...; throw error }wrapper this PR adds. On its own the dead code would be a minor pre-existing nit. But this PR newly depends on the exact error message it produces: the added integration tests assert.rejects.toThrow('Failed to start MCP gateway') (create.test.ts) and pytest.raises(Exception, match='Failed to start MCP gateway') (test_create.py, sync and async), using mcp: { invalid_server: {} }to trigger the realistic non-zero-exit failure mode that mcp-gateway would raise for invalid config.\n\n**What actually happens when the test runs.** On invalid config, mcp-gateway exits non-zero.sandbox.commands.run(...)never returns aCommandResultat all — it throwsCommandExitError(JS) /CommandExitException(Python) straight out of thetryblock, and the manual exit-code check inside thetryis never reached. The message onCommandExitErrorissuper(result.error)(the envderrorfield), and Python'sCommandExitException.stris'Command exited with code N and error:\n'— neither contains the substring'Failed to start MCP gateway'. So the new tests' message-match assertions fail (or, if mcp-gateway happened to exit 0, no exception is raised at all and pytest.raises/.rejects.toThrowfails for that reason instead). Either way, the tests do not exercise the code path they claim to, and the documented user-facing error textFailed to start MCP gateway: is never actually produced for this failure mode.\n\n**Step-by-step proof (JS):**\n1.Sandbox.create({ mcp: { invalid_server: {} } })callssandbox.commands.run('mcp-gateway --config ...', {...})withoutbackground: true.\n2. Commands.run() (commands/index.ts:423) executes return opts?.background ? proc : proc.wait()→ sincebackgroundis not set, it callsproc.wait().\n3. mcp-gateway process exits with code != 0 due to invalid config.\n4. CommandHandle.wait() (commandHandle.ts:177-179) sees this.result.exitCode !== 0and throwsnew CommandExitError(this.result)— the promise fromsandbox.commands.run(...)rejects here; execution never reaches theconst res = ...assignment's continuation.\n5. Thethrowpropagates directly into the surroundingcatch (error)inSandbox.create(), which does await sandbox.kill().catch(() => undefined); throw error— so the cleanup does correctly run, buterroris aCommandExitError, not the intended Error('Failed to start MCP gateway: ...').\n6. The test's .rejects.toThrow('Failed to start MCP gateway')checks the rejected error's message against that substring;CommandExitError's message comes from the envd errorfield, so the assertion fails.\n\n**Fix.** Either (a) explicitly catchCommandExitError/CommandExitExceptioninside thetryand re-raise with the intendedFailed to start MCP gateway: message (usingerror.result.stderr/ the exception's stderr), so the documented contract holds and the new tests pass as written, or (b) drop the now-provably-dead manual exit-code check and adjust the test assertions to match on the actualCommandExitError/CommandExitExceptionmessage/type instead. Option (a) is preferable since it preserves the documented, more informative error text for SDK users.\n\nThe rollback/kill behavior itself (the PR's core goal) is unaffected — thecatch/except` still fires on any exception regardless of which one it is — so this does not break the primary feature, only the error-message contract and the new tests that assert it.


Expand Down
39 changes: 37 additions & 2 deletions packages/js-sdk/tests/sandbox/create.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { assert, test } from 'vitest'
import { assert, expect, test } from 'vitest'

import { Sandbox } from '../../src'
import { CommandExitError, Sandbox } from '../../src'
import { template, isDebug } from '../setup.js'

test.skipIf(isDebug)('create', async () => {
Expand Down Expand Up @@ -32,3 +32,38 @@ test.skipIf(isDebug)('metadata', async () => {
await sbx.kill()
}
})

test.skipIf(isDebug)(
'MCP gateway start failure kills the created sandbox',
async () => {
const metadata = { mcpGatewayCleanupTestId: crypto.randomUUID() }
const query = { state: ['running' as const], metadata }
let remainingSandboxes: Awaited<
ReturnType<ReturnType<typeof Sandbox.list>['nextItems']>
> = []

try {
// The base template has no mcp-gateway binary, so gateway startup
// reliably fails after the sandbox has been allocated.
await expect(
Sandbox.create(template, {
timeoutMs: 60_000,
metadata,
mcp: { invalid_server: {} } as never,
})
).rejects.toThrow(CommandExitError)

remainingSandboxes = await Sandbox.list({ query }).nextItems()
expect(remainingSandboxes).toEqual([])
} finally {
remainingSandboxes = await Sandbox.list({ query })
.nextItems()
.catch(() => remainingSandboxes)
await Promise.all(
remainingSandboxes.map((sandbox) =>
Sandbox.kill(sandbox.sandboxId).catch(() => false)
)
)
}
}
)
21 changes: 14 additions & 7 deletions packages/python-sdk/e2b/sandbox_async/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,13 +242,20 @@ async def create(
token = str(uuid.uuid4())
sandbox._mcp_token = token

res = await sandbox.commands.run(
f"mcp-gateway --config {shlex.quote(json.dumps(mcp))}",
user="root",
envs={"GATEWAY_ACCESS_TOKEN": token},
)
if res.exit_code != 0:
raise Exception(f"Failed to start MCP gateway: {res.stderr}")
try:
res = await sandbox.commands.run(
f"mcp-gateway --config {shlex.quote(json.dumps(mcp))}",
user="root",
envs={"GATEWAY_ACCESS_TOKEN": token},
)
if res.exit_code != 0:
raise Exception(f"Failed to start MCP gateway: {res.stderr}")
except BaseException:
try:
await sandbox.kill()
except BaseException:
pass
Comment on lines +256 to +257

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve cancellation during cleanup

When the async create task is cancelled while this best-effort await sandbox.kill() is in flight—for example an asyncio.timeout around AsyncSandbox.create(..., mcp=...) expires after gateway startup has already failed—asyncio.CancelledError is caught here and discarded. That can abort the DELETE request, still leave the sandbox orphaned, and then raise the original gateway error instead of honoring the caller's cancellation; suppress only ordinary cleanup failures or shield cleanup while re-propagating cancellation.

Useful? React with 👍 / 👎.

raise

return sandbox

Expand Down
21 changes: 14 additions & 7 deletions packages/python-sdk/e2b/sandbox_sync/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,13 +228,20 @@ def create(
token = str(uuid.uuid4())
sandbox._mcp_token = token

res = sandbox.commands.run(
f"mcp-gateway --config {shlex.quote(json.dumps(mcp))}",
user="root",
envs={"GATEWAY_ACCESS_TOKEN": token},
)
if res.exit_code != 0:
raise Exception(f"Failed to start MCP gateway: {res.stderr}")
try:
res = sandbox.commands.run(
f"mcp-gateway --config {shlex.quote(json.dumps(mcp))}",
user="root",
envs={"GATEWAY_ACCESS_TOKEN": token},
)
if res.exit_code != 0:
raise Exception(f"Failed to start MCP gateway: {res.stderr}")
except BaseException:
try:
sandbox.kill()
except BaseException:
pass
raise

return sandbox

Expand Down
31 changes: 30 additions & 1 deletion packages/python-sdk/tests/async/sandbox_async/test_create.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import asyncio
from typing import Any, cast
from uuid import uuid4

import httpx
import pytest

from e2b import AsyncSandbox, SandboxQuery, SandboxState
from e2b import AsyncSandbox, CommandExitException, SandboxQuery, SandboxState
from e2b.api.client.models import (
NewSandbox,
SandboxAutoResumeConfig,
Expand Down Expand Up @@ -36,6 +37,34 @@ async def test_metadata(async_sandbox_factory):
assert False, "Sandbox not found"


@pytest.mark.skip_debug()
async def test_mcp_gateway_start_failure_kills_created_sandbox(template):
metadata = {"mcp_gateway_cleanup_test_id": str(uuid4())}
query = SandboxQuery(state=[SandboxState.RUNNING], metadata=metadata)
remaining_sandboxes = []

try:
# The base template has no mcp-gateway binary, so gateway startup
# reliably fails after the sandbox has been allocated.
with pytest.raises(CommandExitException):
await AsyncSandbox.create(
template,
timeout=60,
metadata=metadata,
mcp=cast(Any, {"invalid_server": {}}),
)

remaining_sandboxes = await AsyncSandbox.list(query=query).next_items()
assert remaining_sandboxes == []
finally:
try:
remaining_sandboxes = await AsyncSandbox.list(query=query).next_items()
except Exception:
pass
for sandbox in remaining_sandboxes:
await AsyncSandbox.kill(sandbox.sandbox_id)


def test_create_payload_serializes_auto_resume_enabled():
body = NewSandbox(
template_id="template-id",
Expand Down
31 changes: 30 additions & 1 deletion packages/python-sdk/tests/sync/sandbox_sync/test_create.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
from time import sleep
from typing import Any, cast
from uuid import uuid4

import httpx
import pytest

from e2b import Sandbox, SandboxState
from e2b import CommandExitException, Sandbox, SandboxState
from e2b.api.client.models import (
NewSandbox,
SandboxAutoResumeConfig,
Expand Down Expand Up @@ -37,6 +38,34 @@ def test_metadata(sandbox_factory):
assert False, "Sandbox not found"


@pytest.mark.skip_debug()
def test_mcp_gateway_start_failure_kills_created_sandbox(template):
metadata = {"mcp_gateway_cleanup_test_id": str(uuid4())}
query = SandboxQuery(state=[SandboxState.RUNNING], metadata=metadata)
remaining_sandboxes = []

try:
# The base template has no mcp-gateway binary, so gateway startup
# reliably fails after the sandbox has been allocated.
with pytest.raises(CommandExitException):
Sandbox.create(
template,
timeout=60,
metadata=metadata,
mcp=cast(Any, {"invalid_server": {}}),
)

remaining_sandboxes = Sandbox.list(query=query).next_items()
assert remaining_sandboxes == []
finally:
try:
remaining_sandboxes = Sandbox.list(query=query).next_items()
except Exception:
pass
for sandbox in remaining_sandboxes:
Sandbox.kill(sandbox.sandbox_id)


def test_create_payload_serializes_auto_resume_enabled():
body = NewSandbox(
template_id="template-id",
Expand Down
Loading