mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-08 17:03:58 +00:00
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 <vincentkoc@ieee.org>
This commit is contained in:
@@ -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");
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string, unknown> {
|
||||
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();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user