fix(ida): preserve busy details across timeout attribution
This commit is contained in:
1 parent
aa521d4348
commit
19f367257d
3 files changed
+84
-28
No files matched your search
@@ -1,4 +1,5 @@
|
||||
import { expect, it } from "vitest";
|
||||
import { AnalysisTimeoutError } from "../domain/analysisErrorCore.js";
|
||||
import type { JsonValue } from "../domain/jsonValue.js";
|
||||
import type { IdaMcpConnection } from "./IdaMcpConnection.js";
|
||||
import { readSettledHealth } from "./IdaBusyHealth.js";
|
||||
@@ -139,8 +140,13 @@ it("reports the busy tool of the last observation", async () => {
|
||||
clock,
|
||||
}).catch((cause: unknown) => cause);
|
||||
|
||||
expect(failure, "the operation names the tool seen last").toMatchObject({
|
||||
operation: "server_health: headless worker still busy in last_tool",
|
||||
expect(
|
||||
failure,
|
||||
"the producer retains the tool seen last independently of attribution",
|
||||
).toMatchObject({
|
||||
operation: "server_health",
|
||||
userMessage: expect.stringContaining("last_tool"),
|
||||
diagnostics: { busy_tool: "last_tool" },
|
||||
cause: busy("last_tool"),
|
||||
});
|
||||
});
|
||||
@@ -166,3 +172,34 @@ it("rejects without another read when the signal aborts after the pause has fulf
|
||||
});
|
||||
expect(failure instanceof Error && failure.cause).toBe(reason);
|
||||
});
|
||||
|
||||
it("retains the last busy observation when a shortened health request times out", async () => {
|
||||
const capturedOutput = {
|
||||
stdout: "",
|
||||
stderr: "transport closed",
|
||||
truncated: false,
|
||||
};
|
||||
const raw = busy("last_tool");
|
||||
let reads = 0;
|
||||
const { connection } = scripted([raw]);
|
||||
connection.call = async (_name, _args, options) => {
|
||||
if (reads++ === 0) return raw;
|
||||
throw new AnalysisTimeoutError("server_health", options?.timeoutMs ?? 0, {
|
||||
capturedOutput,
|
||||
});
|
||||
};
|
||||
const { clock } = virtualClock();
|
||||
const failure = await readSettledHealth(connection, {
|
||||
database: "db",
|
||||
timeoutMs: 350,
|
||||
clock,
|
||||
}).catch((cause: unknown) => cause);
|
||||
expect(failure).toMatchObject({
|
||||
operation: "server_health",
|
||||
timeoutMs: 350,
|
||||
cause: raw,
|
||||
capturedOutput,
|
||||
userMessage: expect.stringContaining("last_tool"),
|
||||
diagnostics: { busy_tool: "last_tool", health_request_timeout_ms: 250 },
|
||||
});
|
||||
});
|
||||
+22
-14
@@ -35,7 +35,7 @@ const isBusyStatus = (raw: JsonValue): boolean =>
|
||||
|
||||
interface LastBusyHealth {
|
||||
readonly raw: JsonValue;
|
||||
readonly operation: string;
|
||||
readonly tool: string;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -90,15 +90,7 @@ export const readSettledHealth = async (
|
||||
cause.timeoutMs === requestTimeoutMs &&
|
||||
requestTimeoutMs < timeoutMs
|
||||
) {
|
||||
throw new AnalysisTimeoutError(lastBusy.operation, timeoutMs, {
|
||||
cause: lastBusy.raw,
|
||||
capturedOutput: cause.capturedOutput,
|
||||
...idaRegistrationTimeoutGuidance(timeoutMs, "readiness"),
|
||||
diagnostics: {
|
||||
health_request_operation: cause.operation,
|
||||
health_request_timeout_ms: cause.timeoutMs,
|
||||
},
|
||||
});
|
||||
throw busyTimeout(lastBusy, timeoutMs, cause);
|
||||
}
|
||||
throw cause;
|
||||
}
|
||||
@@ -109,7 +101,7 @@ export const readSettledHealth = async (
|
||||
}
|
||||
const observedBusy = {
|
||||
raw,
|
||||
operation: `server_health: headless worker still busy in ${busyToolName(busy.data.busy_tool)}`,
|
||||
tool: busyToolName(busy.data.busy_tool),
|
||||
};
|
||||
lastBusy = observedBusy;
|
||||
const expired = (): AnalysisTimeoutError =>
|
||||
@@ -139,8 +131,24 @@ export const readSettledHealth = async (
|
||||
const busyTimeout = (
|
||||
lastBusy: LastBusyHealth,
|
||||
timeoutMs: number,
|
||||
): AnalysisTimeoutError =>
|
||||
new AnalysisTimeoutError(lastBusy.operation, timeoutMs, {
|
||||
requestFailure?: AnalysisTimeoutError,
|
||||
): AnalysisTimeoutError => {
|
||||
const guidance = idaRegistrationTimeoutGuidance(timeoutMs, "readiness");
|
||||
return new AnalysisTimeoutError("server_health", timeoutMs, {
|
||||
cause: lastBusy.raw,
|
||||
...idaRegistrationTimeoutGuidance(timeoutMs, "readiness"),
|
||||
...guidance,
|
||||
userMessage: `${guidance.userMessage} The last reported busy tool was ${lastBusy.tool}.`,
|
||||
diagnostics: {
|
||||
busy_tool: lastBusy.tool,
|
||||
...(requestFailure === undefined
|
||||
? {}
|
||||
: {
|
||||
health_request_operation: requestFailure.operation,
|
||||
health_request_timeout_ms: requestFailure.timeoutMs,
|
||||
}),
|
||||
},
|
||||
...(requestFailure === undefined
|
||||
? {}
|
||||
: { capturedOutput: requestFailure.capturedOutput }),
|
||||
});
|
||||
};
|
||||
@@ -771,22 +771,28 @@ describe("headless IDA transient busy health", () => {
|
||||
expect(projected.retryable, "the caller may retry").toBe(true);
|
||||
expect(
|
||||
projected.details,
|
||||
"the observed busy tool is named exactly in the operation",
|
||||
"the timeout is attributed to the requested operation",
|
||||
).toMatchObject({
|
||||
operation: "server_health: headless worker still busy in config_json_get",
|
||||
operation: "health",
|
||||
timeout_ms: 300,
|
||||
});
|
||||
expect(
|
||||
result.error.message,
|
||||
"the error message names the busy tool",
|
||||
projected.message,
|
||||
"the public error message names the busy tool",
|
||||
).toContain("config_json_get");
|
||||
expect(
|
||||
`${result.error.message} ${projected.message}`,
|
||||
"no Hopper wording leaks in",
|
||||
).not.toContain("Hopper");
|
||||
expect(result.error.cause, "the raw busy payload is the cause").toEqual(
|
||||
busy,
|
||||
);
|
||||
expect(
|
||||
result.error.cause,
|
||||
"reattribution preserves the producing timeout",
|
||||
).toMatchObject({
|
||||
_tag: "AnalysisTimeoutError",
|
||||
operation: "server_health",
|
||||
cause: busy,
|
||||
diagnostics: { busy_tool: "config_json_get" },
|
||||
});
|
||||
expect(
|
||||
elapsed,
|
||||
"the wait runs to the deadline, not less",
|
||||
@@ -808,10 +814,14 @@ describe("headless IDA transient busy health", () => {
|
||||
if (result.ok) throw new Error("expected a busy timeout");
|
||||
expect(
|
||||
projectAnalysisError(result.error).details,
|
||||
"an omitted busy_tool is reported as exactly unknown",
|
||||
"the public operation stays independent of an omitted busy tool",
|
||||
).toMatchObject({
|
||||
operation: "server_health: headless worker still busy in unknown",
|
||||
operation: "health",
|
||||
});
|
||||
expect(result.error.diagnostics).toMatchObject({ busy_tool: "unknown" });
|
||||
expect(projectAnalysisError(result.error).message).toContain(
|
||||
"busy tool was unknown",
|
||||
);
|
||||
await expectCleanClose(producer, client);
|
||||
});
|
||||
|
||||
@@ -832,11 +842,12 @@ describe("headless IDA transient busy health", () => {
|
||||
projectAnalysisError(result.error).details,
|
||||
"only a tool-name-shaped value is repeated",
|
||||
).toMatchObject({
|
||||
operation: "server_health: headless worker still busy in unknown",
|
||||
operation: "health",
|
||||
});
|
||||
expect(result.error.diagnostics).toMatchObject({ busy_tool: "unknown" });
|
||||
expect(
|
||||
result.error.message,
|
||||
"the producer text never reaches the error message",
|
||||
projectAnalysisError(result.error).message,
|
||||
"the producer text never reaches the public error message",
|
||||
).not.toContain(tool);
|
||||
await expectCleanClose(producer, client);
|
||||
},
|
||||
|
||||
Reference in new issue
Block a user