mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-28 05:16:23 -06:00
fix(browser): preserve nested timeouts and cancellation across node actions (#114131)
This commit is contained in:
committed by
GitHub
parent
01f8bd9d12
commit
bba7e30fcc
@@ -0,0 +1,307 @@
|
||||
// Browser tests cover independently bounded delegated node-proxy requests.
|
||||
import { MAX_TIMER_TIMEOUT_MS } from "openclaw/plugin-sdk/number-runtime";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
type BrowserNodeRequest = {
|
||||
nodeId: string;
|
||||
command: string;
|
||||
timeoutMs: number;
|
||||
idempotencyKey: string;
|
||||
params: {
|
||||
method: string;
|
||||
path: string;
|
||||
timeoutMs: number;
|
||||
profile?: string;
|
||||
errorEnvelope: string;
|
||||
};
|
||||
};
|
||||
|
||||
type BrowserNodeResponse = {
|
||||
payload: { result: { ok: true; profile?: string } };
|
||||
};
|
||||
|
||||
type BrowserGatewayCall = (
|
||||
method: string,
|
||||
options: { timeoutMs: number },
|
||||
request: BrowserNodeRequest,
|
||||
extra: { scopes: string[]; signal?: AbortSignal },
|
||||
) => Promise<BrowserNodeResponse>;
|
||||
|
||||
const runtimeMocks = vi.hoisted(() => ({
|
||||
callGatewayTool: vi.fn<BrowserGatewayCall>(),
|
||||
persistBrowserProxyFiles: vi.fn<(_files?: unknown) => Promise<Map<string, string>>>(),
|
||||
applyBrowserProxyPaths: vi.fn<(result: unknown, mapping: Map<string, string>) => void>(),
|
||||
fetchBrowserJson: vi.fn<(...args: unknown[]) => Promise<unknown>>(),
|
||||
}));
|
||||
|
||||
vi.mock("./browser-tool.runtime.js", () => runtimeMocks);
|
||||
|
||||
import { createBrowserNodeProxyRequest } from "./browser-node-proxy.js";
|
||||
|
||||
function createSessionProxy() {
|
||||
return createBrowserNodeProxyRequest({
|
||||
nodeTarget: { nodeId: "node-1" },
|
||||
allowAutomaticHostFallback: false,
|
||||
});
|
||||
}
|
||||
|
||||
function readGatewayCall(index = 0) {
|
||||
const call = runtimeMocks.callGatewayTool.mock.calls[index];
|
||||
if (!call) {
|
||||
throw new Error(`Expected Browser node invocation ${index}`);
|
||||
}
|
||||
const [method, gateway, node, extra] = call;
|
||||
return { method, gateway, node, extra };
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
runtimeMocks.callGatewayTool.mockReset();
|
||||
runtimeMocks.callGatewayTool.mockResolvedValue({ payload: { result: { ok: true } } });
|
||||
runtimeMocks.persistBrowserProxyFiles.mockReset();
|
||||
runtimeMocks.persistBrowserProxyFiles.mockResolvedValue(new Map<string, string>());
|
||||
runtimeMocks.applyBrowserProxyPaths.mockReset();
|
||||
runtimeMocks.fetchBrowserJson.mockReset();
|
||||
});
|
||||
|
||||
describe("Browser node proxy nested watchdogs", () => {
|
||||
it("keeps a requested action inside separate node and Gateway watchdogs", async () => {
|
||||
const signal = new AbortController().signal;
|
||||
|
||||
await createSessionProxy()({
|
||||
method: "GET",
|
||||
path: "/snapshot",
|
||||
profile: "work",
|
||||
timeoutMs: 7_777,
|
||||
signal,
|
||||
});
|
||||
|
||||
const { method, gateway, node, extra } = readGatewayCall();
|
||||
expect(method).toBe("node.invoke");
|
||||
expect(node.nodeId).toBe("node-1");
|
||||
expect(node.command).toBe("browser.proxy");
|
||||
expect([node.params.timeoutMs, node.timeoutMs, gateway.timeoutMs]).toEqual([
|
||||
7_777, 12_777, 17_777,
|
||||
]);
|
||||
expect(node.params.errorEnvelope).toBe("browser-v1");
|
||||
expect(extra).toEqual({ scopes: ["operator.admin"], signal });
|
||||
expect(runtimeMocks.fetchBrowserJson).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("binds the tool execution signal to node requests without their own signal", async () => {
|
||||
const signal = new AbortController().signal;
|
||||
const proxy = createBrowserNodeProxyRequest({
|
||||
nodeTarget: { nodeId: "node-1" },
|
||||
allowAutomaticHostFallback: false,
|
||||
signal,
|
||||
});
|
||||
|
||||
await proxy({ method: "GET", path: "/snapshot" });
|
||||
|
||||
expect(readGatewayCall().extra).toEqual({ scopes: ["operator.admin"], signal });
|
||||
});
|
||||
|
||||
it("keeps an explicit request signal ahead of the bound tool execution signal", async () => {
|
||||
const toolSignal = new AbortController().signal;
|
||||
const requestSignal = new AbortController().signal;
|
||||
const proxy = createBrowserNodeProxyRequest({
|
||||
nodeTarget: { nodeId: "node-1" },
|
||||
allowAutomaticHostFallback: false,
|
||||
signal: toolSignal,
|
||||
});
|
||||
|
||||
await proxy({ method: "GET", path: "/snapshot", signal: requestSignal });
|
||||
|
||||
expect(readGatewayCall().extra).toEqual({
|
||||
scopes: ["operator.admin"],
|
||||
signal: requestSignal,
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps the bound tool execution signal when a node falls back to the host", async () => {
|
||||
const signal = new AbortController().signal;
|
||||
runtimeMocks.callGatewayTool.mockRejectedValueOnce(
|
||||
new Error("Browser control host is not reachable on 127.0.0.1:18791."),
|
||||
);
|
||||
runtimeMocks.fetchBrowserJson.mockResolvedValueOnce({ ok: true, source: "gateway-host" });
|
||||
const proxy = createBrowserNodeProxyRequest({
|
||||
nodeTarget: { nodeId: "node-1" },
|
||||
allowAutomaticHostFallback: true,
|
||||
signal,
|
||||
});
|
||||
|
||||
await expect(proxy({ method: "GET", path: "/snapshot" })).resolves.toMatchObject({
|
||||
ok: true,
|
||||
source: "gateway-host",
|
||||
});
|
||||
|
||||
expect(runtimeMocks.fetchBrowserJson).toHaveBeenCalledWith(
|
||||
"/snapshot",
|
||||
expect.objectContaining({ method: "GET", signal }),
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps default action, node, and Gateway watchdogs strictly nested", async () => {
|
||||
await createSessionProxy()({ method: "GET", path: "/snapshot" });
|
||||
|
||||
const { gateway, node } = readGatewayCall();
|
||||
expect([node.params.timeoutMs, node.timeoutMs, gateway.timeoutMs]).toEqual([
|
||||
20_000, 25_000, 30_000,
|
||||
]);
|
||||
});
|
||||
|
||||
it.each([
|
||||
Number.MAX_SAFE_INTEGER,
|
||||
MAX_TIMER_TIMEOUT_MS,
|
||||
MAX_TIMER_TIMEOUT_MS - 1,
|
||||
MAX_TIMER_TIMEOUT_MS - 4_999,
|
||||
MAX_TIMER_TIMEOUT_MS - 5_000,
|
||||
MAX_TIMER_TIMEOUT_MS - 9_999,
|
||||
MAX_TIMER_TIMEOUT_MS - 10_000,
|
||||
MAX_TIMER_TIMEOUT_MS - 10_001,
|
||||
])("reserves both Node-safe watchdog windows for %i ms", async (timeoutMs) => {
|
||||
await createSessionProxy()({ method: "GET", path: "/snapshot", timeoutMs });
|
||||
|
||||
const actionTimeoutMs = Math.min(timeoutMs, MAX_TIMER_TIMEOUT_MS - 10_000);
|
||||
const { gateway, node } = readGatewayCall();
|
||||
expect([node.params.timeoutMs, node.timeoutMs, gateway.timeoutMs]).toEqual([
|
||||
actionTimeoutMs,
|
||||
actionTimeoutMs + 5_000,
|
||||
actionTimeoutMs + 10_000,
|
||||
]);
|
||||
expect(gateway.timeoutMs).toBeLessThanOrEqual(MAX_TIMER_TIMEOUT_MS);
|
||||
});
|
||||
|
||||
it("finishes ten independently owned, concurrently blocked Browser sessions", async () => {
|
||||
let release!: () => void;
|
||||
const barrier = new Promise<void>((resolve) => {
|
||||
release = resolve;
|
||||
});
|
||||
runtimeMocks.callGatewayTool.mockImplementation(async (_method, _gateway, node) => {
|
||||
await barrier;
|
||||
return { payload: { result: { ok: true, profile: node.params.profile } } };
|
||||
});
|
||||
|
||||
const sessions = Array.from({ length: 10 }, (_, index) => ({
|
||||
profile: `session-${index}`,
|
||||
timeoutMs: 7_777 + index,
|
||||
signal: new AbortController().signal,
|
||||
proxy: createSessionProxy(),
|
||||
}));
|
||||
const completed = new Set<string>();
|
||||
const pending = sessions.map(async ({ profile, timeoutMs, signal, proxy }) => {
|
||||
const result = await proxy({ method: "GET", path: "/snapshot", profile, timeoutMs, signal });
|
||||
completed.add(profile);
|
||||
return result;
|
||||
});
|
||||
|
||||
try {
|
||||
expect(runtimeMocks.callGatewayTool).toHaveBeenCalledTimes(10);
|
||||
expect(completed.size).toBe(0);
|
||||
expect(runtimeMocks.persistBrowserProxyFiles).not.toHaveBeenCalled();
|
||||
const invocationIds = new Set<string>();
|
||||
|
||||
sessions.forEach(({ profile, timeoutMs, signal }, index) => {
|
||||
const { method, gateway, node, extra } = readGatewayCall(index);
|
||||
expect(method).toBe("node.invoke");
|
||||
expect(node.nodeId).toBe("node-1");
|
||||
expect(node.command).toBe("browser.proxy");
|
||||
expect(node.params.profile).toBe(profile);
|
||||
expect(node.params.errorEnvelope).toBe("browser-v1");
|
||||
expect([node.params.timeoutMs, node.timeoutMs, gateway.timeoutMs]).toEqual([
|
||||
timeoutMs,
|
||||
timeoutMs + 5_000,
|
||||
timeoutMs + 10_000,
|
||||
]);
|
||||
expect(extra).toEqual({ scopes: ["operator.admin"], signal });
|
||||
invocationIds.add(node.idempotencyKey);
|
||||
});
|
||||
|
||||
expect(invocationIds.size).toBe(10);
|
||||
expect(runtimeMocks.fetchBrowserJson).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
release();
|
||||
}
|
||||
|
||||
await expect(Promise.all(pending)).resolves.toEqual(
|
||||
sessions.map(({ profile }) => ({ ok: true, profile })),
|
||||
);
|
||||
expect(completed.size).toBe(10);
|
||||
expect(runtimeMocks.persistBrowserProxyFiles).toHaveBeenCalledTimes(10);
|
||||
expect(runtimeMocks.applyBrowserProxyPaths).toHaveBeenCalledTimes(10);
|
||||
expect(runtimeMocks.fetchBrowserJson).not.toHaveBeenCalled();
|
||||
expect(sessions.every(({ proxy }) => !proxy.isHostFallbackActive())).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps one session cancellation isolated from nine concurrent sessions", async () => {
|
||||
let release!: () => void;
|
||||
const barrier = new Promise<void>((resolve) => {
|
||||
release = resolve;
|
||||
});
|
||||
runtimeMocks.callGatewayTool.mockImplementation(
|
||||
(_method, _gateway, node, extra) =>
|
||||
new Promise<BrowserNodeResponse>((resolve, reject) => {
|
||||
const onAbort = () => {
|
||||
const reason = extra.signal?.reason;
|
||||
reject(reason instanceof Error ? reason : new Error("Browser session cancelled"));
|
||||
};
|
||||
if (extra.signal?.aborted) {
|
||||
onAbort();
|
||||
return;
|
||||
}
|
||||
extra.signal?.addEventListener("abort", onAbort, { once: true });
|
||||
void barrier.then(() => {
|
||||
extra.signal?.removeEventListener("abort", onAbort);
|
||||
if (!extra.signal?.aborted) {
|
||||
resolve({ payload: { result: { ok: true, profile: node.params.profile } } });
|
||||
}
|
||||
});
|
||||
}),
|
||||
);
|
||||
|
||||
const sessions = Array.from({ length: 10 }, (_, index) => ({
|
||||
profile: `session-${index}`,
|
||||
timeoutMs: 7_777 + index,
|
||||
controller: new AbortController(),
|
||||
proxy: createSessionProxy(),
|
||||
}));
|
||||
const pending = sessions.map(({ profile, timeoutMs, controller, proxy }) =>
|
||||
proxy({
|
||||
method: "GET",
|
||||
path: "/snapshot",
|
||||
profile,
|
||||
timeoutMs,
|
||||
signal: controller.signal,
|
||||
}),
|
||||
);
|
||||
const completion = Promise.allSettled(pending);
|
||||
const cancelledSession = sessions.at(3);
|
||||
const cancelledRun = pending.at(3);
|
||||
if (!cancelledSession || !cancelledRun) {
|
||||
release();
|
||||
throw new Error("Expected a dedicated cancellation session");
|
||||
}
|
||||
const abortError = new Error("session-3 cancelled");
|
||||
|
||||
try {
|
||||
expect(runtimeMocks.callGatewayTool).toHaveBeenCalledTimes(10);
|
||||
cancelledSession.controller.abort(abortError);
|
||||
await expect(cancelledRun).rejects.toBe(abortError);
|
||||
expect(runtimeMocks.persistBrowserProxyFiles).not.toHaveBeenCalled();
|
||||
expect(runtimeMocks.fetchBrowserJson).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
release();
|
||||
}
|
||||
|
||||
await expect(completion).resolves.toEqual(
|
||||
sessions.map(({ profile }, index) =>
|
||||
index === 3
|
||||
? { status: "rejected", reason: abortError }
|
||||
: { status: "fulfilled", value: { ok: true, profile } },
|
||||
),
|
||||
);
|
||||
expect(runtimeMocks.persistBrowserProxyFiles).toHaveBeenCalledTimes(9);
|
||||
expect(runtimeMocks.applyBrowserProxyPaths).toHaveBeenCalledTimes(9);
|
||||
expect(runtimeMocks.fetchBrowserJson).not.toHaveBeenCalled();
|
||||
expect(sessions.every(({ proxy }) => !proxy.isHostFallbackActive())).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,9 @@
|
||||
import crypto from "node:crypto";
|
||||
import {
|
||||
addTimerTimeoutGraceMs,
|
||||
MAX_TIMER_TIMEOUT_MS,
|
||||
resolveTimerTimeoutMs,
|
||||
} from "openclaw/plugin-sdk/number-runtime";
|
||||
import { createSubsystemLogger } from "openclaw/plugin-sdk/runtime-env";
|
||||
import { isBrowserControlHostUnavailableError } from "./browser-node-fallback.js";
|
||||
import {
|
||||
@@ -65,11 +70,18 @@ async function callBrowserProxy(params: {
|
||||
profile?: string;
|
||||
signal?: AbortSignal;
|
||||
}): Promise<BrowserProxySuccess> {
|
||||
const proxyTimeoutMs =
|
||||
typeof params.timeoutMs === "number" && Number.isFinite(params.timeoutMs)
|
||||
? Math.max(1, Math.floor(params.timeoutMs))
|
||||
: DEFAULT_BROWSER_PROXY_TIMEOUT_MS;
|
||||
const gatewayTimeoutMs = proxyTimeoutMs + BROWSER_PROXY_GATEWAY_TIMEOUT_SLACK_MS;
|
||||
// Reserve both watchdog windows before clamping so timer saturation cannot
|
||||
// make an outer watchdog expire alongside the browser action.
|
||||
const proxyTimeoutMs = Math.min(
|
||||
resolveTimerTimeoutMs(params.timeoutMs, DEFAULT_BROWSER_PROXY_TIMEOUT_MS),
|
||||
MAX_TIMER_TIMEOUT_MS - 2 * BROWSER_PROXY_GATEWAY_TIMEOUT_SLACK_MS,
|
||||
);
|
||||
const nodeInvokeTimeoutMs =
|
||||
addTimerTimeoutGraceMs(proxyTimeoutMs, BROWSER_PROXY_GATEWAY_TIMEOUT_SLACK_MS) ??
|
||||
proxyTimeoutMs;
|
||||
const gatewayTimeoutMs =
|
||||
addTimerTimeoutGraceMs(nodeInvokeTimeoutMs, BROWSER_PROXY_GATEWAY_TIMEOUT_SLACK_MS) ??
|
||||
nodeInvokeTimeoutMs;
|
||||
let payload: { payload?: unknown; payloadJSON?: unknown } | null;
|
||||
try {
|
||||
payload = await callGatewayTool<{ payload?: unknown; payloadJSON?: unknown }>(
|
||||
@@ -78,9 +90,9 @@ async function callBrowserProxy(params: {
|
||||
{
|
||||
nodeId: params.nodeId,
|
||||
command: "browser.proxy",
|
||||
// node.invoke owns a separate watchdog from browser.proxy. Keep both
|
||||
// bounded, with enough outer slack for the proxy result to cross back.
|
||||
timeoutMs: gatewayTimeoutMs,
|
||||
// Keep the browser action, node watchdog, and Gateway RPC on distinct
|
||||
// budgets so a detailed node timeout can cross both outer boundaries.
|
||||
timeoutMs: nodeInvokeTimeoutMs,
|
||||
params: {
|
||||
method: params.method,
|
||||
path: params.path,
|
||||
@@ -136,17 +148,24 @@ async function callLocalBrowserControl(params: Parameters<BrowserProxyRequest>[0
|
||||
export function createBrowserNodeProxyRequest(params: {
|
||||
nodeTarget: { nodeId: string; label?: string };
|
||||
allowAutomaticHostFallback: boolean;
|
||||
signal?: AbortSignal;
|
||||
}): BrowserProxyRequest {
|
||||
let hostFallbackActive = false;
|
||||
const dispatch = async (request: Parameters<BrowserProxyRequest>[0]) => {
|
||||
// Bind cancellation once so every node action and its safe host fallback
|
||||
// inherit their execution signal without overriding an explicit request.
|
||||
const requestWithSignal =
|
||||
request.signal || params.signal
|
||||
? { ...request, signal: request.signal ?? params.signal }
|
||||
: request;
|
||||
if (hostFallbackActive) {
|
||||
return await callLocalBrowserControl(request);
|
||||
return await callLocalBrowserControl(requestWithSignal);
|
||||
}
|
||||
try {
|
||||
const proxy = await callBrowserProxy({
|
||||
nodeId: params.nodeTarget.nodeId,
|
||||
markControlHostUnavailable: params.allowAutomaticHostFallback,
|
||||
...request,
|
||||
...requestWithSignal,
|
||||
});
|
||||
const mapping = await persistBrowserProxyFiles(proxy.files);
|
||||
applyBrowserProxyPaths(proxy.result, mapping);
|
||||
@@ -164,7 +183,7 @@ export function createBrowserNodeProxyRequest(params: {
|
||||
logger.warn(
|
||||
`browser node ${params.nodeTarget.label ?? params.nodeTarget.nodeId} control host unavailable; falling back to Gateway host`,
|
||||
);
|
||||
return await callLocalBrowserControl(request);
|
||||
return await callLocalBrowserControl(requestWithSignal);
|
||||
}
|
||||
};
|
||||
return Object.assign(dispatch, {
|
||||
|
||||
@@ -532,6 +532,7 @@ function nodeInvokeCall(callIndex: number): {
|
||||
nodeId?: string;
|
||||
command?: string;
|
||||
timeoutMs?: number;
|
||||
idempotencyKey?: string;
|
||||
params?: {
|
||||
method?: string;
|
||||
path?: string;
|
||||
@@ -542,7 +543,7 @@ function nodeInvokeCall(callIndex: number): {
|
||||
body?: Record<string, unknown>;
|
||||
};
|
||||
};
|
||||
extra?: { scopes?: string[] };
|
||||
extra?: { scopes?: string[]; signal?: AbortSignal };
|
||||
} {
|
||||
const toolName = mockCallArg<string>(gatewayMocks.callGatewayTool, callIndex, 0);
|
||||
const options = mockCallArg<{ timeoutMs?: number }>(gatewayMocks.callGatewayTool, callIndex, 1);
|
||||
@@ -550,6 +551,7 @@ function nodeInvokeCall(callIndex: number): {
|
||||
nodeId?: string;
|
||||
command?: string;
|
||||
timeoutMs?: number;
|
||||
idempotencyKey?: string;
|
||||
params?: {
|
||||
method?: string;
|
||||
path?: string;
|
||||
@@ -559,7 +561,7 @@ function nodeInvokeCall(callIndex: number): {
|
||||
body?: Record<string, unknown>;
|
||||
};
|
||||
}>(gatewayMocks.callGatewayTool, callIndex, 2);
|
||||
const extra = mockCallArg<{ scopes?: string[] } | undefined>(
|
||||
const extra = mockCallArg<{ scopes?: string[]; signal?: AbortSignal } | undefined>(
|
||||
gatewayMocks.callGatewayTool,
|
||||
callIndex,
|
||||
3,
|
||||
@@ -572,6 +574,50 @@ function lastNodeInvokeCall(): ReturnType<typeof nodeInvokeCall> {
|
||||
return nodeInvokeCall(-1);
|
||||
}
|
||||
|
||||
function blockBrowserNodeGateway(count = 1): () => void {
|
||||
let release!: () => void;
|
||||
const barrier = new Promise<void>((resolve) => {
|
||||
release = resolve;
|
||||
});
|
||||
|
||||
for (let index = 0; index < count; index += 1) {
|
||||
gatewayMocks.callGatewayTool.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise<Record<string, unknown>>((resolve, reject) => {
|
||||
const { request, extra } = lastNodeInvokeCall();
|
||||
const signal = extra?.signal;
|
||||
const onAbort = () => {
|
||||
const reason = signal?.reason;
|
||||
reject(reason instanceof Error ? reason : new Error("Browser tool cancelled"));
|
||||
};
|
||||
if (signal?.aborted) {
|
||||
onAbort();
|
||||
return;
|
||||
}
|
||||
signal?.addEventListener("abort", onAbort, { once: true });
|
||||
void barrier.then(() => {
|
||||
signal?.removeEventListener("abort", onAbort);
|
||||
if (!signal?.aborted) {
|
||||
resolve({
|
||||
ok: true,
|
||||
payload: {
|
||||
result: {
|
||||
ok: true,
|
||||
running: true,
|
||||
profile: request.params?.profile,
|
||||
path: "/tmp/test.png",
|
||||
},
|
||||
},
|
||||
});
|
||||
}
|
||||
});
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
return release;
|
||||
}
|
||||
|
||||
describe("browser tool output schema", () => {
|
||||
it("accepts snapshot details", async () => {
|
||||
const tool = createBrowserTool();
|
||||
@@ -673,7 +719,7 @@ describe("browser tool download actions", () => {
|
||||
});
|
||||
|
||||
const { options, request } = lastNodeInvokeCall();
|
||||
expect(options.timeoutMs).toBe(40_000);
|
||||
expect(options.timeoutMs).toBe(45_000);
|
||||
expect(request.params?.path).toBe("/wait/download");
|
||||
expect(request.params?.timeoutMs).toBe(35_000);
|
||||
expect(request.params?.body).toEqual({
|
||||
@@ -705,7 +751,7 @@ describe("browser tool download actions", () => {
|
||||
});
|
||||
|
||||
const { options, request } = lastNodeInvokeCall();
|
||||
expect(options.timeoutMs).toBe(130_000);
|
||||
expect(options.timeoutMs).toBe(135_000);
|
||||
expect(request.params?.timeoutMs).toBe(125_000);
|
||||
expect(request.params?.path).toBe("/download");
|
||||
expect(request.params?.body).toMatchObject({ ref: "e12", path: "report.pdf" });
|
||||
@@ -819,7 +865,7 @@ describe("browser tool snapshot maxChars", () => {
|
||||
await tool.execute?.("call-1", { action: "status", profile: "user", target: "node" });
|
||||
|
||||
const { options, request } = lastNodeInvokeCall();
|
||||
expect(options.timeoutMs).toBe(50_000);
|
||||
expect(options.timeoutMs).toBe(55_000);
|
||||
expect(request.params?.method).toBe("GET");
|
||||
expect(request.params?.path).toBe("/");
|
||||
expect(request.params?.profile).toBe("user");
|
||||
@@ -980,8 +1026,8 @@ describe("browser tool snapshot maxChars", () => {
|
||||
|
||||
expect(gatewayMocks.callGatewayTool).toHaveBeenCalledWith(
|
||||
"node.invoke",
|
||||
// proxy adds a 5_000 ms slack on top of the per-request timeout.
|
||||
expect.objectContaining({ timeoutMs: 7777 + 5_000 }),
|
||||
// The Gateway watchdog must also outlive the separate node watchdog.
|
||||
expect.objectContaining({ timeoutMs: 7777 + 10_000 }),
|
||||
expect.objectContaining({
|
||||
command: "browser.proxy",
|
||||
params: expect.objectContaining({
|
||||
@@ -1102,7 +1148,7 @@ describe("browser tool snapshot maxChars", () => {
|
||||
await tool.execute?.("call-1", { action: "status", target: "node" });
|
||||
|
||||
const { options, request, extra } = lastNodeInvokeCall();
|
||||
expect(options.timeoutMs).toBe(25_000);
|
||||
expect(options.timeoutMs).toBe(30_000);
|
||||
expect(extra?.scopes).toEqual(["operator.admin"]);
|
||||
expect(request.nodeId).toBe("node-1");
|
||||
expect(request.command).toBe("browser.proxy");
|
||||
@@ -1111,6 +1157,99 @@ describe("browser tool snapshot maxChars", () => {
|
||||
expect(browserClientMocks.browserStatus).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ action: "status", path: "/" },
|
||||
{ action: "screenshot", path: "/screenshot" },
|
||||
])("cancels an actual blocked node-backed $action tool execution", async ({ action, path }) => {
|
||||
mockSingleBrowserProxyNode();
|
||||
const release = blockBrowserNodeGateway();
|
||||
const controller = new AbortController();
|
||||
const abortError = new Error(`${action} tool execution cancelled`);
|
||||
const pending = createBrowserTool().execute!(
|
||||
`cancel-node-${action}`,
|
||||
{ action, target: "node" },
|
||||
controller.signal,
|
||||
);
|
||||
|
||||
try {
|
||||
await vi.waitFor(() => expect(gatewayMocks.callGatewayTool).toHaveBeenCalledTimes(1));
|
||||
const { request, extra } = lastNodeInvokeCall();
|
||||
expect(request.params?.path).toBe(path);
|
||||
expect(extra?.signal).toBe(controller.signal);
|
||||
controller.abort(abortError);
|
||||
await expect(pending).rejects.toBe(abortError);
|
||||
expect(toolCommonMocks.fetchBrowserJson).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
release();
|
||||
}
|
||||
});
|
||||
|
||||
it("isolates one cancelled real Browser tool execution from nine blocked node sessions", async () => {
|
||||
mockSingleBrowserProxyNode();
|
||||
const release = blockBrowserNodeGateway(10);
|
||||
const sessions = Array.from({ length: 10 }, (_, index) => ({
|
||||
profile: `session-${index}`,
|
||||
controller: new AbortController(),
|
||||
tool: createBrowserTool(),
|
||||
}));
|
||||
const completed = new Set<string>();
|
||||
const pending = sessions.map(({ profile, controller, tool }) =>
|
||||
tool.execute!(
|
||||
`browser-tool-${profile}`,
|
||||
{ action: "status", target: "node", profile },
|
||||
controller.signal,
|
||||
).then((result) => {
|
||||
completed.add(profile);
|
||||
return result;
|
||||
}),
|
||||
);
|
||||
const completion = Promise.allSettled(pending);
|
||||
const cancelledSession = sessions.at(3);
|
||||
const cancelledRun = pending.at(3);
|
||||
if (!cancelledSession || !cancelledRun) {
|
||||
release();
|
||||
throw new Error("Expected a dedicated Browser tool cancellation session");
|
||||
}
|
||||
const abortError = new Error("Browser tool session-3 cancelled");
|
||||
|
||||
try {
|
||||
await vi.waitFor(() => expect(gatewayMocks.callGatewayTool).toHaveBeenCalledTimes(10));
|
||||
expect(completed.size).toBe(0);
|
||||
const invocationIds = new Set<string>();
|
||||
sessions.forEach(({ profile, controller }, index) => {
|
||||
const { request, extra } = nodeInvokeCall(index);
|
||||
expect(request.params?.path).toBe("/");
|
||||
expect(request.params?.profile).toBe(profile);
|
||||
expect(extra?.signal).toBe(controller.signal);
|
||||
if (request.idempotencyKey) {
|
||||
invocationIds.add(request.idempotencyKey);
|
||||
}
|
||||
});
|
||||
expect(invocationIds.size).toBe(10);
|
||||
cancelledSession.controller.abort(abortError);
|
||||
await expect(cancelledRun).rejects.toBe(abortError);
|
||||
expect(completed.size).toBe(0);
|
||||
expect(toolCommonMocks.fetchBrowserJson).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
release();
|
||||
}
|
||||
|
||||
await expect(completion).resolves.toEqual(
|
||||
sessions.map(({ profile }, index) =>
|
||||
index === 3
|
||||
? { status: "rejected", reason: abortError }
|
||||
: {
|
||||
status: "fulfilled",
|
||||
value: expect.objectContaining({
|
||||
details: expect.objectContaining({ ok: true, profile }),
|
||||
}),
|
||||
},
|
||||
),
|
||||
);
|
||||
expect(completed.size).toBe(9);
|
||||
expect(toolCommonMocks.fetchBrowserJson).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("falls back to the Gateway host when an auto-selected node has no browser host", async () => {
|
||||
mockSingleBrowserProxyNode();
|
||||
gatewayMocks.callGatewayTool.mockRejectedValueOnce(
|
||||
@@ -1420,7 +1559,7 @@ describe("browser tool snapshot maxChars", () => {
|
||||
await tool.execute?.("call-1", { action: "doctor", target: "node" });
|
||||
|
||||
const { options, request } = lastNodeInvokeCall();
|
||||
expect(options.timeoutMs).toBe(25_000);
|
||||
expect(options.timeoutMs).toBe(30_000);
|
||||
expect(request.nodeId).toBe("node-1");
|
||||
expect(request.command).toBe("browser.proxy");
|
||||
expect(request.params?.method).toBe("GET");
|
||||
@@ -1667,7 +1806,7 @@ describe("browser tool snapshot maxChars", () => {
|
||||
|
||||
const { options, request } = lastNodeInvokeCall();
|
||||
const body = request.params?.body as { targetId?: string; timeoutMs?: number } | undefined;
|
||||
expect(options.timeoutMs).toBe(17_345);
|
||||
expect(options.timeoutMs).toBe(22_345);
|
||||
expect(request.params?.method).toBe("POST");
|
||||
expect(request.params?.path).toBe("/screenshot");
|
||||
expect(request.params?.timeoutMs).toBe(12_345);
|
||||
@@ -1692,7 +1831,7 @@ describe("browser tool snapshot maxChars", () => {
|
||||
|
||||
const { options, request } = lastNodeInvokeCall();
|
||||
const body = request.params?.body as { timeoutMs?: number } | undefined;
|
||||
expect(options.timeoutMs).toBe(25_000);
|
||||
expect(options.timeoutMs).toBe(30_000);
|
||||
expect(request.params?.timeoutMs).toBe(20_000);
|
||||
expect(body?.timeoutMs).toBe(20_000);
|
||||
});
|
||||
@@ -1728,11 +1867,11 @@ describe("browser tool snapshot maxChars", () => {
|
||||
|
||||
expect((result?.details as { refsFallback?: string } | undefined)?.refsFallback).toBe("role");
|
||||
const firstCall = nodeInvokeCall(0);
|
||||
expect(firstCall.options.timeoutMs).toBe(25_000);
|
||||
expect(firstCall.options.timeoutMs).toBe(30_000);
|
||||
expect(firstCall.request.params?.path).toBe("/snapshot");
|
||||
expect(firstCall.request.params?.query?.refs).toBe("aria");
|
||||
const secondCall = nodeInvokeCall(1);
|
||||
expect(secondCall.options.timeoutMs).toBe(25_000);
|
||||
expect(secondCall.options.timeoutMs).toBe(30_000);
|
||||
expect(secondCall.request.params?.path).toBe("/snapshot");
|
||||
expect(secondCall.request.params?.query?.refs).toBe("role");
|
||||
});
|
||||
@@ -1753,7 +1892,7 @@ describe("browser tool snapshot maxChars", () => {
|
||||
});
|
||||
|
||||
const { options, request } = lastNodeInvokeCall();
|
||||
expect(options.timeoutMs).toBe(25_000);
|
||||
expect(options.timeoutMs).toBe(30_000);
|
||||
expect(request.params?.timeoutMs).toBe(20_000);
|
||||
});
|
||||
|
||||
@@ -1778,7 +1917,7 @@ describe("browser tool snapshot maxChars", () => {
|
||||
await tool.execute?.("call-1", { action: "status", profile: "user" });
|
||||
|
||||
const { options, request } = lastNodeInvokeCall();
|
||||
expect(options.timeoutMs).toBe(50_000);
|
||||
expect(options.timeoutMs).toBe(55_000);
|
||||
expect(request.nodeId).toBe("node-1");
|
||||
expect(request.command).toBe("browser.proxy");
|
||||
expect(request.params?.profile).toBe("user");
|
||||
@@ -1829,7 +1968,7 @@ describe("browser tool snapshot maxChars", () => {
|
||||
await tool.execute?.("call-1", { action: "status", profile: "user", target: "node" });
|
||||
|
||||
const { options, request } = lastNodeInvokeCall();
|
||||
expect(options.timeoutMs).toBe(50_000);
|
||||
expect(options.timeoutMs).toBe(55_000);
|
||||
expect(request.nodeId).toBe("node-1");
|
||||
expect(request.command).toBe("browser.proxy");
|
||||
expect(request.params?.profile).toBe("user");
|
||||
@@ -1848,7 +1987,7 @@ describe("browser tool snapshot maxChars", () => {
|
||||
await tool.execute?.("call-1", { action: "status", profile: "user", node: "node-1" });
|
||||
|
||||
const { options, request } = lastNodeInvokeCall();
|
||||
expect(options.timeoutMs).toBe(50_000);
|
||||
expect(options.timeoutMs).toBe(55_000);
|
||||
expect(request.nodeId).toBe("node-1");
|
||||
expect(request.command).toBe("browser.proxy");
|
||||
expect(request.params?.profile).toBe("user");
|
||||
@@ -2515,7 +2654,7 @@ describe("browser tool act compatibility", () => {
|
||||
});
|
||||
|
||||
const { options, request } = lastNodeInvokeCall();
|
||||
expect(options.timeoutMs).toBe(75_000);
|
||||
expect(options.timeoutMs).toBe(80_000);
|
||||
expect(request.params?.path).toBe("/act");
|
||||
expect(request.params?.body).toEqual({
|
||||
kind: "wait",
|
||||
@@ -2551,7 +2690,7 @@ describe("browser tool act compatibility", () => {
|
||||
const { options, request } = lastNodeInvokeCall();
|
||||
expect(request.params?.timeoutMs).toBe(95_000);
|
||||
expect(request.timeoutMs).toBe(100_000);
|
||||
expect(options.timeoutMs).toBe(100_000);
|
||||
expect(options.timeoutMs).toBe(105_000);
|
||||
});
|
||||
|
||||
it("rejects fractional act request timeouts before node proxy calls", async () => {
|
||||
|
||||
@@ -520,7 +520,7 @@ export function createBrowserTool(opts?: {
|
||||
opts?.allowHostControl !== false,
|
||||
);
|
||||
const proxyRequest = nodeTarget
|
||||
? createBrowserNodeProxyRequest({ nodeTarget, allowAutomaticHostFallback })
|
||||
? createBrowserNodeProxyRequest({ nodeTarget, allowAutomaticHostFallback, signal })
|
||||
: null;
|
||||
const toolTimeoutMs =
|
||||
requestedTimeoutMs ??
|
||||
|
||||
Reference in New Issue
Block a user