Skip to content

fix(sdk): select stack-trace frames by SDK boundary instead of fixed depth - #1599

Merged
mishushakov merged 6 commits into
mainfrom
boundary-based-stack-frames
Jul 24, 2026
Merged

fix(sdk): select stack-trace frames by SDK boundary instead of fixed depth#1599
mishushakov merged 6 commits into
mainfrom
boundary-based-stack-frames

Conversation

@mishushakov

@mishushakov mishushakov commented Jul 23, 2026

Copy link
Copy Markdown
Member

Description

Template build stack traces were captured by walking a fixed number of frames (STACK_TRACE_DEPTH plus ±1 arithmetic at ~15 call sites), which broke whenever the frame count between new Error() and user code shifted — TS class-field initializer frames (#1539) and Bun's tail-call frame elision were both this bug. This PR makes two related changes:

  1. Boundary-based frame selection. The caller's frame is now the first one whose file lies outside the SDK package, making extra transpiler frames and elided delegating frames irrelevant. In the JS SDK, frame parsing is delegated to error-stack-parser-es (ESM-only, so it's a devDependency inlined into both dist formats via tsdown noExternal — the engines range includes Node versions without require(esm)); the Python SDK equivalently walks f_back until co_filename leaves the e2b package root, in the shared builder used by both sync and async. If no user frame is identifiable (e.g. the SDK is bundled into the caller's own file), capture degrades to no trace rather than a wrong frame.
  2. Dead machinery removed. Because boundary capture resolves through SDK-internal delegation (remove()runCmd(), fromDockerfile() → parser) to the user's call site on its own, the suppress/override collection machinery (runInNewStackTraceContext, runInStackTraceOverrideContext, the enabled/override flags, and their Python equivalents) became redundant and is removed — superseding the approach in feat(js-sdk): run the template test suite on Bun #1596.

Error .stack synthesis (keeping the Name: message header and the throw site on cause) was prototyped here and backed out — it will come as a follow-up PR.

Usage

No API changes — build errors now point at the user's call site regardless of runtime or transpiler:

const template = Template()
  .fromBaseImage()
  .runCmd('./does-not-exist') // ← build failures point exactly here

await Template.build(template, 'my-template')

Testing

  • JS: unit + template vitest projects green against the real API (incl. 27 per-method stacktrace tests pinning exact call-site line/columns, bunInstall now covered); edge-compat bundle test and CLI build verified; built CJS/ESM dists smoke-tested with require()/import().
  • Python: all 184 template tests green (shared + sync + async, incl. both test_stacktrace.py suites, bun_install now covered); ruff and ty clean.

🤖 Generated with Claude Code

…depth

Replace STACK_TRACE_DEPTH fixed-depth walking with boundary-based frame
selection: the caller's frame is the first one whose file lies outside
the SDK package. Frame parsing in the JS SDK is delegated to
error-stack-parser-es (ESM-only, inlined via tsdown noExternal since the
engines range includes Node versions without require(esm)). The Python
SDK walks f_back until co_filename leaves the e2b package root.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cla-bot cla-bot Bot added the cla-signed label Jul 23, 2026
@changeset-bot

changeset-bot Bot commented Jul 23, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 4e3aa98

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

This PR includes changesets to release 2 packages
Name Type
e2b Patch
@e2b/python-sdk 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

@cursor

cursor Bot commented Jul 23, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches error attribution across both SDKs' template builders with no public API change; wrong boundary logic would mis-point build failures, but behavior is heavily covered by stacktrace tests.

Overview
Template build errors now attribute steps to the user's call site by picking the first stack frame outside the SDK package, instead of a fixed STACK_TRACE_DEPTH. That stays correct when transpilers add frames (e.g. TS class fields) or runtimes skip delegating frames (e.g. Bun).

JS: getCallerFrame / getCallerDirectory use error-stack-parser-es and SDK-directory boundary detection; STACK_TRACE_DEPTH and stack-trace override helpers (runInNewStackTraceContext, runInStackTraceOverrideContext, enable/disable flags) are removed so delegating methods like remove()runCmd() still record the right site.

Python: Same boundary walk via inspect and the e2b package root; equivalent override machinery is removed.

Tests add bunInstall stacktrace coverage and assert caller resolution from outside the SDK; JS bundles the parser via tsdown noExternal.

Reviewed by Cursor Bugbot for commit 4e3aa98. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Package Artifacts

Built from d823efc. Download artifacts from this workflow run.

JS SDK (e2b@2.35.4-boundary-based-stack-frames.0):

npm install ./e2b-2.35.4-boundary-based-stack-frames.0.tgz

CLI (@e2b/cli@2.15.1-boundary-based-stack-frames.0):

npm install ./e2b-cli-2.15.1-boundary-based-stack-frames.0.tgz

Python SDK (e2b==2.34.0+boundary.based.stack.frames):

pip install ./e2b-2.34.0+boundary.based.stack.frames-py3-none-any.whl

mishushakov and others added 2 commits July 23, 2026 20:23
Boundary-based capture resolves through SDK-internal delegation to the
user's call site, so collectStackTrace inside runCmd/copy/parseDockerfile
already records the right frame. The disable/enable + collect-after
wrapper and the override context (and their Python equivalents) are
redundant — remove them and call the delegated methods directly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Errors carrying a template-definition stack trace previously overwrote
error.stack with raw frame lines, so reporters printing error.stack lost
the failure message and the SDK-internal throw site. withStackTrace()
now synthesizes `Name: message\n<frames>` lazily (subclass names set
after super() still appear) and preserves the natural stack on
error.cause. The copyItems re-throw wrapper is removed in both SDKs —
copy()'s boundary capture already points validation errors at the
copyItems call site.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mishushakov
mishushakov marked this pull request as ready for review July 23, 2026 18:38
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment thread packages/js-sdk/src/errors.ts Outdated
Plain `error.cause = x` creates an enumerable own property, so every
error decorated by withStackTrace() leaked a "cause" key into
Object.keys() and JSON.stringify() output. Native `new Error(msg,
{cause})` installs cause non-enumerable per spec — do the same via
Object.defineProperty.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reverts 9cd287b and 3c29239: withStackTrace() header synthesis,
cause preservation, and the copyItems re-throw removal. Errors go back
to plain `this.stack = stackTrace`; the boundary-based frame selection
itself is unaffected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Jul 24, 2026

Copy link
Copy Markdown

SDK-263

@mishushakov
mishushakov merged commit 3f46d56 into main Jul 24, 2026
34 checks passed
@mishushakov
mishushakov deleted the boundary-based-stack-frames branch July 24, 2026 12:42
mishushakov added a commit that referenced this pull request Jul 24, 2026
## Description

Adds `--project template` to `test:bun` so the Bun CI leg runs the
template suite, matching the Deno leg (#1595).

No code changes are needed: the template suite previously failed under
Bun because Bun's JavaScriptCore elides tail-call frames and the
fixed-depth stack walk attributed build errors one frame past the user's
call site (the workaround attempt in #1596 was closed in favor of
#1599). With #1599's boundary-based frame selection (now merged), the
suite passes under Bun as-is.

The CI workflow already passes `E2B_API_KEY`/`E2B_DOMAIN` to the Bun
leg, and the matrix comment (updated in #1595) already covers Bun
re-running API-backed suites, so `package.json` is the only change.

## Testing

Full `test:bun` (unit + connectionConfig + template) green locally on
Bun 1.3.14 against the real API: 530 passed, 35 skipped, 0 failed —
including all 34 stack-trace/caller-directory tests that pin exact user
call-site line/columns, the frames Bun used to elide.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants