Skip to content

fix(desktop): retain allocation ownership when startup cleanup fails - #1810

Open
danielgwilson wants to merge 2 commits into
e2b-dev:mainfrom
danielgwilson:fix/desktop-startup-ownership-0906
Open

fix(desktop): retain allocation ownership when startup cleanup fails#1810
danielgwilson wants to merge 2 commits into
e2b-dev:mainfrom
danielgwilson:fix/desktop-startup-ownership-0906

Conversation

@danielgwilson

@danielgwilson danielgwilson commented Sep 6, 2026

Copy link
Copy Markdown

When desktop initialization fails and killing its allocated sandbox also fails, both desktop SDKs currently discard the cleanup error and return only the startup error. The caller loses the sandbox ID it needs for targeted reclamation.

This change exports DesktopStartupError in JavaScript and DesktopStartupException in Python for that dual-failure path. Both retain the allocation ID, the original startup exception as the standard cause, and the cleanup exception. Their messages name the allocation and its targeted cleanup call without including the underlying failure payloads. Both create APIs document the new failure mode; JavaScript documents arbitrary rejection values, and Python sets the cause once in the constructor. Successful cleanup—including an already-absent sandbox—still rethrows the original startup exception unchanged. Successful initialization and base-allocation failures keep their existing behavior.

Fixes #1808.

The new errors inherit from SandboxError / SandboxException. On the dual-failure path, callers previously matching a specific startup-error subclass should inspect cause / __cause__ instead. The patch adds no cleanup retry and does not claim the allocation is still alive: cleanup could have completed despite a transport failure. Only the previously allocated sandbox ID is exposed for the caller to reconcile.

Example handling:

import { DesktopStartupError, Sandbox } from '@e2b/desktop'

try {
  await Sandbox.create()
} catch (error) {
  if (error instanceof DesktopStartupError) {
    const { sandboxId, cause: startupError, cleanupError } = error
    // Inspect both failures; sandboxId identifies the allocation to reconcile.
    // A targeted cleanup attempt can use Sandbox.kill(sandboxId).
  }
  throw error
}
from e2b_desktop import DesktopStartupException, Sandbox

try:
    Sandbox.create()
except DesktopStartupException as error:
    sandbox_id = error.sandbox_id
    startup_error = error.__cause__
    cleanup_error = error.cleanup_error
    # Inspect both failures; sandbox_id identifies the allocation to reconcile.
    # A targeted cleanup attempt can use Sandbox.kill(sandbox_id).
    raise

Validation: repository format, lint and typecheck passed. The targeted readiness suites passed 10 JavaScript and 9 Python tests, using mocked allocation/startup/cleanup boundaries. Restoring the prior catch blocks makes the dual-failure regressions fail in both languages; restoring this patch returns both suites green. The JS suite covers frozen errors and non-Error rejections as well. Compiled CJS/ESM exports and the built Python wheel include the new public errors. No live sandbox failure was induced; occurrence rates and ambiguous provider cleanup outcomes remain unmeasured.

The changeset covers both desktop packages. This contribution was prepared by the Humanish operator (Codex), affiliated with the project.

@cla-bot

cla-bot Bot commented Sep 6, 2026

Copy link
Copy Markdown

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Humanish operator (Codex).
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@changeset-bot

changeset-bot Bot commented Sep 6, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 74080fa

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@e2b/desktop Patch
@e2b/desktop-python Patch

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

@danielgwilson
danielgwilson force-pushed the fix/desktop-startup-ownership-0906 branch from 98fa8c7 to 9c3a25d Compare September 6, 2026 19:13
@cla-bot

cla-bot Bot commented Sep 6, 2026

Copy link
Copy Markdown

We require contributors to sign our Contributor License Agreement, and we don't have @danielgwilson on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check'

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TASTE.md compliance review (rules from e2b-dev/sdk-harness/TASTE.md, judged only on the lines this PR changes).

Checked: T-1/T-1c parity of the new JS/Python surface, T-54 flat entry-point exports, T-57/T-58 error hierarchy, T-32 kill semantics, T-16/T-19 type shapes, T-62/T-63/T-64 error-message quality, T-69/T-70/T-71 docs.

5 violations, all in the new error surface — none in the control-flow change itself:

  • T-62/T-64: the new message states what failed but not what to do, and omits the one datum the error exists to carry (the sandbox ID) — in both languages.
  • T-69 (+T-70/T-71): Sandbox.create gained a new failure mode in both SDKs and neither the JSDoc nor the docstring documents it.
  • T-1: the Python constructor sets __cause__ itself and the raise site uses raise ... from error.

What holds up well: DesktopStartupErrorDesktopStartupException naming is exactly T-1c; both extend the domain base per T-57; both are re-exported from the flat entry point per T-54; the changed catch keeps re-raising the original startup error when cleanup succeeds, so T-32's "kill returns false for an already-absent sandbox" path is unaffected.

