From b41c0b674654083fb3cc4696df032bb64d876e97 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 16 Jun 2026 01:51:41 +0200 Subject: [PATCH] fix(cli): preserve gateway request errors in json mode --- scripts/e2e/kitchen-sink-rpc-walk.mjs | 92 +++++++--- src/cli/gateway-cli.coverage.test.ts | 33 ++++ src/cli/gateway-cli/register.ts | 193 +++++++++++---------- src/gateway/call.test.ts | 42 +++++ src/gateway/call.ts | 51 ++++++ test/scripts/kitchen-sink-rpc-walk.test.ts | 59 ++++++- 6 files changed, 356 insertions(+), 114 deletions(-) diff --git a/scripts/e2e/kitchen-sink-rpc-walk.mjs b/scripts/e2e/kitchen-sink-rpc-walk.mjs index 9332f211f19..35130cc698a 100644 --- a/scripts/e2e/kitchen-sink-rpc-walk.mjs +++ b/scripts/e2e/kitchen-sink-rpc-walk.mjs @@ -365,8 +365,16 @@ export function runCommand(command, args, options = {}) { ? `timed out after ${timeoutMs}ms` : `failed with ${signal || status}`; reject( - new Error( - `${command} ${args.join(" ")} ${failure}${detail ? `\n${tailText(detail)}` : ""}`, + Object.assign( + new Error( + `${command} ${args.join(" ")} ${failure}${detail ? `\n${tailText(detail)}` : ""}`, + ), + { + signal, + status, + stderr: stderr.text, + stdout: stdout.text, + }, ), ); }); @@ -443,6 +451,38 @@ function parseJsonOutput(stdout) { throw new Error(`JSON output was not parseable:\n${tailText(trimmed)}`); } +export function parseGatewayCliRequestFailure(error) { + if (typeof error?.stdout !== "string" || !error.stdout.trim()) { + return null; + } + let payload; + try { + payload = parseJsonOutput(error.stdout); + } catch { + return null; + } + const requestError = payload?.ok === false ? payload.error : null; + if ( + requestError?.type !== "gateway_request_error" || + !isNonEmptyString(requestError.code) || + !isNonEmptyString(requestError.message) || + typeof requestError.retryable !== "boolean" || + (requestError.retryAfterMs !== undefined && + (typeof requestError.retryAfterMs !== "number" || + !Number.isInteger(requestError.retryAfterMs) || + requestError.retryAfterMs < 0)) + ) { + return null; + } + return Object.assign(new Error(requestError.message), { + name: "GatewayClientRequestError", + gatewayCode: requestError.code, + ...(requestError.details !== undefined ? { details: requestError.details } : {}), + retryable: requestError.retryable, + ...(requestError.retryAfterMs !== undefined ? { retryAfterMs: requestError.retryAfterMs } : {}), + }); +} + function boundedJsonPreview(value, space) { try { return JSON.stringify(previewJsonValue(value), null, space) ?? String(value); @@ -632,25 +672,30 @@ async function importCallGatewayModule() { async function rpcCallViaCli(method, params, options) { const config = resolveKitchenSinkRpcConfig(options.env); - const { stdout } = await runOpenClaw( - options.runner, - [ - "gateway", - "call", - method, - "--url", - `ws://127.0.0.1:${options.port}`, - "--token", - TOKEN, - "--timeout", - String(config.rpcTimeoutMs), - "--json", - "--params", - JSON.stringify(params ?? {}), - ], - options.env, - createRpcCliRunOptions(method, options), - ); + let stdout; + try { + ({ stdout } = await runOpenClaw( + options.runner, + [ + "gateway", + "call", + method, + "--url", + `ws://127.0.0.1:${options.port}`, + "--token", + TOKEN, + "--timeout", + String(config.rpcTimeoutMs), + "--json", + "--params", + JSON.stringify(params ?? {}), + ], + options.env, + createRpcCliRunOptions(method, options), + )); + } catch (error) { + throw parseGatewayCliRequestFailure(error) ?? error; + } return parseJsonOutput(stdout); } @@ -1399,10 +1444,7 @@ export async function assertOperatorRpcDenied(probe, call) { } catch (error) { const gatewayCode = error?.gatewayCode; const message = String(error?.message ?? ""); - if ( - (gatewayCode === undefined || gatewayCode === "INVALID_REQUEST") && - message.includes("unauthorized role: operator") - ) { + if (gatewayCode === "INVALID_REQUEST" && message.includes("unauthorized role: operator")) { return; } throw error; diff --git a/src/cli/gateway-cli.coverage.test.ts b/src/cli/gateway-cli.coverage.test.ts index c48e380ad39..139d6bd021a 100644 --- a/src/cli/gateway-cli.coverage.test.ts +++ b/src/cli/gateway-cli.coverage.test.ts @@ -13,6 +13,7 @@ type DiscoveredBeacon = Awaited< >[number]; const callGateway = vi.fn<(opts: unknown) => Promise<{ ok: true }>>(async () => ({ ok: true })); +const formatGatewayClientRequestErrorJson = vi.fn(); const formatGatewayTransportErrorJson = vi.fn(); const startGatewayServer = vi.fn< (port: number, opts?: unknown) => Promise<{ close: () => Promise }> @@ -55,6 +56,8 @@ vi.mock( url: "ws://127.0.0.1:18789", }), callGateway: (opts: unknown) => callGateway(opts), + formatGatewayClientRequestErrorJson: (error: unknown) => + formatGatewayClientRequestErrorJson(error), formatGatewayTransportErrorJson: (error: unknown) => formatGatewayTransportErrorJson(error), isGatewayCredentialsRequiredError: () => false, randomIdempotencyKey: () => "rk_test", @@ -156,6 +159,8 @@ describe("gateway-cli coverage", () => { startGatewayServer.mockClear(); inspectPortUsage.mockClear(); formatPortDiagnostics.mockClear(); + formatGatewayClientRequestErrorJson.mockReset(); + formatGatewayClientRequestErrorJson.mockReturnValue(null); formatGatewayTransportErrorJson.mockReset(); formatGatewayTransportErrorJson.mockReturnValue(null); }); @@ -232,6 +237,34 @@ describe("gateway-cli coverage", () => { expect(runtimeErrors.join("\n")).not.toContain("gateway closed"); }); + it.each([ + ["call", ["gateway", "call", "skills.bins", "--json"]], + ["usage cost", ["gateway", "usage-cost", "--json"]], + ["stability", ["gateway", "stability", "--json"]], + ])("writes JSON for gateway %s request failures in JSON mode", async (_label, args) => { + const error = Object.assign(new Error("unauthorized role: operator"), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + }); + const payload = { + ok: false, + error: { + type: "gateway_request_error", + code: "INVALID_REQUEST", + message: "unauthorized role: operator", + retryable: false, + }, + }; + callGateway.mockRejectedValueOnce(error); + formatGatewayClientRequestErrorJson.mockReturnValueOnce(payload); + + await expectGatewayExit(args); + + expect(formatGatewayClientRequestErrorJson).toHaveBeenCalledWith(error); + expect(defaultRuntime.writeJson).toHaveBeenCalledWith(payload); + expect(runtimeErrors.join("\n")).not.toContain("unauthorized role"); + }); + it("prints the latest stability bundle without calling Gateway", async () => { callGateway.mockClear(); const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-gateway-cli-bundle-")); diff --git a/src/cli/gateway-cli/register.ts b/src/cli/gateway-cli/register.ts index 7ccbfda3ba1..082919ef370 100644 --- a/src/cli/gateway-cli/register.ts +++ b/src/cli/gateway-cli/register.ts @@ -108,13 +108,15 @@ async function runGatewayCommand( label?: string, opts?: { json?: boolean }, ) { - // JSON mode preserves structured gateway transport errors for automation callers. + // JSON mode preserves structured gateway errors for automation callers. try { await action(); } catch (err) { if (opts?.json) { - const { formatGatewayTransportErrorJson } = await import("../../gateway/call.js"); - const payload = formatGatewayTransportErrorJson(err); + const { formatGatewayClientRequestErrorJson, formatGatewayTransportErrorJson } = + await import("../../gateway/call.js"); + const payload = + formatGatewayClientRequestErrorJson(err) ?? formatGatewayTransportErrorJson(err); if (payload) { defaultRuntime.writeJson(payload); defaultRuntime.exit(1); @@ -499,20 +501,24 @@ export function registerGatewayCli(program: Command) { .argument("", "Method name (health/status/system-presence/cron.*)") .option("--params ", "JSON object string for params", "{}") .action(async (method, opts, command) => { - await runGatewayCommand(async () => { - const rpcOpts = resolveGatewayRpcOptions(opts, command); - const params = JSON.parse(String(opts.params ?? "{}")); - const result = await callGatewayCli(method, rpcOpts, params); - if (rpcOpts.json) { + await runGatewayCommand( + async () => { + const rpcOpts = resolveGatewayRpcOptions(opts, command); + const params = JSON.parse(String(opts.params ?? "{}")); + const result = await callGatewayCli(method, rpcOpts, params); + if (rpcOpts.json) { + defaultRuntime.writeJson(result); + return; + } + const rich = isRich(); + defaultRuntime.log( + `${colorize(rich, theme.heading, "Gateway call")}: ${colorize(rich, theme.muted, String(method))}`, + ); defaultRuntime.writeJson(result); - return; - } - const rich = isRich(); - defaultRuntime.log( - `${colorize(rich, theme.heading, "Gateway call")}: ${colorize(rich, theme.muted, String(method))}`, - ); - defaultRuntime.writeJson(result); - }, "Gateway call failed"); + }, + "Gateway call failed", + { json: Boolean(opts.json) }, + ); }), ); @@ -522,20 +528,24 @@ export function registerGatewayCli(program: Command) { .description("Fetch usage cost summary from session logs") .option("--days ", "Number of days to include", "30") .action(async (opts, command) => { - await runGatewayCommand(async () => { - const rpcOpts = resolveGatewayRpcOptions(opts, command); - const days = parseDaysOption(opts.days); - const result = await callGatewayCli("usage.cost", rpcOpts, { days }); - if (rpcOpts.json) { - defaultRuntime.writeJson(result); - return; - } - const rich = isRich(); - const summary = result as CostUsageSummary; - for (const line of await renderCostUsageSummaryAsync(summary, days, rich)) { - defaultRuntime.log(line); - } - }, "Gateway usage cost failed"); + await runGatewayCommand( + async () => { + const rpcOpts = resolveGatewayRpcOptions(opts, command); + const days = parseDaysOption(opts.days); + const result = await callGatewayCli("usage.cost", rpcOpts, { days }); + if (rpcOpts.json) { + defaultRuntime.writeJson(result); + return; + } + const rich = isRich(); + const summary = result as CostUsageSummary; + for (const line of await renderCostUsageSummaryAsync(summary, days, rich)) { + defaultRuntime.log(line); + } + }, + "Gateway usage cost failed", + { json: Boolean(opts.json) }, + ); }), ); @@ -608,71 +618,78 @@ export function registerGatewayCli(program: Command) { .option("--export", "Write a shareable support diagnostics export", false) .option("--output ", "Diagnostics export output .zip path") .action(async (opts, command) => { - await runGatewayCommand(async () => { - const { normalizeDiagnosticStabilityQuery, selectDiagnosticStabilitySnapshot } = - await import("../../logging/diagnostic-stability.js"); - const rpcOpts = resolveGatewayRpcOptions(opts, command); - const query = normalizeDiagnosticStabilityQuery( - { - limit: opts.limit, - sinceSeq: opts.sinceSeq, - type: opts.type, - }, - { defaultLimit: 25 }, - ); - const bundleTarget = normalizeStabilityBundleTarget(opts.bundle); - if (opts.export) { - await writeSupportExportFromCli({ - json: rpcOpts.json, - output: opts.output, - stabilityBundle: bundleTarget ?? "latest", - rpc: rpcOpts, - }); - return; - } - if (bundleTarget) { - const result = await readStabilityBundleTarget(bundleTarget); - if (result.status !== "found") { - throw new Error(formatBundleError(result)); - } - const snapshot = selectDiagnosticStabilitySnapshot(result.bundle.snapshot, query); - if (rpcOpts.json) { - defaultRuntime.writeJson({ - path: result.path, - mtimeMs: result.mtimeMs, - bundle: { - ...result.bundle, - snapshot, - }, + await runGatewayCommand( + async () => { + const { normalizeDiagnosticStabilityQuery, selectDiagnosticStabilitySnapshot } = + await import("../../logging/diagnostic-stability.js"); + const rpcOpts = resolveGatewayRpcOptions(opts, command); + const query = normalizeDiagnosticStabilityQuery( + { + limit: opts.limit, + sinceSeq: opts.sinceSeq, + type: opts.type, + }, + { defaultLimit: 25 }, + ); + const bundleTarget = normalizeStabilityBundleTarget(opts.bundle); + if (opts.export) { + await writeSupportExportFromCli({ + json: rpcOpts.json, + output: opts.output, + stabilityBundle: bundleTarget ?? "latest", + rpc: rpcOpts, }); return; } + if (bundleTarget) { + const result = await readStabilityBundleTarget(bundleTarget); + if (result.status !== "found") { + throw new Error(formatBundleError(result)); + } + const snapshot = selectDiagnosticStabilitySnapshot(result.bundle.snapshot, query); + if (rpcOpts.json) { + defaultRuntime.writeJson({ + path: result.path, + mtimeMs: result.mtimeMs, + bundle: { + ...result.bundle, + snapshot, + }, + }); + return; + } + const rich = isRich(); + for (const line of renderStabilityBundleSummary({ + bundle: result.bundle, + path: result.path, + rich, + snapshot, + })) { + defaultRuntime.log(line); + } + return; + } + + const result = await callGatewayCli("diagnostics.stability", rpcOpts, { + limit: query.limit, + ...(query.type ? { type: query.type } : {}), + ...(query.sinceSeq !== undefined ? { sinceSeq: query.sinceSeq } : {}), + }); + if (rpcOpts.json) { + defaultRuntime.writeJson(result); + return; + } const rich = isRich(); - for (const line of renderStabilityBundleSummary({ - bundle: result.bundle, - path: result.path, + for (const line of renderStabilitySummary( + result as DiagnosticStabilitySnapshot, rich, - snapshot, - })) { + )) { defaultRuntime.log(line); } - return; - } - - const result = await callGatewayCli("diagnostics.stability", rpcOpts, { - limit: query.limit, - ...(query.type ? { type: query.type } : {}), - ...(query.sinceSeq !== undefined ? { sinceSeq: query.sinceSeq } : {}), - }); - if (rpcOpts.json) { - defaultRuntime.writeJson(result); - return; - } - const rich = isRich(); - for (const line of renderStabilitySummary(result as DiagnosticStabilitySnapshot, rich)) { - defaultRuntime.log(line); - } - }, "Gateway stability failed"); + }, + "Gateway stability failed", + { json: Boolean(opts.json) }, + ); }), ); diff --git a/src/gateway/call.test.ts b/src/gateway/call.test.ts index d650009a37b..ea82316909b 100644 --- a/src/gateway/call.test.ts +++ b/src/gateway/call.test.ts @@ -175,6 +175,7 @@ const { callGateway, callGatewayCli, callGatewayScoped, + formatGatewayClientRequestErrorJson, formatGatewayTransportErrorJson, isGatewayTransportError, } = await import("./call.js"); @@ -1466,6 +1467,47 @@ describe("callGateway error details", () => { }); }); + it("formats typed request errors for CLI JSON output", () => { + const error = Object.assign(new Error("unauthorized role: operator"), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + details: { method: "skills.bins" }, + retryable: false, + retryAfterMs: 250, + }); + + expect(formatGatewayClientRequestErrorJson(error)).toEqual({ + ok: false, + error: { + type: "gateway_request_error", + code: "INVALID_REQUEST", + message: "unauthorized role: operator", + details: { method: "skills.bins" }, + retryable: false, + retryAfterMs: 250, + }, + }); + expect( + formatGatewayClientRequestErrorJson( + Object.assign(new Error("unauthorized role: operator"), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + retryable: "no", + }), + ), + ).toBeNull(); + expect( + formatGatewayClientRequestErrorJson( + Object.assign(new Error("unauthorized role: operator"), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + retryable: false, + retryAfterMs: -1, + }), + ), + ).toBeNull(); + }); + it("charges event-loop readiness against the wrapper timeout", async () => { startMode = "silent"; setLocalLoopbackGatewayConfig(); diff --git a/src/gateway/call.ts b/src/gateway/call.ts index 46114d5451f..7a504d454b2 100644 --- a/src/gateway/call.ts +++ b/src/gateway/call.ts @@ -201,6 +201,18 @@ export type GatewayTransportErrorJson = { }; }; +export type GatewayClientRequestErrorJson = { + ok: false; + error: { + type: "gateway_request_error"; + code: string; + message: string; + details?: unknown; + retryable: boolean; + retryAfterMs?: number; + }; +}; + export type GatewayProbeConnectionDetails = GatewayConnectionDetails & { tlsFingerprint?: string; preauthHandshakeTimeoutMs?: number; @@ -237,6 +249,45 @@ export function formatGatewayTransportErrorJson(value: unknown): GatewayTranspor }; } +export function formatGatewayClientRequestErrorJson( + value: unknown, +): GatewayClientRequestErrorJson | null { + if (!(value instanceof Error) || value.name !== "GatewayClientRequestError") { + return null; + } + const requestError = value as Error & { + gatewayCode?: unknown; + details?: unknown; + retryable?: unknown; + retryAfterMs?: unknown; + }; + if ( + typeof requestError.gatewayCode !== "string" || + requestError.gatewayCode.length === 0 || + requestError.message.length === 0 || + typeof requestError.retryable !== "boolean" || + (requestError.retryAfterMs !== undefined && + (typeof requestError.retryAfterMs !== "number" || + !Number.isInteger(requestError.retryAfterMs) || + requestError.retryAfterMs < 0)) + ) { + return null; + } + return { + ok: false, + error: { + type: "gateway_request_error", + code: requestError.gatewayCode, + message: requestError.message, + ...(requestError.details !== undefined ? { details: requestError.details } : {}), + retryable: requestError.retryable, + ...(requestError.retryAfterMs !== undefined + ? { retryAfterMs: requestError.retryAfterMs } + : {}), + }, + }; +} + export function isGatewayTransportError(value: unknown): value is GatewayTransportError { if (value instanceof GatewayTransportError) { return true; diff --git a/test/scripts/kitchen-sink-rpc-walk.test.ts b/test/scripts/kitchen-sink-rpc-walk.test.ts index 548140a657d..a160fed00de 100644 --- a/test/scripts/kitchen-sink-rpc-walk.test.ts +++ b/test/scripts/kitchen-sink-rpc-walk.test.ts @@ -42,6 +42,7 @@ import { listKitchenSinkAuthorizationRpcProbeNames, listKitchenSinkReadOnlyRpcProbeNames, makeEnv, + parseGatewayCliRequestFailure, readPositiveInt, readBoundedResponseText, runCommand, @@ -638,6 +639,20 @@ setInterval(() => {}, 1000); code: "ENOENT", }); }); + + it("preserves failed command output for structured consumers", async () => { + await expect( + runCommand(process.execPath, [ + "-e", + 'process.stdout.write("request failure"); process.stderr.write("diagnostic"); process.exit(7)', + ]), + ).rejects.toMatchObject({ + status: 7, + signal: null, + stdout: "request failure", + stderr: "diagnostic", + }); + }); }); describe("kitchen-sink RPC caller loading", () => { @@ -844,12 +859,54 @@ describe("kitchen-sink RPC command catalog assertions", () => { "openclaw gateway call skills.bins failed with 1\nGateway call failed: unauthorized role: operator", ); }), - ).resolves.toBeUndefined(); + ).rejects.toThrow("Gateway call failed: unauthorized role: operator"); await expect( assertOperatorRpcDenied({ method: "skills.bins", params: {} }, async () => ({})), ).rejects.toThrow("skills.bins unexpectedly allowed operator access"); }); + it("reconstructs typed request failures from gateway CLI JSON", async () => { + const { formatGatewayClientRequestErrorJson } = await import("../../src/gateway/call.js"); + const payload = formatGatewayClientRequestErrorJson( + Object.assign(new Error("unauthorized role: operator"), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + details: { method: "skills.bins" }, + retryable: false, + retryAfterMs: 250, + }), + ); + + expect( + parseGatewayCliRequestFailure({ + stdout: JSON.stringify(payload), + }), + ).toMatchObject({ + name: "GatewayClientRequestError", + message: "unauthorized role: operator", + gatewayCode: "INVALID_REQUEST", + details: { method: "skills.bins" }, + retryable: false, + retryAfterMs: 250, + }); + expect(parseGatewayCliRequestFailure(new Error("plain failure"))).toBeNull(); + for (const invalidFields of [{ retryable: "no" }, { retryable: false, retryAfterMs: -1 }]) { + expect( + parseGatewayCliRequestFailure({ + stdout: JSON.stringify({ + ok: false, + error: { + type: "gateway_request_error", + code: "INVALID_REQUEST", + message: "unauthorized role: operator", + ...invalidFields, + }, + }), + }), + ).toBeNull(); + } + }); + it("requires provenance for effective Kitchen Sink plugin tools too", () => { expect(() => assertExpectedKitchenSinkToolEntries(