refactor(gateway): centralize connect failure classification (#120505)

* refactor(gateway): centralize connect failure classification

* fix(gateway): project connect failure into status JSON

* fix(protocol): restore remote-auth context in pairing remediation

* fix(gateway): classify generic probe failures by close reason

* fix(gateway): keep transport closes unreachable

* fix(gateway): preserve rate-limit diagnostics

* fix(gateway): preserve typed rate-limit failures
This commit is contained in:
Peter Steinberger
2026-08-08 18:34:01 -07:00
committed by GitHub
parent 75dbe52e3e
commit c8cd98cf52
22 changed files with 1128 additions and 61 deletions
+160
View File
@@ -1,6 +1,8 @@
// Daemon probe tests cover gateway probe command behavior and output.
import { describe, expect, it, vi } from "vitest";
import { gatewayProbeResultSawGateway } from "../../commands/gateway-health-auth-diagnostic.js";
import { probeGatewayStatus } from "./probe.js";
import type { DaemonStatus } from "./status.gather.js";
const callGatewayMock = vi.hoisted(() => vi.fn());
const probeGatewayMock = vi.hoisted(() => vi.fn());
@@ -17,6 +19,19 @@ vi.mock("../progress.js", () => ({
withProgress: async (_opts: unknown, fn: () => Promise<unknown>) => await fn(),
}));
function createDaemonStatus(rpc: NonNullable<DaemonStatus["rpc"]>): DaemonStatus {
return {
service: {
label: "test service",
loaded: true,
loadedText: "loaded",
notLoadedText: "not loaded",
},
rpc,
extraServices: [],
};
}
describe("probeGatewayStatus", () => {
const pairingPendingAuth = {
role: null,
@@ -39,6 +54,7 @@ describe("probeGatewayStatus", () => {
kind: "connect",
capability: "pairing_pending",
auth: pairingPendingAuth,
connectFailure: { kind: "pairing-required" },
error: "gateway closed (1008): pairing required",
});
}
@@ -85,6 +101,148 @@ describe("probeGatewayStatus", () => {
});
});
it("projects allowlisted connect failure details without serializing raw payloads", async () => {
probeGatewayMock.mockResolvedValueOnce({
ok: false,
error: "connect failed",
close: { code: 1008, reason: "connect failed" },
connectErrorDetails: {
code: "PAIRING_REQUIRED",
reason: "scope-upgrade",
secret: "do-not-print",
},
auth: pairingPendingAuth,
});
const result = await probeGatewayStatus({
url: "ws://127.0.0.1:19191",
timeoutMs: 5_000,
json: true,
});
expect(result.ok).toBe(false);
if (result.ok) {
throw new Error("expected failed gateway probe");
}
expect(result.connectFailure).toEqual({
kind: "pairing-required",
detailCode: "PAIRING_REQUIRED",
});
expect(result).not.toHaveProperty("connectErrorDetails");
expect(gatewayProbeResultSawGateway(result)).toBe(true);
const json = JSON.stringify(createDaemonStatus(result));
expect(json).not.toContain("do-not-print");
expect(json).not.toContain('"secret"');
expect(json).not.toContain("scope-upgrade");
});
it("classifies a legacy pairing close when the probe error is generic", async () => {
probeGatewayMock.mockResolvedValueOnce({
ok: false,
error: "connect failed",
close: { code: 1008, reason: "pairing required" },
});
const result = await probeGatewayStatus({
url: "ws://127.0.0.1:19191",
timeoutMs: 5_000,
json: true,
});
expect(result.ok).toBe(false);
if (result.ok) {
throw new Error("expected failed gateway probe");
}
expect(result.error).toBe("connect failed");
expect(result.connectFailure).toEqual({ kind: "pairing-required" });
expect(gatewayProbeResultSawGateway(result)).toBe(true);
});
it("does not classify an unvalidated transport close as a reachable gateway", async () => {
probeGatewayMock.mockResolvedValueOnce({
ok: false,
error: "connect ECONNREFUSED 127.0.0.1:19191",
close: { code: 1006, reason: "" },
});
const result = await probeGatewayStatus({
url: "ws://127.0.0.1:19191",
timeoutMs: 5_000,
json: true,
});
expect(result.ok).toBe(false);
if (result.ok) {
throw new Error("expected failed gateway probe");
}
expect(result.connectFailure).toEqual({ kind: "unreachable" });
expect(gatewayProbeResultSawGateway(result)).toBe(false);
});
it("projects authentication rate limits as reachable temporary lockouts", async () => {
probeGatewayMock.mockResolvedValueOnce({
ok: false,
error: "connect failed",
close: {
code: 1008,
reason: "unauthorized: too many failed authentication attempts (retry later)",
},
connectErrorDetails: {
code: "AUTH_RATE_LIMITED",
authReason: "rate_limited",
recommendedNextStep: "wait_then_retry",
retryAfterMs: 60_000,
},
});
const result = await probeGatewayStatus({
url: "ws://127.0.0.1:19191",
timeoutMs: 5_000,
json: true,
});
expect(result.ok).toBe(false);
if (result.ok) {
throw new Error("expected failed gateway probe");
}
expect(result.connectFailure).toEqual({
kind: "rate-limited",
detailCode: "AUTH_RATE_LIMITED",
});
expect(gatewayProbeResultSawGateway(result)).toBe(true);
expect(JSON.stringify(createDaemonStatus(result))).not.toContain("retryAfterMs");
});
it("omits unknown detail codes from serialized daemon status", async () => {
probeGatewayMock.mockResolvedValueOnce({
ok: false,
error: "connect failed",
close: { code: 1008, reason: "connect failed" },
connectErrorDetails: {
code: "FUTURE_SENSITIVE_CODE",
secret: "do-not-print-unknown",
},
auth: pairingPendingAuth,
});
const result = await probeGatewayStatus({
url: "ws://127.0.0.1:19191",
timeoutMs: 5_000,
json: true,
});
expect(result.ok).toBe(false);
if (result.ok) {
throw new Error("expected failed gateway probe");
}
expect(result.connectFailure).toEqual({ kind: "gateway-rejected" });
const json = JSON.stringify(createDaemonStatus(result));
expect(json).not.toContain("FUTURE_SENSITIVE_CODE");
expect(json).not.toContain("do-not-print-unknown");
});
it("preserves gateway server version from the connect probe", async () => {
callGatewayMock.mockReset();
probeGatewayMock.mockReset();
@@ -285,6 +443,7 @@ describe("probeGatewayStatus", () => {
expect(result).toEqual({
ok: false,
kind: "read",
connectFailure: { kind: "unreachable" },
error:
"gateway status RPC skipped because configured gateway credentials are disabled for this status request",
});
@@ -484,6 +643,7 @@ describe("probeGatewayStatus", () => {
expect(result).toEqual({
ok: false,
kind: "read",
connectFailure: { kind: "unreachable" },
error: "missing scope: operator.admin",
});
expect(probeGatewayMock).not.toHaveBeenCalled();
+33 -2
View File
@@ -1,5 +1,10 @@
// Gateway status probe helper used by `gateway status` service diagnostics.
import { redactSensitiveUrlLikeString } from "@openclaw/net-policy/redact-sensitive-url";
import {
classifyGatewayConnectFailure,
ConnectErrorDetailCodes,
readConnectErrorDetailCode,
} from "../../../packages/gateway-protocol/src/connect-error-details.js";
import type { OpenClawConfig } from "../../config/types.js";
import type { GatewayProbeResult } from "../../gateway/probe.js";
import { formatErrorMessage } from "../../infra/errors.js";
@@ -14,6 +19,9 @@ type GatewayStatusRequireRpcProbeResult = {
type GatewayStatusProbeResult = GatewayProbeResult | GatewayStatusRequireRpcProbeResult;
const probeGatewayModuleLoader = createLazyImportLoader(() => import("../../gateway/probe.js"));
const CONNECT_ERROR_DETAIL_CODE_VALUES: ReadonlySet<string> = new Set(
Object.values(ConnectErrorDetailCodes),
);
async function loadProbeGatewayModule(): Promise<typeof import("../../gateway/probe.js")> {
return await probeGatewayModuleLoader.load();
@@ -36,6 +44,21 @@ function resolveGatewayStatusProbeDetails(result: GatewayStatusProbeResult) {
return "authProbe" in result ? result.authProbe : result;
}
function projectGatewayConnectFailure(params: {
details?: unknown;
message: string;
reason?: string;
}) {
// Daemon status is serialized for diagnostics, so raw gateway details must
// stop here; only closed classification facts may cross this boundary.
const failure = classifyGatewayConnectFailure(params);
const detailCode = readConnectErrorDetailCode(params.details);
return {
kind: failure.kind,
...(detailCode && CONNECT_ERROR_DETAIL_CODE_VALUES.has(detailCode) ? { detailCode } : {}),
};
}
function readRuntimeVersionFromStatusPayload(payload: unknown): string | null {
if (!payload || typeof payload !== "object") {
return null;
@@ -129,6 +152,7 @@ export async function probeGatewayStatus(opts: {
...(version != null ? { version } : {}),
} as const;
}
const error = redactSensitiveUrlLikeString(resolveProbeFailureMessage(result));
return {
ok: false,
kind,
@@ -136,15 +160,22 @@ export async function probeGatewayStatus(opts: {
auth,
...serverSummary,
...(version != null ? { version } : {}),
connectFailure: projectGatewayConnectFailure({
details: probeDetails?.connectErrorDetails,
message: error,
reason: probeDetails?.close?.reason,
}),
// Probe failure text can echo the credential-bearing target URL (close
// reasons, transport errors); status renderers print it verbatim.
error: redactSensitiveUrlLikeString(resolveProbeFailureMessage(result)),
error,
} as const;
} catch (err) {
const error = redactSensitiveUrlLikeString(formatErrorMessage(err));
return {
ok: false,
kind,
error: redactSensitiveUrlLikeString(formatErrorMessage(err)),
connectFailure: projectGatewayConnectFailure({ message: error }),
error,
} as const;
}
}
+7
View File
@@ -2,6 +2,7 @@
import fs from "node:fs/promises";
import { uniqueStrings } from "@openclaw/normalization-core/string-normalization";
import JSON5 from "json5";
import type { classifyGatewayConnectFailure } from "../../../packages/gateway-protocol/src/connect-error-details.js";
import {
createConfigIO,
resolveConfigPath,
@@ -123,6 +124,8 @@ type CliStatusSummary = {
entrypoint?: string;
};
type GatewayConnectFailureKind = ReturnType<typeof classifyGatewayConnectFailure>["kind"];
const gatewayProbeAuthModuleLoader = createLazyImportLoader(
() => import("../../gateway/probe-auth.js"),
);
@@ -343,6 +346,10 @@ export type DaemonStatus = {
};
version?: string | null;
error?: string;
connectFailure?: {
kind: GatewayConnectFailureKind;
detailCode?: string;
};
url?: string;
authWarning?: string;
};