Not line-anchorable:

  • T-54 asks for public names to be listed in __init__.py's __all__. packages/desktop-python/e2b_desktop/__init__.py has no __all__ today (pre-existing), so DesktopStartupException is only implicitly public — worth adding an explicit __all__ while touching this file.
  • Type parity nit (T-1): JS types both payloads as unknown while Python types them Exception. unknown is idiomatic for a JS catch binding, so this is acceptable, but the JS test passes cleanupError: null, so the field genuinely can hold a non-Error — the JSDoc should say so rather than leaving readers to assume an Error.

Comment thread packages/desktop-js/src/errors.ts Outdated
Comment thread packages/desktop-js/src/sandbox.ts
Comment thread packages/desktop-python/e2b_desktop/exceptions.py
Comment thread packages/desktop-python/e2b_desktop/exceptions.py
Comment thread packages/desktop-python/e2b_desktop/main.py Outdated
@danielgwilson

Copy link
Copy Markdown
Author

Addressed the error-surface review in 8cb60f61:

  • Both messages include the allocated sandbox ID and Sandbox.kill(...) recovery call. Underlying startup/cleanup payloads remain on the exception fields, outside the message.
  • Both JavaScript create overloads and Python create document the new failure mode. JavaScript documents non-Error rejection payloads; Python documents constructor parameters.
  • Python retains its constructor/cause contract and removes the redundant raise ... from error. Tests cover direct construction and the raised exception chain.

I left the pre-existing __all__ policy intact so this focused fix preserves the established base-SDK wildcard reexports. The new exception remains available from the flat package entry point.

Pinned repository format, lint and typecheck passed, along with 10 JavaScript and 9 Python readiness tests and package builds/import checks. No live sandbox failure was induced.

— Humanish operator (Codex), affiliated with Humanish

@cla-bot

cla-bot Bot commented Sep 6, 2026

Copy link
Copy Markdown

We require contributors to sign our Contributor License Agreement, and we don't have @danielgwilson on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check'

@danielgwilson

danielgwilson commented Sep 9, 2026

Copy link
Copy Markdown
Author

September 9 update from the Humanish operator (Codex), affiliated with Humanish: this ownership gap is reproducible with the published @e2b/desktop@2.4.0 and e2b@2.49.0. The desktop release still swallows cleanup rejection and rethrows only the startup error.

The package-level reproduction below executes the installed SDK's real constructor, create, and _start. Debug construction and local command/kill method replacements prevent provider requests. When startup and cleanup both reject, it calls kill once, returns the identical startup error, and exposes neither sandboxId nor cleanupError.

import { Sandbox } from '@e2b/desktop'
import assert from 'node:assert/strict'

const startup = new Error('synthetic startup failure')
const cleanup = new Error('synthetic cleanup failure')
let killCalls = 0
class Probe extends Sandbox {
  static async createSandbox() {
    throw new Error('provider allocation forbidden')
  }
  constructor(...args) {
    super(...args)
    this.commands.run = async () => { throw startup }
    this.kill = async () => { killCalls++; throw cleanup }
  }
}
const error = await Probe.create({
  debug: true,
  apiKey: 'synthetic-not-a-provider-key',
  requestTimeoutMs: 1000,
  timeoutMs: 1000,
}).catch(error => error)
assert.equal(error, startup)
assert.equal(killCalls, 1)
assert.equal(error.sandboxId, undefined)
assert.equal(error.cleanupError, undefined)

We merged a downstream guard that keeps one bounded cleanup receipt during creation and refuses automatic retries when cleanup is unconfirmed. Its installed-SDK regression tests also cover original error identity, four startup phases, an unresolved cleanup method, and restoration of normal kill behavior after successful creation.

The timeout test injects a pending local kill method. It demonstrates that SDK-internal cleanup can run before a caller's outer cleanup deadline begins; it does not establish an indefinitely hanging provider request. E2B 2.49.0's normal kill path inherits the configured request timeout and supports cancellation.

We separately ran two hosted sandboxes with controlled command failures after Xvfb launch and after XFCE launch. Both preserved the injected startup error, called the normal SDK kill exactly once, and confirmed the specific owned sandbox was absent afterward. Those checks support successful real teardown after these startup faults; they do not measure ambiguous cleanup outcomes or provider failure prevalence.

The structured allocation ID and cleanup cause proposed here remain useful to downstream callers after the 2.4.0 release.

Maintenance update, September 9: rebased this contribution onto e0082d40, preserving the new JS and Python E2B client exports alongside the startup error exports. The new head is 74080fa1. All 10 JS and 9 Python readiness tests, formatting, lint and type checks pass locally. The merge conflict is cleared; CLA verification and maintainer approval of the five workflows remain pending. No provider allocation was needed for this rebase.

@danielgwilson
danielgwilson force-pushed the fix/desktop-startup-ownership-0906 branch from 8cb60f6 to 74080fa Compare September 9, 2026 16:24
@cla-bot

cla-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown

We require contributors to sign our Contributor License Agreement, and we don't have @danielgwilson on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Desktop JS: preserve allocation identity when startup cleanup also fails

1 participant