Add trace ID extraction and appending to error messages - #1664
Conversation
When a failed API or envd response carries a trace header (X-Trace-ID, or the GCP X-Cloud-Trace-Context / AWS X-Amzn-Trace-Id edge headers), the JS SDK, Python SDK (sync and async), and CLI now append '(trace ID: ...)' to the error message so users can report the ID and it can be correlated with server-side traces. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0147b6bTi2gq4Yvm7VrLrxLD
🦋 Changeset detectedLatest commit: b7eeae2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
PR SummaryLow Risk Overview Reviewed by Cursor Bugbot for commit b7eeae2. Bugbot is set up for automated code reviews on this repo. Configure here. |
Package ArtifactsBuilt from 84e7f36. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.39.1-claude-trace-id-error-messages-cbzqwl.0.tgzCLI ( npm install ./e2b-cli-2.16.2-claude-trace-id-error-messages-cbzqwl.0.tgzPython SDK ( pip install ./e2b-2.39.1+claude.trace.id.error.messages.cbzqwl-py3-none-any.whl |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0147b6bTi2gq4Yvm7VrLrxLD
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if (key?.toLowerCase() !== 'root' || !value) { | ||
| continue | ||
| } | ||
|
|
||
| const match = value.match(/^1-([0-9a-f]{8})-([0-9a-f]{24})$/i) | ||
| return match ? match[1] + match[2] : value | ||
| } | ||
| } | ||
|
|
||
| return undefined | ||
| } | ||
|
|
||
| function throwE2BRequestError( | ||
| error: E2BResponseError, | ||
| errMsg?: string, | ||
| traceId?: string | ||
| ): never { | ||
| let message: string | ||
| const code = error.code ?? 0 | ||
| switch (code) { |
There was a problem hiding this comment.
🟡 CLAUDE.md requires 'Create or update tests covering affected codepaths,' but this PR leaves packages/cli/tests/utils/errors.test.ts untouched even though errors.ts gains a new extractTraceId() function and trace-ID appending in throwE2BRequestError/handleE2BRequestError. No case exercises X-Trace-ID, GCP X-Cloud-Trace-Context, or AWS X-Amzn-Trace-Id extraction/normalization for the CLI, unlike the JS SDK (new traceId.test.ts + updated handleApiError.test.ts/handleEnvdApiError.test.ts) and Python SDK (test_trace_id.py), which both got full coverage for the identical logic.
Extended reasoning...
This PR introduces a new, non-trivial codepath in packages/cli/src/utils/errors.ts: extractTraceId() parses three different header formats (a direct X-Trace-ID, the GCP X-Cloud-Trace-Context edge header which requires splitting on /, and the AWS X-Amzn-Trace-Id edge header which requires parsing a Root=1-<8 hex>-<24 hex> field and normalizing it into a 32-hex trace ID), and it wires the extracted value into throwE2BRequestError/handleE2BRequestError so it gets appended to the thrown error message as (trace ID: ...).
None of this is covered by tests in the CLI package. packages/cli/tests/utils/errors.test.ts already exists and already tests handleE2BRequestError for various status codes, but the PR leaves it completely untouched — a grep for trace/Trace across packages/cli/tests returns nothing. There is no test that passes a response/headers-bearing object into handleE2BRequestError, so none of the three extraction branches, the AWS regex normalization, the priority ordering (direct > GCP > AWS), or the final message-appending behavior is exercised at all for the CLI.
This is a direct violation of the repository's own CLAUDE.md instructions, which explicitly state: 'Create or update tests covering affected codepaths and run them using pnpm run test.' The PR's own diff demonstrates the intended standard: the JS SDK got a full new traceId.test.ts (extraction, normalization, priority ordering, edge cases) plus updated handleApiError.test.ts and handleEnvdApiError.test.ts with trace-ID-specific cases, and the Python SDK got a full new test_trace_id.py covering the same logic end to end. The CLI implementation — which, per the related duplication finding, re-implements this parsing logic independently rather than sharing it with the SDK's traceId.ts — is the only one of the three places this feature landed that received zero test coverage.
Concrete proof of the gap: take the AWS-header path, since it's the most failure-prone (regex parsing + normalization). In extractTraceId, if X-Amzn-Trace-Id is Root=1-5759e988-bd862e3fe1be46a994272793;Sampled=1, the code should return 5759e988bd862e3fe1be46a994272793 (the two hex groups joined), and this exact case is verified in both packages/js-sdk/tests/traceId.test.ts and packages/python-sdk/tests/test_trace_id.py. But if the CLI's regex or splitting logic were subtly wrong — e.g. an off-by-one in the hex-group lengths, or the ; split failing to find Root= first — nothing in pnpm run test for the CLI package would catch it, since handleE2BRequestError is only ever invoked in the test suite without a response argument. The same blind spot applies to the GCP-header branch and to the case-insensitivity of headers.get('x-trace-id').
Fix: add test cases to packages/cli/tests/utils/errors.test.ts mirroring the ones already written for the JS SDK — pass a response: { headers: new Headers({...}) } into handleE2BRequestError and assert the thrown E2BRequestError's message ends with (trace ID: ...) for each of: direct X-Trace-ID, GCP X-Cloud-Trace-Context, AWS X-Amzn-Trace-Id (including the raw-fallback case for a malformed Root= value), priority ordering when multiple headers are present, and the no-headers case where the message is left unchanged.
This does not block merging on its own — missing tests don't cause a runtime failure, and the trace-ID feature works correctly as shipped. But it's an explicit, unambiguous violation of a stated project convention, and the parity gap with the JS/Python SDKs (both of which got thorough coverage for the identical logic) makes it worth flagging as a nit for the author to close before merge.
| } | ||
|
|
||
| /** | ||
| * Extract a trace ID from HTTP response headers of a failed request, so the | ||
| * error message can carry an ID that E2B support can correlate with | ||
| * server-side traces. Checks `X-Trace-ID`, then the GCP | ||
| * (`X-Cloud-Trace-Context`) and AWS (`X-Amzn-Trace-Id`) edge trace headers. | ||
| */ | ||
| function extractTraceId(response?: ResponseHeadersLike): string | undefined { | ||
| const headers = response?.headers | ||
| if (!headers || typeof headers.get !== 'function') { | ||
| return undefined | ||
| } | ||
|
|
||
| const direct = headers.get('x-trace-id')?.trim() | ||
| if (direct) { | ||
| return direct | ||
| } | ||
|
|
||
| // GCP edge: "TRACE_ID/SPAN_ID;o=OPTIONS" | ||
| const gcp = headers.get('x-cloud-trace-context')?.split('/')[0]?.trim() | ||
| if (gcp) { | ||
| return gcp | ||
| } | ||
|
|
||
| // AWS edge: "Root=1-<8 hex>-<24 hex>;..." — the two hex parts joined are | ||
| // the 32-hex trace ID the server logs | ||
| const aws = headers.get('x-amzn-trace-id') | ||
| if (aws) { | ||
| for (const field of aws.split(';')) { |
There was a problem hiding this comment.
🟡 packages/cli/src/utils/errors.ts:37-70 duplicates the exact trace-ID header parsing logic (X-Trace-ID / X-Cloud-Trace-Context / X-Amzn-Trace-Id, including the AWS regex and GCP split) already implemented in packages/js-sdk/src/traceId.ts's extractTraceId. Since the CLI already depends on and imports from 'e2b', consider exporting extractTraceId from js-sdk's public index and importing it here instead of copy-pasting, to avoid the two copies silently drifting on future changes.
Extended reasoning...
What the bug is: packages/cli/src/utils/errors.ts (lines 29-58/37-70) reimplements a byte-for-byte copy of the trace-ID extraction logic that this same PR adds to packages/js-sdk/src/traceId.ts. Both versions check headers in the same priority order — X-Trace-ID (direct, trimmed), X-Cloud-Trace-Context (GCP edge, split('/')[0].trim()), and X-Amzn-Trace-Id (AWS edge, splitting on ;, matching Root=1-<8 hex>-<24 hex> with the identical regex /^1-([0-9a-f]{8})-([0-9a-f]{24})$/i, and joining the two hex groups) — with only the wrapper signature differing (CLI takes a {headers}-shaped object, js-sdk's version takes the Headers object directly).\n\nWhy it happens / code path: This PR introduces extractTraceId/appendTraceId in packages/js-sdk/src/traceId.ts and wires them into packages/js-sdk/src/api/index.ts and packages/js-sdk/src/envd/api.ts. Rather than reusing that helper for the CLI's error handling in packages/cli/src/utils/errors.ts, the PR hand-writes an equivalent extractTraceId function locally. The CLI package already depends on and imports from e2b (see packages/cli/src/api.ts: import * as e2b from 'e2b', and package.json lists e2b: workspace:^), so nothing structurally prevents sharing the implementation — it's purely that extractTraceId is not currently re-exported from js-sdk's public index.ts, which would need to be added for the CLI to import it.\n\nWhy existing code doesn't prevent it: There's no lint rule or test that would catch two independently-written implementations of the same parsing logic living in separate packages; both pass their own test suites (traceId.test.ts for js-sdk, presumably covered indirectly for the CLI) without any cross-check that they stay in sync.\n\nImpact: This doesn't cause incorrect behavior today — both copies are currently identical and produce the same trace IDs. The risk is purely maintenance: any future change to trace-ID parsing (e.g. supporting a new header format, fixing an edge case in the AWS regex, adjusting GCP parsing) has to be applied in two places, and it would be easy to update one copy and forget the other, causing the CLI and SDK to silently disagree on trace IDs for the same failure.\n\nProof (concrete example): Suppose in a follow-up PR someone fixes a bug in the AWS regex — say widening it to accept a 16-hex span ID variant, changing the pattern in packages/js-sdk/src/traceId.ts only (since that's the 'canonical' looking implementation reused by the JS SDK's API/envd handlers). packages/cli/src/utils/errors.ts's local copy still has the old regex. Now: a request fails with an AWS trace header in the new format; the JS SDK correctly extracts and reports the trace ID; the CLI (which wraps the same underlying e2b client and hits the same API) falls through the regex match and falls back to the raw Root=... string (or drops the ID depending on which branch), producing an inconsistent/wrong trace ID for the exact same server-side failure. A user following CLI-reported error output to file a support ticket would give E2B support a malformed or missing trace ID, right where this feature is supposed to help.\n\nHow to fix: Export extractTraceId (and optionally appendTraceId) from js-sdk's public src/index.ts, then in packages/cli/src/utils/errors.ts import it and adapt the call site to pass res.response?.headers directly (a trivial signature bridge, since the CLI's ResponseHeadersLike.headers is already the same {get(name): string|null} shape as js-sdk's HeadersLike), deleting the ~45-line duplicated block entirely.
mishushakov
left a comment
mishushakov
left a comment
There was a problem hiding this comment.
reviewed, please remove duplicate method in JS SDK and you can modify the default Error classes to accept a trace field in the constructor that will append it to the message, which is cleaner than relying on helpers side-effects
|
|
||
| function throwE2BRequestError(error: E2BResponseError, errMsg?: string): never { | ||
| type ResponseHeadersLike = { | ||
| headers?: { get(name: string): string | null } |
There was a problem hiding this comment.
you can use built-in Headers type from undici?
| errorClass, | ||
| stackTrace | ||
| return appendTraceId( | ||
| apiErrorFromCode( |
There was a problem hiding this comment.
maybe just change apiErrorFromCode instead
| * 3. `X-Amzn-Trace-Id` (AWS edge) — `Root=1-<8 hex>-<24 hex>;...`, the two | ||
| * hex parts joined are the 32-hex trace ID the server logs. | ||
| */ | ||
| export function extractTraceId( |
There was a problem hiding this comment.
this is same code as in packages/cli/src/utils/errors.ts
| * Append the trace ID of the failed request to the error message, so users | ||
| * can include it when reporting the failure to E2B. | ||
| */ | ||
| export function appendTraceId<T extends Error | undefined>( |
There was a problem hiding this comment.
you could also modify E2B error classes to have trace id in message instead of this
| if message is None and e.status_code not in (401, 429): | ||
| return default_exception_class(f"{e.status_code}: {e.content}").with_traceback( | ||
| stack_trace | ||
| return append_trace_id( |
There was a problem hiding this comment.
feel free to update default_exception_class instead
Address review feedback: - Error/exception classes now take an optional trace ID in the constructor and append it to the message, instead of a post-hoc append helper; apiErrorFromCode / api_exception_from_code and the envd error maps thread it through. - The CLI reuses extractTraceId exported from the JS SDK instead of keeping its own copy, and types headers with the built-in Headers. - Cover AWS raw-value fallback and header priority in the CLI tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0147b6bTi2gq4Yvm7VrLrxLD
|
Addressed the review in 0adc234:
Also added CLI test cases for the AWS raw-value fallback and header priority. Generated by Claude Code |
|
@djeebus Second pass — I re-reviewed after 0adc234 and verified the build locally. All five of my earlier comments are addressed; details plus a few independent findings below. My earlier comments
A. The feature is inert against production today, and 2 of its 3 header paths likely never fireYou flagged this in the description and asked someone to verify which header a real failed response returns. I checked against live prod:
No trace header on any path. And Suggestion: land B. JS error-class arity is now inconsistent, and it's a live trap
apiErrorFromCode(500, 'boom', AuthenticationError, 'STACK', 'realtrace')
// typechecks clean; message === "500: boom (trace ID: STACK)"No current call site hits it — Related: six call sites now read C. Unrelated change bundled in
D.
|
…ck change Address second-pass review: - AuthenticationError/GitAuthError take (message, stackTrace, traceId) like the other error classes, closing the arity trap where a custom errorClass could receive the stack trace in the trace ID slot. - VolumeError no longer applies the stack trace, restoring its pre-existing behavior; the parameter stays for the errorClass shape. - Drop the unreachable typeof guard in extractTraceId. - Export extract_trace_id from the Python package for JS/Python parity with the new extractTraceId export, and mention both in the changeset. - Declare headers on SupportsApiErrorResponse so ty checks the access. - Cover custom errorClass trace ID pass-through in handleApiError tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0147b6bTi2gq4Yvm7VrLrxLD
|
Second-pass items addressed in b7eeae2:
A is @djeebus's call, so I've left the GCP/AWS branches in place for now and updated the PR description with your production findings (no trace header on any live path, Build Packages failure on 0adc234 was the transient 503 you diagnosed; the new push re-runs CI. Generated by Claude Code |
3 participants
Summary
This change adds trace ID extraction from HTTP response headers and appends them to error messages across the SDK and CLI. When API or envd requests fail, the error message now includes the trace ID (e.g.,
(trace ID: abc123)) so users can report it to E2B support for correlation with server-side traces.Usage examples
No behavioral API changes — the trace ID shows up in existing error messages when the failed response carries a trace header.
JS SDK
Python SDK (sync and async)
CLI
Key Changes
Trace ID parsing:
packages/js-sdk/src/traceId.ts(single implementation, exported asextractTraceIdfrom thee2bpackage and reused by the CLI) ande2b/trace_id.py(exported asextract_trace_idfor JS/Python parity):X-Trace-ID(direct),X-Cloud-Trace-Context(GCP edge),X-Amzn-Trace-Id(AWS edge)Root=1-<8 hex>-<24 hex>to the 32-hex form the server logs asedge_trace_idError classes carry the trace ID in the constructor and append it to the message:
SandboxError(message, stackTrace, traceId)and the other JS classes (uniform 3-arg shape, includingAuthenticationError/GitAuthError);SandboxException(message, trace_id)and the other Python exception roots.apiErrorFromCode/api_exception_from_codeand the envd error maps thread the trace ID through.Wiring:
handleApiError(JS),handle_api_exception(Python),handleEnvdApiError/handle_envd_api_exception/ahandle_envd_api_exception(envd HTTP), and the CLI'shandleE2BRequestErrorread the response headers and pass the extracted ID into the error constructors.Not covered — envd RPC (connect) paths:
sandbox.commands/sandbox.filesRPC errors do not carry the trace ID, because Python'sconnectrpc.ConnectErrorexposes no response metadata, and the JS side is kept symmetric. This is where most in-sandbox failures surface, so the feature currently reaches HTTP API and envd HTTP (file transfer) errors only.Test coverage: header extraction and normalization edge cases, priority ordering, case-insensitivity, missing-header fallbacks, custom
errorClass/error_mappass-through, and CLI parsing — in JS, Python (sync + async), and the CLI.Note on the server side
Verified against production (2026-08-13): no failed
api.e2b.devor sandbox-edge response currently returns any trace header, so this change is inert until the server emits one. The API handlers already compute the trace ID (c.Set("traceID", traceID)in infra) — a one-linec.Header("X-Trace-ID", traceID)there would activate the primary path. The GCP/AWS edge-header branches are speculative (those are request-side conventions); whether to keep them or trim toX-Trace-ID-only once the infra change lands is an open reviewer question for @djeebus.https://claude.ai/code/session_0147b6bTi2gq4Yvm7VrLrxLD