Skip to content

fix(sdk): wrap thrown error bodies in real Error so TUI stops rendering [object Object]#26584

Merged
kitlangton merged 2 commits into
devfrom
kit/error-visibility-sdk-and-allsettled
May 9, 2026
Merged

fix(sdk): wrap thrown error bodies in real Error so TUI stops rendering [object Object]#26584
kitlangton merged 2 commits into
devfrom
kit/error-visibility-sdk-and-allsettled

Conversation

@kitlangton
Copy link
Copy Markdown
Contributor

What this fixes

Since the v1.14.42 Hono → Effect HttpApi migration, opencode HTTP errors that come back with a JSON body (the common case for 4xx — e.g. {name:"NotFoundError",data:{message:"Session not found: ses_x"}}) get thrown by the SDK as raw POJOs, not Error instances. The TUI catches a non-Error object whose `.message` and `.stack` are undefined, and renders `[object Object]` or blank — even though the server provided a perfectly usable message.

Verified empirically (probe in plan mode):

$ POST /session/ses_no_such → 404 {name:"NotFoundError",data:{message:"Session not found: ses_no_such"}}
SDK throws:
  typeof: object
  e instanceof Error: false
  e.message: undefined
  String(e): [object Object]

This is the same family as #26566 (TUI catches undefined .data) and the underlying reason users have been reporting opaque crashes since v1.14.42.

What this PR does

  1. Extracts a single shared wrapClientError interceptor in packages/sdk/js/src/error-interceptor.ts.
  2. Handles every error body shape: NamedError POJO → message extracted from data.message/message/name; plain object → fallback message; string → wrapped string; empty body / network failure → descriptive Error with method+url+status.
  3. Attaches the original parsed body and status under .cause for callers that need structured fields.
  4. Wires the same interceptor into both v1 (client.ts) and v2 (v2/client.ts) wrappers — plugins (which import the v1 SDK) get the same treatment as the TUI.
  5. Adds packages/opencode/test/server/sdk-error-shape.test.ts with two regression cases against the real in-process server: a 404 with NamedError body, and a 400 with empty body. Verified red→green→red→green during development.

What this PR does NOT change

  • Wire shape unchanged. Servers still respond with the same body. Plugins that don't use throwOnError: true (e.g. opencode-gemini-auth, which uses the result-tuple path) are byte-for-byte identical to before. This deliberately stays inside the deferred SchemaErrorMiddleware issue ([BUG on v1.14.42]: Upon trying to start up, non-descript error message is shown #26546 root cause).
  • The framework-level HttpMiddleware.logger is already running and already logs the cause server-side — that part is fine and untouched.

Followups

  • Fix D (Promise.allSettled in sync.tsx) — small, but needs additional test infra to assert against the bootstrap path. Separate PR.
  • Fix E (typed HttpApiError.BadRequest carries a message body) — would surface the cause to clients on top of just status. Larger surface; revisit after observing what opaque cases remain.

Test

  • bun run test test/server/sdk-error-shape.test.ts — 2 pass (would fail without wrapClientError)
  • bun run typecheck — clean

The generated v2 SDK already had an interceptor that wrapped *empty*
error bodies into Error, but for the common opencode case — a 4xx with
a NamedError-shaped JSON body like
\`{name:"NotFoundError",data:{message:"Session not found: ses_x"}}\` —
it threw the raw POJO. Downstream the TUI catches a non-Error object
whose \`.message\`/\`.stack\` are undefined and renders \`[object Object]\`
or blank, even though the server provided a perfectly usable message.

This:

- extracts \`wrapClientError\` to \`packages/sdk/js/src/error-interceptor.ts\`
- handles every body shape: NamedError POJO, plain object, string,
  empty / network failure
- attaches the original parsed body and status under \`.cause\`
- adds the same interceptor to the v1 wrapper so plugins (which
  import \`@opencode-ai/sdk\`, not \`/v2\`) get the same treatment

Includes a regression test that exercises both shapes against the
in-process server (404 with NamedError body; 400 with empty body).
PR review caught two regressions in the unconditional wrap:

1. Existing test test/server/httpapi-sdk.test.ts asserted the raw POJO
   shape on both result.error and the thrown value. The unconditional
   wrap broke the thrown assertion.
2. Two TUI consumers read result.error directly:
   dialog-workspace-create.tsx:111 (.name === \"VcsApplyError\")
   dialog-provider.tsx:189 (JSON.stringify(result.error))
   The wrapped Error has name === \"Error\" and stringifies to {},
   so both branches silently broke.

Fix: gate wrapClientError on opts.throwOnError. The result-tuple
path is now byte-for-byte identical to before — existing field
reads keep working. Only the throwing path gets the wrapped Error.

Update the existing httpapi-sdk test to assert the new throwing
shape (Error instance with .message + .cause.body) while
preserving the result-tuple expectation unchanged.
@kitlangton kitlangton merged commit 1136317 into dev May 9, 2026
10 checks passed
@kitlangton kitlangton deleted the kit/error-visibility-sdk-and-allsettled branch May 9, 2026 22:46
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.

1 participant