fix(python-sdk): don't crash the build error path on a non-numeric reason.step - #1802
fix(python-sdk): don't crash the build error path on a non-numeric reason.step#1802nblintao wants to merge 2 commits into
Conversation
|
We require contributors to sign our Contributor License Agreement, and we don't have @nblintao 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' |
🦋 Changeset detectedLatest commit: 9f88c45 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
TASTE.md review: checked T-1/T-2 (sync/async parity — both build_api.py mirrors updated identically), T-42/T-62 (build error path still raises BuildException and threads the local traceback), T-71 (reST docstring updated for the new None return). No violations found in the changed lines; the internal helper's Optional[int] return and the is not None and 0 <= idx < len guard are the compliant form.
Non-blocking parity note (not a changed line): the JS twin getBuildStepIndex in packages/js-sdk/src/template/utils.ts returns Number(step) → NaN for the same inputs, which happens to index to undefined rather than crash, so behavior matches — but its @returns still claims an index is always returned. Worth aligning the JSDoc (T-1) in a follow-up.
…ason.step get_build_step_index hard-codes that reason.step is "base", "finalize", or a numeric string, and int(step) has no fallback — any other value raises ValueError from inside the error path, masking the real BuildException. Such values occur in practice: e2b Cloud's own optimize and resize-disk phases fill step with those literal strings, and E2B-compatible backends have shipped instruction text there (the spec leaves the field an unconstrained string). A step with no usable index now maps to None and the build error is raised without a local traceback, matching the JS SDK's behavior. Also bounds-check negative indices at both call sites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
b7964e0 to
6ca3017
Compare
|
We require contributors to sign our Contributor License Agreement, and we don't have @nblintao 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' |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
We require contributors to sign our Contributor License Agreement, and we don't have @nblintao 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' |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
1 participant
Problem
On a failed template build,
get_build_step_index(packages/python-sdk/e2b/template/utils.py) assumesreason.stepis"base","finalize", or a numeric string —int(step)has no fallback, and the call sites intemplate_sync/build_api.py/template_async/build_api.pydon't catch. Any other value raisesValueErrorfrom inside the error path, masking the realBuildExceptionthe user should have seen.Non-numeric values occur in practice:
stepwith the literal strings"optimize"and"resize-disk"when those phases fail (e2b-dev/infra,packages/orchestrator/pkg/template/build/phases/{optimize,ensurefreedisk}/builder.go→UnwrapUserErrorpassesStepTypethrough verbatim) — so the crash is reachable against the official backend, not only third-party ones.BuildStatusReason.step: "Step that failed"), so nothing constrains it to be numeric.The JS SDK already degrades gracefully here (
Number(step)→NaN→ no traceback, error still thrown); this brings the Python SDK to parity.Change
get_build_step_indexreturnsOptional[int]: non-numeric, non-sentinel steps map toNone("no matching local stack trace") instead of raising.Noneand negative indices, so theBuildExceptionis raised without a local traceback rather than being masked.🤖 Generated with Claude Code