From dff45cae4cb20e2d2d16bd0789582ad373da119b Mon Sep 17 00:00:00 2001 From: Wynne668 Date: Wed, 1 Jul 2026 18:07:09 +0800 Subject: [PATCH] fix(code-mode): surface QuickJS error name and message to the model (#95906) * fix(code-mode): surface QuickJS error name and message to the model * fix(code-mode): preserve host error diagnostics --------- Co-authored-by: Vincent Koc --- src/agents/code-mode.test.ts | 82 +++++++++++++++++++++++++++++++--- src/agents/code-mode.worker.ts | 30 ++++++++++++- 2 files changed, 105 insertions(+), 7 deletions(-) diff --git a/src/agents/code-mode.test.ts b/src/agents/code-mode.test.ts index bc484be31982..ec207ab2ff13 100644 --- a/src/agents/code-mode.test.ts +++ b/src/agents/code-mode.test.ts @@ -1639,6 +1639,76 @@ describe("Code Mode", () => { expect(testing.activeRuns.size).toBe(beforeRunCount); }); + it("surfaces the QuickJS error name and message for guest syntax errors", async () => { + const { config, catalogRef, tools: codeModeTools } = createCodeModeHarness(); + applyCodeModeCatalog({ + tools: [...codeModeTools, pluginTool("fake_noop", "Noop")], + config, + sessionId: "session-code-mode", + sessionKey: "agent:main:main", + runId: "run-code-mode", + catalogRef, + }); + + const details = resultDetails( + await codeModeTools[0].execute("code-call-syntax", { code: "const x = ;" }), + ); + + expect(details.status).toBe("failed"); + const error = String(details.error); + // Regression guard: QuickJS stacks are frames only, so the error used to + // collapse to a bare "at openclaw-code-mode:user.js:..." location with the + // actual cause dropped. The model now sees the name and message. + expect(error).toContain("SyntaxError"); + expect(error).toContain("unexpected token"); + expect(error.startsWith("at ")).toBe(false); + }); + + it("surfaces the QuickJS error name and message for guest runtime errors", async () => { + const { config, catalogRef, tools: codeModeTools } = createCodeModeHarness(); + applyCodeModeCatalog({ + tools: [...codeModeTools, pluginTool("fake_noop", "Noop")], + config, + sessionId: "session-code-mode", + sessionKey: "agent:main:main", + runId: "run-code-mode", + catalogRef, + }); + + const details = resultDetails( + await codeModeTools[0].execute("code-call-runtime", { code: "return missingFn();" }), + ); + + expect(details.status).toBe("failed"); + const error = String(details.error); + expect(error).toContain("ReferenceError"); + expect(error).toContain("missingFn is not defined"); + expect(error.startsWith("at ")).toBe(false); + }); + + it("does not duplicate host error headers or expose host stack frames", async () => { + const { config, catalogRef, tools: codeModeTools } = createCodeModeHarness(); + applyCodeModeCatalog({ + tools: [...codeModeTools, pluginTool("fake_noop", "Noop")], + config, + sessionId: "session-code-mode", + sessionKey: "agent:main:main", + runId: "run-code-mode", + catalogRef, + }); + + const details = resultDetails( + await codeModeTools[0].execute("code-call-host-error", { + code: 'return globalThis.__openclawHostRequest("unsupported", "[]");', + }), + ); + + expect(details).toMatchObject({ + status: "failed", + error: "Error: unsupported code mode bridge method", + }); + }); + it("clamps omitted code-mode catalog search limits to maxSearchLimit", async () => { const catalogRef = createToolSearchCatalogRef(); const config = { @@ -1896,7 +1966,7 @@ describe("Code Mode", () => { ); expect(details.status).toBe("failed"); - expect(details.error).toBe("boom"); + expect(String(details.error)).toContain("Error: boom"); expect(details.output).toEqual([{ type: "text", text: "before" }]); }); @@ -2045,9 +2115,11 @@ describe("Code Mode", () => { ); expect(result.status).toBe("failed"); - expect(result).toMatchObject({ - code: "internal_error", - error: "interrupted", - }); + // A guest error whose message happens to be "interrupted" must stay + // internal_error and not be misclassified as a QuickJS interrupt/timeout. + expect(result).toMatchObject({ code: "internal_error" }); + if (result.status === "failed") { + expect(result.error).toContain("interrupted"); + } }); }); diff --git a/src/agents/code-mode.worker.ts b/src/agents/code-mode.worker.ts index 9502bd37af08..c8d872390db4 100644 --- a/src/agents/code-mode.worker.ts +++ b/src/agents/code-mode.worker.ts @@ -131,6 +131,11 @@ function isQuickJsInterruptedError(error: unknown): boolean { if (error instanceof CodeModeGuestError) { return false; } + // Match on the raw QuickJS message, not the formatted errorMessage() string, + // which now leads with the error name and appends backtrace frames. + if (error instanceof JSException) { + return error.message === "interrupted"; + } return errorMessage(error) === "interrupted"; } @@ -150,9 +155,22 @@ function isRecord(value: unknown): value is Record { return Boolean(value && typeof value === "object" && !Array.isArray(value)); } +// QuickJS error stacks are backtrace frames only (" at file:line:col"), with +// no leading "Name: message" header like V8. Returning .stack alone therefore +// dropped the actual cause, surfacing failures to the model as a bare location +// (e.g. "at openclaw-code-mode:user.js:2:37"). Lead with name+message so the +// model can self-correct, and keep the frames for location. +function formatQuickJsError(name: string, message: string, stack: string | undefined): string { + const header = message ? `${name}: ${message}` : name; + if (!stack || stack.split(/\r?\n/, 1)[0] === header) { + return header; + } + return `${header}\n${stack}`; +} + function errorMessage(error: unknown): string { if (error instanceof JSException) { - return error.stack || error.message || String(error); + return formatQuickJsError(error.name, error.message, error.stack); } if (error instanceof Error) { return error.message || String(error); @@ -575,7 +593,15 @@ async function readCompletedResult(vm: QuickJS, resultHandle: JSValueHandle): Pr const settled = await vm.resolvePromise(resultHandle); if ("error" in settled) { try { - throw new CodeModeGuestError(errorMessage(vm.dump(settled.error))); + // vm.dump rebuilds a host Error carrying the QuickJS name/message/stack; + // format it like the synchronous path so async rejections keep their cause + // and location instead of collapsing to the bare message. + const dumped = vm.dump(settled.error); + const text = + dumped instanceof Error + ? formatQuickJsError(dumped.name, dumped.message, dumped.stack) + : errorMessage(dumped); + throw new CodeModeGuestError(text); } finally { settled.error.dispose(); }