Sfoglia il codice sorgente

fix(sdk): wrap thrown error bodies in Error

SDK throwOnError paths now convert structured response bodies into real Error instances while preserving the original body and status in cause.
Kit Langton 3 mesi fa
parent
commit
11363170ca

+ 9 - 1
packages/opencode/test/server/httpapi-sdk.test.ts

@@ -394,8 +394,16 @@ describe("HttpApi SDK", () => {
         const missing = yield* capture(() => sdk.session.get({ sessionID }))
         const thrown = yield* captureThrown(() => sdk.session.get({ sessionID }, { throwOnError: true }))
 
+        // Result-tuple path: error body is preserved as-is so existing
+        // consumers reading `result.error.name` / `JSON.stringify(error)`
+        // keep working byte-for-byte.
         expect(missing.error).toEqual(expected)
-        expect(thrown).toEqual(expected)
+        // throwOnError path: SDK wraps the body in a real Error with the
+        // server's message, with the original parsed body preserved under
+        // `.cause.body`.
+        expect(thrown).toBeInstanceOf(Error)
+        expect((thrown as Error).message).toBe(expected.data.message)
+        expect(((thrown as Error).cause as { body: unknown }).body).toEqual(expected)
         return {
           status: missing.status,
           error: missing.error,

+ 74 - 0
packages/opencode/test/server/sdk-error-shape.test.ts

@@ -0,0 +1,74 @@
+/**
+ * Regression tests for the SDK error shape — the v2 SDK's `throwOnError: true`
+ * path used to throw raw values (empty strings or POJOs from JSON-decoded
+ * error bodies). The TUI catches those and `e.message`/`e.stack` are
+ * undefined, so users see `[object Object]` or a blank crash.
+ *
+ * Both cases must throw a real `Error` instance with a non-empty `.message`
+ * extracted from the response body, plus `.status` and `.body` attached.
+ */
+import { afterEach, describe, expect, test } from "bun:test"
+import { createOpencodeClient } from "@opencode-ai/sdk/v2"
+import { Server } from "../../src/server/server"
+import * as Log from "@opencode-ai/core/util/log"
+import { disposeAllInstances, tmpdir } from "../fixture/fixture"
+import { resetDatabase } from "../fixture/db"
+
+void Log.init({ print: false })
+
+afterEach(async () => {
+  await disposeAllInstances()
+  await resetDatabase()
+})
+
+function client(directory: string) {
+  return createOpencodeClient({
+    baseUrl: "http://test",
+    directory,
+    fetch: ((req: Request) => Server.Default().app.fetch(req)) as unknown as typeof fetch,
+  })
+}
+
+describe("v2 SDK error shape", () => {
+  test("404 with NamedError body throws a real Error carrying the server message", async () => {
+    await using tmp = await tmpdir({ git: true, config: { formatter: false, lsp: false } })
+    const sdk = client(tmp.path)
+
+    let caught: unknown
+    try {
+      await sdk.session.get({ sessionID: "ses_no_such" }, { throwOnError: true })
+    } catch (e) {
+      caught = e
+    }
+
+    expect(caught).toBeInstanceOf(Error)
+    const err = caught as Error
+    const cause = err.cause as { body?: any; status?: number }
+    expect(err.message).toContain("Session not found")
+    expect(cause.status).toBe(404)
+    expect(cause.body).toMatchObject({
+      name: "NotFoundError",
+      data: { message: expect.stringContaining("Session not found") },
+    })
+  })
+
+  test("400 with empty body throws a real Error naming the status", async () => {
+    await using tmp = await tmpdir({ config: { formatter: false, lsp: false } })
+    const sdk = client(tmp.path)
+
+    let caught: unknown
+    try {
+      // POST /sync/history with `aggregate: -1` triggers schema validation
+      // that returns an empty 400 body (verified via plan-mode probe).
+      await sdk.sync.history.list({ aggregate: -1 } as any, { throwOnError: true })
+    } catch (e) {
+      caught = e
+    }
+
+    expect(caught).toBeInstanceOf(Error)
+    const err = caught as Error
+    const cause = err.cause as { status?: number }
+    expect(err.message.length).toBeGreaterThan(0)
+    expect(cause.status).toBe(400)
+  })
+})

+ 2 - 0
packages/sdk/js/src/client.ts

@@ -3,6 +3,7 @@ export * from "./gen/types.gen.js"
 import { createClient } from "./gen/client/client.gen.js"
 import { type Config } from "./gen/client/types.gen.js"
 import { OpencodeClient } from "./gen/sdk.gen.js"
+import { wrapClientError } from "./error-interceptor.js"
 export { type Config as OpencodeClientConfig, OpencodeClient }
 
 function pick(value: string | null, fallback?: string) {
@@ -51,5 +52,6 @@ export function createOpencodeClient(config?: Config & { directory?: string }) {
 
   const client = createClient(config)
   client.interceptors.request.use((request) => rewrite(request, config?.directory))
+  client.interceptors.error.use(wrapClientError)
   return new OpencodeClient({ client })
 }

+ 51 - 0
packages/sdk/js/src/error-interceptor.ts

@@ -0,0 +1,51 @@
+/**
+ * Wrap whatever the generated client decoded from a non-2xx error body
+ * into a real `Error` so downstream formatters (TUI, plugins) get a
+ * useful `.message` instead of `[object Object]` or blank. The original
+ * parsed body and status live under `.cause` for callers that need
+ * structured fields.
+ *
+ * Only fires when the caller used `{ throwOnError: true }`. Callers that
+ * read `result.error` directly (the result-tuple path) get the parsed
+ * body unchanged so existing field-level reads (`.error.name`,
+ * `JSON.stringify(error)`, etc.) are byte-for-byte identical to before.
+ */
+export function wrapClientError(
+  error: unknown,
+  response: Response | undefined,
+  request: Request | undefined,
+  opts: { throwOnError?: boolean } | undefined,
+): unknown {
+  if (!opts?.throwOnError) return error
+  if (error instanceof Error) return error
+
+  // NamedError-shaped responses (the common case for opencode 4xx) come
+  // through as POJOs — extract a useful message first, then wrap.
+  if (typeof error === "object" && error !== null && Object.keys(error).length > 0) {
+    const obj = error as { data?: { message?: unknown }; message?: unknown; name?: unknown }
+    const message =
+      (typeof obj.data?.message === "string" && obj.data.message) ||
+      (typeof obj.message === "string" && obj.message) ||
+      (typeof obj.name === "string" && obj.name) ||
+      describe(request, response)
+    return new Error(message, { cause: { body: error, status: response?.status } })
+  }
+
+  if (typeof error === "string" && error.length > 0) {
+    return new Error(error, { cause: { body: error, status: response?.status } })
+  }
+
+  // Empty body / network failure / undefined / null / empty object.
+  const reason = response ? "(empty response body)" : "network error (no response)"
+  return new Error(`opencode server ${describe(request, response)}: ${reason}`, {
+    cause: { body: error, status: response?.status },
+  })
+}
+
+function describe(request: Request | undefined, response: Response | undefined) {
+  const method = request?.method ?? "?"
+  const url = request?.url ?? "?"
+  const status = response?.status
+  const statusText = response?.statusText
+  return `${method} ${url}${status ? " → " + status : ""}${statusText ? " " + statusText : ""}`
+}

+ 2 - 19
packages/sdk/js/src/v2/client.ts

@@ -3,6 +3,7 @@ export * from "./gen/types.gen.js"
 import { createClient } from "./gen/client/client.gen.js"
 import { type Config } from "./gen/client/types.gen.js"
 import { OpencodeClient } from "./gen/sdk.gen.js"
+import { wrapClientError } from "../error-interceptor.js"
 export { type Config as OpencodeClientConfig, OpencodeClient }
 
 function pick(value: string | null, fallback?: string, encode?: (value: string) => string) {
@@ -84,24 +85,6 @@ export function createOpencodeClient(config?: Config & { directory?: string; exp
 
     return response
   })
-  // The generated client falls back to throwing a literal `{}` when the server
-  // responds with an empty / unparseable error body, which surfaces as a bare
-  // `{}` in TUI / CLI error output. Wrap ONLY that case in a real Error so
-  // downstream formatters get a useful message — but pass through any parsed
-  // JSON error body unchanged so existing consumers can still inspect fields.
-  client.interceptors.error.use((error, response, request) => {
-    const isEmpty =
-      error === undefined ||
-      error === null ||
-      error === "" ||
-      (typeof error === "object" && !(error instanceof Error) && Object.keys(error).length === 0)
-    if (!isEmpty) return error
-    const method = request?.method ?? "?"
-    const url = request?.url ?? "?"
-    if (!response) return new Error(`opencode server ${method} ${url}: network error (no response)`)
-    const status = response.status
-    const statusText = response.statusText ? " " + response.statusText : ""
-    return new Error(`opencode server ${method} ${url} → ${status}${statusText}: (empty response body)`)
-  })
+  client.interceptors.error.use(wrapClientError)
   return new OpencodeClient({ client })
 }