From 05d279cc652b93e6053696fa8cefb94970f9b09e Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Tue, 28 Apr 2026 11:58:19 +0700 Subject: [PATCH] fix(server): surface real errors and child-process output in diagnostics Provider diagnostics were stringifying Error objects through JSON.stringify, which yields {} and hides the cause. Spawned children logged stderr to the daemon log but never attached it to thrown errors, so the diagnostic showed "Server: Unavailable ({})" with no clue why. Generalize toDiagnosticErrorMessage to extract message, code, signal, stderr, stdout, and recursive cause from any Error (including execFile-style errors), and never return {} or empty. Buffer stderr/stdout in OpenCode startServer and include them in startup-failure rejections. Stop swallowing errors in resolveBinaryVersion. Drop the duplicate stringifyUnknownError helpers in opencode-agent and pi-direct-agent and route every callsite through the single canonical helper. --- .../agent/providers/diagnostic-utils.test.ts | 78 +++++++++++++++ .../agent/providers/diagnostic-utils.ts | 97 +++++++++++++++++-- .../server/agent/providers/opencode-agent.ts | 82 +++++++++------- .../server/agent/providers/pi-direct-agent.ts | 13 +-- 4 files changed, 216 insertions(+), 54 deletions(-) create mode 100644 packages/server/src/server/agent/providers/diagnostic-utils.test.ts diff --git a/packages/server/src/server/agent/providers/diagnostic-utils.test.ts b/packages/server/src/server/agent/providers/diagnostic-utils.test.ts new file mode 100644 index 000000000..93770d772 --- /dev/null +++ b/packages/server/src/server/agent/providers/diagnostic-utils.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, test } from "vitest"; + +import { toDiagnosticErrorMessage } from "./diagnostic-utils.js"; + +describe("toDiagnosticErrorMessage", () => { + test("returns message for plain Error", () => { + expect(toDiagnosticErrorMessage(new Error("boom"))).toBe("boom"); + }); + + test("includes stderr, stdout, code, and signal from execFile-style Error", () => { + const error = new Error("Command failed: opencode --version") as Error & { + stderr: string; + stdout: string; + code: number; + signal: string; + }; + error.stderr = "permission denied\n"; + error.stdout = "partial output"; + error.code = 127; + error.signal = "SIGTERM"; + + const message = toDiagnosticErrorMessage(error); + expect(message).toContain("Command failed: opencode --version"); + expect(message).toContain("exit code: 127"); + expect(message).toContain("signal: SIGTERM"); + expect(message).toContain("stderr: permission denied"); + expect(message).toContain("stdout: partial output"); + }); + + test("preserves multi-line stderr on a real Error", () => { + const error = new Error("Command failed") as Error & { stderr: string }; + error.stderr = "line one\nline two\nline three"; + const message = toDiagnosticErrorMessage(error); + expect(message).toContain("stderr: line one\nline two\nline three"); + }); + + test("recursively formats Error cause", () => { + const inner = new Error("inner failure") as Error & { stderr: string }; + inner.stderr = "inner stderr"; + const outer = new Error("outer failure", { cause: inner }); + const message = toDiagnosticErrorMessage(outer); + expect(message).toContain("outer failure"); + expect(message).toContain("caused by: inner failure"); + expect(message).toContain("stderr: inner stderr"); + }); + + test("serializes plain objects rather than returning {}", () => { + expect(toDiagnosticErrorMessage({ foo: "bar" })).toBe('{"foo":"bar"}'); + }); + + test("returns trimmed string when given a string", () => { + expect(toDiagnosticErrorMessage("oops")).toBe("oops"); + expect(toDiagnosticErrorMessage(" spaced ")).toBe("spaced"); + }); + + test("returns Unknown error for null and undefined", () => { + expect(toDiagnosticErrorMessage(null)).toBe("Unknown error"); + expect(toDiagnosticErrorMessage(undefined)).toBe("Unknown error"); + }); + + test("returns Unknown error for an Error with no message and no extras", () => { + const error = new Error(""); + expect(toDiagnosticErrorMessage(error)).toBe("Unknown error"); + }); + + test("truncates very long stderr", () => { + const long = "x".repeat(5000); + const error = new Error("big") as Error & { stderr: string }; + error.stderr = long; + const message = toDiagnosticErrorMessage(error); + expect(message).toContain("…(truncated)"); + expect(message.length).toBeLessThan(long.length + 200); + }); + + test("returns Unknown error for empty plain object after serialization fallback", () => { + expect(toDiagnosticErrorMessage({})).toBe("Unknown error"); + }); +}); diff --git a/packages/server/src/server/agent/providers/diagnostic-utils.ts b/packages/server/src/server/agent/providers/diagnostic-utils.ts index 08c7e5bd0..248cbe9bf 100644 --- a/packages/server/src/server/agent/providers/diagnostic-utils.ts +++ b/packages/server/src/server/agent/providers/diagnostic-utils.ts @@ -14,7 +14,7 @@ export function formatProviderDiagnosticError(providerName: string, error: unkno return formatProviderDiagnostic(providerName, [ { label: "Error", - value: error instanceof Error ? error.message : String(error), + value: toDiagnosticErrorMessage(error), }, ]); } @@ -33,16 +33,97 @@ export function formatDiagnosticStatus( return formatAvailabilityStatus(available); } -export function toDiagnosticErrorMessage(error: unknown): string { - if (error instanceof Error && error.message) { - return error.message; +const DIAGNOSTIC_OUTPUT_CAP = 4096; + +function truncateForDiagnostic(value: string): string { + const trimmed = value.trim(); + if (trimmed.length <= DIAGNOSTIC_OUTPUT_CAP) { + return trimmed; } - if (typeof error === "string" && error.trim().length > 0) { - return error; + return `${trimmed.slice(0, DIAGNOSTIC_OUTPUT_CAP)}…(truncated)`; +} + +function readStringProperty(error: object, key: string): string | undefined { + const value = (error as Record)[key]; + if (typeof value === "string") { + return value; + } + if (typeof value === "number" || typeof value === "bigint") { + return String(value); + } + return undefined; +} + +function readUnknownProperty(error: object, key: string): unknown { + return (error as Record)[key]; +} + +function pushIfNonEmpty(sections: string[], label: string, value: string | undefined): void { + if (value && value.trim().length > 0) { + sections.push(`${label}: ${value.trim()}`); + } +} + +function pushTruncatedIfNonEmpty( + sections: string[], + label: string, + value: string | undefined, +): void { + if (value && value.trim().length > 0) { + sections.push(`${label}: ${truncateForDiagnostic(value)}`); + } +} + +function formatErrorDiagnostic(error: Error): string { + const sections: string[] = []; + if (error.message && error.message.trim().length > 0) { + sections.push(error.message.trim()); + } + pushIfNonEmpty(sections, "exit code", readStringProperty(error, "code")); + pushIfNonEmpty(sections, "signal", readStringProperty(error, "signal")); + pushTruncatedIfNonEmpty(sections, "stderr", readStringProperty(error, "stderr")); + pushTruncatedIfNonEmpty(sections, "stdout", readStringProperty(error, "stdout")); + const cause = readUnknownProperty(error, "cause"); + if (cause !== undefined && cause !== null) { + const causeMessage = toDiagnosticErrorMessage(cause); + if (causeMessage && causeMessage !== "Unknown error") { + sections.push(`caused by: ${causeMessage}`); + } + } + return sections.length > 0 ? sections.join("\n") : "Unknown error"; +} + +function formatNonErrorDiagnostic(error: unknown): string { + try { + const serialized = JSON.stringify(error); + if (serialized && serialized !== "{}" && serialized !== '""') { + return serialized; + } + } catch { + // fall through to String() below + } + + const stringified = String(error); + if (stringified.length > 0 && stringified !== "[object Object]") { + return stringified; } return "Unknown error"; } +export function toDiagnosticErrorMessage(error: unknown): string { + if (error instanceof Error) { + return formatErrorDiagnostic(error); + } + if (typeof error === "string") { + const trimmed = error.trim(); + return trimmed.length > 0 ? trimmed : "Unknown error"; + } + if (error === null || error === undefined) { + return "Unknown error"; + } + return formatNonErrorDiagnostic(error); +} + export async function resolveBinaryVersion(binaryPath: string): Promise { try { const { stdout } = await execCommand(binaryPath, ["--version"], { @@ -50,8 +131,8 @@ export async function resolveBinaryVersion(binaryPath: string): Promise timeout: 5_000, }); return stdout.trim() || "unknown"; - } catch { - return "unknown"; + } catch (error) { + return `error: ${toDiagnosticErrorMessage(error)}`; } } diff --git a/packages/server/src/server/agent/providers/opencode-agent.ts b/packages/server/src/server/agent/providers/opencode-agent.ts index a9e080b5e..61c713953 100644 --- a/packages/server/src/server/agent/providers/opencode-agent.ts +++ b/packages/server/src/server/agent/providers/opencode-agent.ts @@ -225,22 +225,6 @@ function toOpenCodeMcpConfig(config: McpServerConfig): OpenCodeMcpConfig { }; } -function stringifyUnknownError(error: unknown): string { - if (typeof error === "string") { - return error; - } - try { - return JSON.stringify(error); - } catch { - return String(error); - } -} - -function normalizeTurnFailureError(error: unknown): string { - const normalized = stringifyUnknownError(error).trim(); - return normalized.length > 0 ? normalized : "Unknown error"; -} - type TerminalTurnEvent = Extract< AgentStreamEvent, { type: "turn_completed" | "turn_failed" | "turn_canceled" } @@ -251,7 +235,7 @@ function toTerminalTurnEvent(event: AgentStreamEvent): TerminalTurnEvent | null return { type: "turn_failed", provider: "opencode", - error: normalizeTurnFailureError(event.error), + error: toDiagnosticErrorMessage(event.error), }; } if (event.type === "turn_completed" || event.type === "turn_canceled") { @@ -286,7 +270,7 @@ async function reconcileOpenCodeSessionClose(params: { logger.warn( { sessionId, - error: normalizeTurnFailureError(response.error), + error: toDiagnosticErrorMessage(response.error), }, "Failed to abort OpenCode session during close", ); @@ -295,7 +279,7 @@ async function reconcileOpenCodeSessionClose(params: { logger.warn( { sessionId, - error: normalizeTurnFailureError(error), + error: toDiagnosticErrorMessage(error), }, "Failed to abort OpenCode session during close", ); @@ -311,7 +295,7 @@ async function reconcileOpenCodeSessionClose(params: { logger.warn( { sessionId, - error: normalizeTurnFailureError(response.error), + error: toDiagnosticErrorMessage(response.error), }, "Failed to archive OpenCode session during close", ); @@ -320,7 +304,7 @@ async function reconcileOpenCodeSessionClose(params: { logger.warn( { sessionId, - error: normalizeTurnFailureError(error), + error: toDiagnosticErrorMessage(error), }, "Failed to archive OpenCode session during close", ); @@ -345,7 +329,7 @@ function isOpenCodeHeadersTimeoutFailure(error: unknown): boolean { continue; } - const normalized = stringifyUnknownError(current).trim().toLowerCase(); + const normalized = toDiagnosticErrorMessage(current).trim().toLowerCase(); if (normalized) { diagnostics.add(normalized); } @@ -380,7 +364,7 @@ function isOpenCodeHeadersTimeoutFailure(error: unknown): boolean { } function isAlreadyPresentMcpError(error: unknown): boolean { - const normalized = stringifyUnknownError(error).toLowerCase(); + const normalized = toDiagnosticErrorMessage(error).toLowerCase(); return MCP_ALREADY_PRESENT_ERROR_TOKENS.some((token) => normalized.includes(token)); } @@ -873,14 +857,37 @@ export class OpenCodeServerManager { ); let started = false; + let stderrBuffer = ""; + let stdoutBuffer = ""; + const STARTUP_BUFFER_CAP = 8192; + const appendCapped = (current: string, chunk: string): string => { + if (current.length >= STARTUP_BUFFER_CAP) { + return current; + } + const remaining = STARTUP_BUFFER_CAP - current.length; + return current + chunk.slice(0, remaining); + }; + const buildStartupErrorMessage = (headline: string): string => { + const sections = [headline]; + const stderrTrimmed = stderrBuffer.trim(); + if (stderrTrimmed.length > 0) { + sections.push(`stderr: ${stderrTrimmed}`); + } + const stdoutTrimmed = stdoutBuffer.trim(); + if (stdoutTrimmed.length > 0) { + sections.push(`stdout: ${stdoutTrimmed}`); + } + return sections.join("\n"); + }; const timeout = setTimeout(() => { if (!started) { - reject(new Error("OpenCode server startup timeout")); + reject(new Error(buildStartupErrorMessage("OpenCode server startup timeout"))); } }, 30_000); serverProcess.stdout?.on("data", (data: Buffer) => { const output = data.toString(); + stdoutBuffer = appendCapped(stdoutBuffer, output); if (output.includes("listening on") && !started) { started = true; clearTimeout(timeout); @@ -895,18 +902,21 @@ export class OpenCodeServerManager { }); serverProcess.stderr?.on("data", (data: Buffer) => { - this.logger.error({ stderr: data.toString().trim() }, "OpenCode server stderr"); + const output = data.toString(); + stderrBuffer = appendCapped(stderrBuffer, output); + this.logger.error({ stderr: output.trim() }, "OpenCode server stderr"); }); serverProcess.on("error", (error) => { clearTimeout(timeout); - reject(error); + const headline = error instanceof Error ? error.message : String(error); + reject(new Error(buildStartupErrorMessage(headline))); }); serverProcess.on("exit", (code) => { if (!started) { clearTimeout(timeout); - reject(new Error(`OpenCode server exited with code ${code}`)); + reject(new Error(buildStartupErrorMessage(`OpenCode server exited with code ${code}`))); } if (this.currentServer?.process === serverProcess) { this.currentServer = null; @@ -1195,7 +1205,7 @@ export class OpenCodeAgentClient implements AgentClient { const { url } = await this.serverManager.ensureRunning(); serverStatus = `Running (${url})`; } catch (error) { - serverStatus = `Unavailable (${normalizeTurnFailureError(error)})`; + serverStatus = `Unavailable (${toDiagnosticErrorMessage(error)})`; } if (available) { @@ -1504,7 +1514,7 @@ export function translateOpenCodeEvent( events.push({ type: "turn_failed", provider: "opencode", - error: normalizeTurnFailureError(event.properties.error), + error: toDiagnosticErrorMessage(event.properties.error), }); } break; @@ -1821,7 +1831,7 @@ function appendOpenCodeSessionStatus( events.push({ type: "turn_failed", provider: "opencode", - error: normalizeTurnFailureError(status.message), + error: toDiagnosticErrorMessage(status.message), }); } // "retry" and "busy" are transient — no terminal event. @@ -2054,7 +2064,7 @@ class OpenCodeAgentSession implements AgentSession { ); return; } - const errorMsg = normalizeTurnFailureError(response.error); + const errorMsg = toDiagnosticErrorMessage(response.error); this.finishForegroundTurn( { type: "turn_failed", provider: "opencode", error: errorMsg }, turnId, @@ -2080,7 +2090,7 @@ class OpenCodeAgentSession implements AgentSession { return; } this.finishForegroundTurn( - { type: "turn_failed", provider: "opencode", error: normalizeTurnFailureError(err) }, + { type: "turn_failed", provider: "opencode", error: toDiagnosticErrorMessage(err) }, turnId, ); }); @@ -2109,7 +2119,7 @@ class OpenCodeAgentSession implements AgentSession { { type: "turn_failed", provider: "opencode", - error: normalizeTurnFailureError(promptResponse.error), + error: toDiagnosticErrorMessage(promptResponse.error), }, turnId, ); @@ -2121,7 +2131,7 @@ class OpenCodeAgentSession implements AgentSession { { type: "turn_failed", provider: "opencode", - error: normalizeTurnFailureError(error), + error: toDiagnosticErrorMessage(error), }, turnId, ); @@ -2186,7 +2196,7 @@ class OpenCodeAgentSession implements AgentSession { { type: "turn_failed", provider: "opencode", - error: normalizeTurnFailureError(error), + error: toDiagnosticErrorMessage(error), }, turnId, ); @@ -2585,7 +2595,7 @@ class OpenCodeAgentSession implements AgentSession { } throw new Error( - `Failed to ${operation} OpenCode MCP server '${name}': ${stringifyUnknownError(error)}`, + `Failed to ${operation} OpenCode MCP server '${name}': ${toDiagnosticErrorMessage(error)}`, ); } diff --git a/packages/server/src/server/agent/providers/pi-direct-agent.ts b/packages/server/src/server/agent/providers/pi-direct-agent.ts index 754f39686..8098924e9 100644 --- a/packages/server/src/server/agent/providers/pi-direct-agent.ts +++ b/packages/server/src/server/agent/providers/pi-direct-agent.ts @@ -624,13 +624,6 @@ function mapToolDetail(toolCall: PiTrackedToolCall, result?: PiToolResult): Tool } } -function stringifyUnknownError(error: unknown): string { - if (error instanceof Error) { - return error.message; - } - return typeof error === "string" ? error : "Unknown Pi error"; -} - function parseModelReference(modelId: string | null): PiModelReference | null { if (!modelId) { return null; @@ -756,7 +749,7 @@ function isPiRequestAbortError(error: unknown): boolean { return true; } - return /\brequest was aborted\b/i.test(stringifyUnknownError(error)); + return /\brequest was aborted\b/i.test(toDiagnosticErrorMessage(error)); } function resolveThinkingOptionId( @@ -1113,7 +1106,7 @@ export class PiDirectAgentSession implements AgentSession { type: "turn_canceled", provider: PI_PROVIDER, turnId: failedTurnId, - reason: stringifyUnknownError(error), + reason: toDiagnosticErrorMessage(error), }); return; } @@ -1121,7 +1114,7 @@ export class PiDirectAgentSession implements AgentSession { type: "turn_failed", provider: PI_PROVIDER, turnId: failedTurnId, - error: stringifyUnknownError(error), + error: toDiagnosticErrorMessage(error), }); });