Skip to content

fix: proxy body forwarding, part ID prefix, and SDK error handling for prompt_async #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
drewsephski wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: proxy body forwarding, part ID prefix, and SDK error handling for prompt_async #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
0 ...use-bun-instead-of-node-vite-npm-pnpm.mdc → ...use-bun-instead-of-node-vite-npm-pnpm.mdc
View file
Open in desktop
File renamed without changes.
64 changes: 28 additions & 36 deletions apps/web/src/app/api/opencode/[port]/[[...path]]/route.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,11 +281,11 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => {
const fetchOptions = mockFetch.mock.calls[0]?.[1] as {
method: string
body: unknown
duplex?: string
}
expect(fetchOptions.method).toBe("POST")
expect(fetchOptions.body).toBeDefined()
expect(fetchOptions.duplex).toBe("half")
// Body is read as text string, not streamed
expect(typeof fetchOptions.body).toBe("string")
expect(JSON.parse(fetchOptions.body as string)).toEqual({ name: "Test Session" })
})

it("handles PUT requests with body", async () => {
Expand Down Expand Up @@ -313,11 +313,10 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => {
const fetchOptions = mockFetch.mock.calls[0]?.[1] as {
method: string
body: unknown
duplex?: string
}
expect(fetchOptions.method).toBe("PUT")
expect(fetchOptions.body).toBeDefined()
expect(fetchOptions.duplex).toBe("half")
expect(typeof fetchOptions.body).toBe("string")
expect(JSON.parse(fetchOptions.body as string)).toEqual({ name: "Updated Session" })
})

it("handles PATCH requests with body", async () => {
Expand Down Expand Up @@ -345,21 +344,16 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => {
const fetchOptions = mockFetch.mock.calls[0]?.[1] as {
method: string
body: unknown
duplex?: string
}
expect(fetchOptions.method).toBe("PATCH")
expect(fetchOptions.body).toBeDefined()
expect(fetchOptions.duplex).toBe("half")
expect(typeof fetchOptions.body).toBe("string")
expect(JSON.parse(fetchOptions.body as string)).toEqual({ status: "active" })
})

it("handles DELETE requests", async () => {
// TODO: BUG - Route crashes on 204 responses
// Current implementation calls response.text() for ALL responses
// then creates NextResponse(body, { status }), which fails for 204
// See message to coordinator - needs fix in route.ts
const mockFetch = vi.fn().mockResolvedValue({
ok: true,
status: 200, // Use 200 instead of 204 to avoid crash
status: 200,
headers: new Headers({ "content-type": "application/json" }),
text: vi.fn().mockResolvedValue('{"deleted": true}'),
})
Expand All @@ -375,7 +369,7 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => {
expect(response.status).toBe(200)
const fetchOptions = mockFetch.mock.calls[0]?.[1] as { method: string; body: unknown }
expect(fetchOptions.method).toBe("DELETE")
expect(fetchOptions.body).toBeNull()
expect(fetchOptions.body).toBeUndefined()
})

it("handles OPTIONS requests", async () => {
Expand Down Expand Up @@ -572,27 +566,25 @@ describe("API Proxy Route /api/opencode/[port]/[[...path]]", () => {
expect(response.headers.get("Content-Type")).toBe("application/json")
})

// TODO: BUG - Route crashes on 204 responses, skipping this test
// Once route.ts is fixed to handle 204 properly, uncomment this test:
// it("returns empty response for 204 No Content", async () => {
// global.fetch = vi.fn().mockResolvedValue({
// ok: true,
// status: 204,
// headers: new Headers(),
// text: vi.fn().mockResolvedValue(""),
// }) as unknown as typeof fetch
//
// const request = new NextRequest("http://localhost:3000/api/opencode/4056/sessions/123", {
// method: "DELETE",
// })
// const params = Promise.resolve({ port: "4056", path: ["sessions", "123"] })
//
// const response = await DELETE(request, { params })
//
// expect(response.status).toBe(204)
// const body = await response.text()
// expect(body).toBe("")
// })
it("returns empty response for 204 No Content", async () => {
global.fetch = vi.fn().mockResolvedValue({
ok: true,
status: 204,
headers: new Headers(),
text: vi.fn().mockResolvedValue(""),
}) as unknown as typeof fetch

const request = new NextRequest("http://localhost:3000/api/opencode/4056/sessions/123", {
method: "DELETE",
})
const params = Promise.resolve({ port: "4056", path: ["sessions", "123"] })

const response = await DELETE(request, { params })

expect(response.status).toBe(204)
const body = await response.text()
expect(body).toBe("")
})

it("handles large response bodies", async () => {
const largeBody = JSON.stringify({ data: "x".repeat(10000) })
Expand Down
17 changes: 12 additions & 5 deletions apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,18 +119,20 @@ async function proxyRequest(
}

// Copy body for POST/PUT/PATCH
let body: ReadableStream | null = null
// Read as string instead of streaming ReadableStream because:
// 1. Bun's fetch doesn't need duplex:"half" (Node.js-only option)
// 2. request.body can be null in Next.js 16 if body is already consumed
// 3. Most proxy payloads are small JSON - memory isn't a concern
let body: string | undefined
if (["POST", "PUT", "PATCH"].includes(request.method)) {
body = request.body
body = await request.text()
}

// Proxy request to OpenCode server
const response = await fetch(targetUrl, {
method: request.method,
headers,
body,
// @ts-expect-error - duplex mode needed for streaming request bodies
duplex: body ? "half" : undefined,
body: body || undefined,
})

// Handle non-2xx responses
Expand All @@ -144,6 +146,11 @@ async function proxyRequest(
)
}

// Handle 204 No Content (no body to read)
if (response.status === 204) {
return new NextResponse(null, { status: 204 })
}

// Return proxied response
const responseBody = await response.text()
return new NextResponse(responseBody, {
Expand Down
3 changes: 2 additions & 1 deletion apps/web/src/app/session/[id]/debug-panel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ export function DebugPanel({ sessionId, isOpen }: DebugPanelProps) {
const [fetchError, setFetchError] = useState<string | null>(null)

// Config and store hooks
const { directory } = getOpencodeConfig()
const { directory: rawDirectory } = getOpencodeConfig()
const directory = rawDirectory ?? ""
const messagesWithParts = useMessagesWithParts(sessionId)
const storeMessages = useMessages(sessionId)

Expand Down
18 changes: 9 additions & 9 deletions apps/web/src/app/session/[id]/session-layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,15 +266,15 @@ export function SessionLayout({
}: SessionLayoutProps) {
return (
<>
{/* Inject OpenCode config for factory hooks - must have directory from URL */}
{directory && (
<OpencodeSSRPlugin
config={{
baseUrl: "/api/opencode",
directory,
}}
/>
)}
{/* Inject OpenCode config for factory hooks - always render to override ServerStatus default */}
{/* ServerStatus sets directory:"" which breaks store lookups - session page must override */}
{/* Always pass directory even if empty string - OpencodeConfig requires it */}
<OpencodeSSRPlugin
config={{
baseUrl: "/api/opencode",
directory: directory ?? "",
}}
/>
<SessionContent
sessionId={sessionId}
directory={directory}
Expand Down
8 changes: 8 additions & 0 deletions bun.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion packages/core/src/api/prompt.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import type { AutocompleteState } from "../atoms/prompt.js"
*/
let partIdCounter = 0
function generatePartId(): string {
return `part-${Date.now()}-${partIdCounter++}`
// Backend validates part IDs must start with "prt"
return `prt-${Date.now()}-${partIdCounter++}`
}

/**
Expand Down
13 changes: 13 additions & 0 deletions packages/core/src/atoms/sessions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,19 @@ export const SessionAtom = {
path: { id: sessionId },
body,
})

// SDK returns { error: { status, statusText, body } } for non-2xx
// instead of throwing — check and throw so errors propagate
const sdkResult = result as {
error?: { status?: number; statusText?: string; body?: string }
data?: unknown
}
if (sdkResult.error) {
const detail =
sdkResult.error.body ?? sdkResult.error.statusText ?? "Unknown error"
throw new Error(detail)
}
Comment on lines +168 to +179
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

@hey-api/openapi-ts client response error object shape body statusText status

💡 Result:

In the @hey-api/openapi-ts ecosystem, the structure of error objects depends heavily on your throwOnError configuration and the specific client version used [1][2]. When throwOnError: false (the default) is used, the client returns a result object with a standardized shape rather than throwing an exception [1][2]. This object contains: - error: The parsed error body (or undefined if no error occurred) [1][2]. - response: An object containing the response metadata, including status, statusText, and headers [1][2]. Note that response may be undefined if the error resulted from a network failure or request construction issue [1][3]. - request: The request object, which may also be undefined if the error occurred before the request was built [1][3]. When throwOnError: true is used, behavior varies by configuration: - Default Behavior: The client throws the raw parsed error body directly [4][2]. This often results in the loss of HTTP metadata (status codes, etc.) because the thrown value is a plain object rather than an Error instance [4][2]. - Wrapper Behavior (Opt-in): By configuring the client with throwOnErrorStyle: 'wrapper' (available in newer versions), the client throws a FetchError instance [2]. This class extends Error and explicitly exposes the following properties: -.error: The typed backend error body [2]. -.status: The HTTP status code [2]. -.statusText: The status text [2]. -.headers: The response headers [2]. -.request: The request object (may be undefined) [2]. -.response: The response object (may be undefined) [2]. To ensure robust error handling, it is recommended to check for the existence of response before accessing its properties, as network-level errors will not have a response object [1][3]. If you require a consistent error shape throughout your application, using the 'wrapper' style is the preferred modern approach [2].

Citations:

Harden SDK error-to-Error conversion for non-string error.body

@hey-api/openapi-ts’s error field contains the parsed backend error body when using the non-throwing result style, so error.body is not guaranteed to be a string; if it’s an object, new Error(detail) will stringify to "[object Object]" and lose useful backend details.

🛠️ Proposed normalization 
 					const sdkResult = result as {
-						error?: { status?: number; statusText?: string; body?: string }
+						error?: { status?: number; statusText?: string; body?: unknown }
 						data?: unknown
 					}
 					if (sdkResult.error) {
-						const detail =
-							sdkResult.error.body ?? sdkResult.error.statusText ?? "Unknown error"
-						throw new Error(detail)
+						const { status, statusText, body } = sdkResult.error
+						const bodyText =
+							typeof body === "string" ? body : body != null ? JSON.stringify(body) : undefined
+						const detail = bodyText ?? statusText ?? "Unknown error"
+						throw new Error(status ? `[${status}] ${detail}` : detail)
 					}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/atoms/sessions.ts` around lines 168 - 179, The SDK error
handling in sessions.ts currently treats sdkResult.error.body as a string, which
can produce "[object Object]" for parsed error bodies; update the conversion in
the block that inspects sdkResult.error (the sdkResult variable) to normalize
the error body: if error.body is a string use it, if it's an object attempt
JSON.stringify(error.body) (catching and falling back to String(error.body) on
failure), and include status/statusText alongside the normalized body to build a
single informative detail string before throwing new Error(detail).


return result
},
catch: (error) =>
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/next-ssr-plugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { useServerInsertedHTML } from "next/navigation"

export interface OpencodeConfig {
baseUrl: string
directory: string
directory?: string
}

export interface OpencodeSSRPluginProps {
Expand Down