fix(browser): evict stale extension relay targets from cache (#31362)
* fix(browser): prune stale extension relay targets * test(browser): cover relay stale target pruning * changelog: note extension relay stale target fix
This commit is contained in:
Vincent Koc
@@ -51,6 +51,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Browser/Extension navigation reattach: preserve debugger re-attachment when relay is temporarily disconnected by deferring relay attach events until reconnect/re-announce, reducing post-navigation tab loss. (#28725) Thanks @stone-jin.
|
||||
- Browser/Profile defaults: prefer `openclaw` profile over `chrome` in headless/no-sandbox environments unless an explicit `defaultProfile` is configured. (#14944) Thanks @BenediktSchackenberg.
|
||||
- Browser/Remote CDP ownership checks: skip local-process ownership errors for non-loopback remote CDP profiles when HTTP is reachable but the websocket handshake fails, and surface the remote websocket attach/retry path instead. (#15582) Landed from contributor (#28780) Thanks @stubbi, @bsormagec, @unblockedgamesstudio and @vincentkoc.
|
||||
- Browser/Extension relay stale tabs: evict stale cached targets from `/json/list` when extension targets are destroyed/crashed or commands fail with missing target/session errors. (#6175) Thanks @vincentkoc.
|
||||
- CLI/Browser start timeout: honor `openclaw browser --timeout <ms> start` and stop by removing the fixed 15000ms override so slower Chrome startups can use caller-provided timeouts. (#22412, #23427) Thanks @vincentkoc.
|
||||
- Browser/CDP startup diagnostics: include Chrome stderr output and a Linux no-sandbox hint in startup timeout errors so failed launches are easier to diagnose. (#29312) Thanks @veast.
|
||||
- Docker/Image health checks: add Dockerfile `HEALTHCHECK` that probes gateway `GET /healthz` so container runtimes can mark unhealthy instances without requiring auth credentials in the probe command. (#11478) Thanks @U-C4N and @vincentkoc.
|
||||
|
||||
@@ -730,6 +730,148 @@ describe("chrome extension relay server", () => {
|
||||
RELAY_TEST_TIMEOUT_MS,
|
||||
);
|
||||
|
||||
it("removes cached targets from /json/list when targetDestroyed arrives", async () => {
|
||||
const { ext } = await startRelayWithExtension();
|
||||
|
||||
ext.send(
|
||||
JSON.stringify({
|
||||
method: "forwardCDPEvent",
|
||||
params: {
|
||||
method: "Target.attachedToTarget",
|
||||
params: {
|
||||
sessionId: "cb-tab-1",
|
||||
targetInfo: {
|
||||
targetId: "t1",
|
||||
type: "page",
|
||||
title: "Example",
|
||||
url: "https://example.com",
|
||||
},
|
||||
waitingForDebugger: false,
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
await waitForListMatch(
|
||||
async () =>
|
||||
(await fetch(`${cdpUrl}/json/list`, {
|
||||
headers: relayAuthHeaders(cdpUrl),
|
||||
}).then((r) => r.json())) as Array<{ id?: string }>,
|
||||
(list) => list.some((target) => target.id === "t1"),
|
||||
);
|
||||
|
||||
ext.send(
|
||||
JSON.stringify({
|
||||
method: "forwardCDPEvent",
|
||||
params: {
|
||||
method: "Target.targetDestroyed",
|
||||
params: { targetId: "t1" },
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const updatedList = await waitForListMatch(
|
||||
async () =>
|
||||
(await fetch(`${cdpUrl}/json/list`, {
|
||||
headers: relayAuthHeaders(cdpUrl),
|
||||
}).then((r) => r.json())) as Array<{ id?: string }>,
|
||||
(list) => list.every((target) => target.id !== "t1"),
|
||||
);
|
||||
|
||||
expect(updatedList.some((target) => target.id === "t1")).toBe(false);
|
||||
ext.close();
|
||||
});
|
||||
|
||||
it("prunes stale cached targets after target-not-found command errors", async () => {
|
||||
const { port, ext } = await startRelayWithExtension();
|
||||
const extQueue = createMessageQueue(ext);
|
||||
|
||||
ext.send(
|
||||
JSON.stringify({
|
||||
method: "forwardCDPEvent",
|
||||
params: {
|
||||
method: "Target.attachedToTarget",
|
||||
params: {
|
||||
sessionId: "cb-tab-1",
|
||||
targetInfo: {
|
||||
targetId: "t1",
|
||||
type: "page",
|
||||
title: "Example",
|
||||
url: "https://example.com",
|
||||
},
|
||||
waitingForDebugger: false,
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
await waitForListMatch(
|
||||
async () =>
|
||||
(await fetch(`${cdpUrl}/json/list`, {
|
||||
headers: relayAuthHeaders(cdpUrl),
|
||||
}).then((r) => r.json())) as Array<{ id?: string }>,
|
||||
(list) => list.some((target) => target.id === "t1"),
|
||||
);
|
||||
|
||||
const cdp = new WebSocket(`ws://127.0.0.1:${port}/cdp`, {
|
||||
headers: relayAuthHeaders(`ws://127.0.0.1:${port}/cdp`),
|
||||
});
|
||||
await waitForOpen(cdp);
|
||||
const cdpQueue = createMessageQueue(cdp);
|
||||
|
||||
cdp.send(
|
||||
JSON.stringify({
|
||||
id: 77,
|
||||
method: "Runtime.evaluate",
|
||||
sessionId: "cb-tab-1",
|
||||
params: { expression: "1+1" },
|
||||
}),
|
||||
);
|
||||
|
||||
let forwardedId: number | null = null;
|
||||
for (let attempt = 0; attempt < 6; attempt++) {
|
||||
const msg = JSON.parse(await extQueue.next()) as { method?: string; id?: number };
|
||||
if (msg.method === "forwardCDPCommand" && typeof msg.id === "number") {
|
||||
forwardedId = msg.id;
|
||||
break;
|
||||
}
|
||||
}
|
||||
expect(forwardedId).not.toBeNull();
|
||||
|
||||
ext.send(
|
||||
JSON.stringify({
|
||||
id: forwardedId,
|
||||
error: "No target with given id",
|
||||
}),
|
||||
);
|
||||
|
||||
let response: { id?: number; error?: { message?: string } } | null = null;
|
||||
for (let attempt = 0; attempt < 6; attempt++) {
|
||||
const msg = JSON.parse(await cdpQueue.next()) as {
|
||||
id?: number;
|
||||
error?: { message?: string };
|
||||
};
|
||||
if (msg.id === 77) {
|
||||
response = msg;
|
||||
break;
|
||||
}
|
||||
}
|
||||
expect(response?.id).toBe(77);
|
||||
expect(response?.error?.message ?? "").toContain("No target with given id");
|
||||
|
||||
const updatedList = await waitForListMatch(
|
||||
async () =>
|
||||
(await fetch(`${cdpUrl}/json/list`, {
|
||||
headers: relayAuthHeaders(cdpUrl),
|
||||
}).then((r) => r.json())) as Array<{ id?: string }>,
|
||||
(list) => list.every((target) => target.id !== "t1"),
|
||||
);
|
||||
expect(updatedList.some((target) => target.id === "t1")).toBe(false);
|
||||
|
||||
cdp.close();
|
||||
ext.close();
|
||||
});
|
||||
|
||||
it("rebroadcasts attach when a session id is reused for a new target", async () => {
|
||||
const { port, ext } = await startRelayWithExtension();
|
||||
|
||||
|
||||
@@ -367,6 +367,70 @@ export async function ensureChromeExtensionRelayServer(opts: {
|
||||
ws.send(JSON.stringify(res));
|
||||
};
|
||||
|
||||
const dropConnectedTargetSession = (sessionId: string): ConnectedTarget | undefined => {
|
||||
const existing = connectedTargets.get(sessionId);
|
||||
if (!existing) {
|
||||
return undefined;
|
||||
}
|
||||
connectedTargets.delete(sessionId);
|
||||
return existing;
|
||||
};
|
||||
|
||||
const dropConnectedTargetsByTargetId = (targetId: string): ConnectedTarget[] => {
|
||||
const removed: ConnectedTarget[] = [];
|
||||
for (const [sessionId, target] of connectedTargets) {
|
||||
if (target.targetId !== targetId) {
|
||||
continue;
|
||||
}
|
||||
connectedTargets.delete(sessionId);
|
||||
removed.push(target);
|
||||
}
|
||||
return removed;
|
||||
};
|
||||
|
||||
const broadcastDetachedTarget = (target: ConnectedTarget, targetId?: string) => {
|
||||
broadcastToCdpClients({
|
||||
method: "Target.detachedFromTarget",
|
||||
params: {
|
||||
sessionId: target.sessionId,
|
||||
targetId: targetId ?? target.targetId,
|
||||
},
|
||||
sessionId: target.sessionId,
|
||||
});
|
||||
};
|
||||
|
||||
const isMissingTargetError = (err: unknown) => {
|
||||
const message = (err instanceof Error ? err.message : String(err)).toLowerCase();
|
||||
return (
|
||||
message.includes("target not found") ||
|
||||
message.includes("no target with given id") ||
|
||||
message.includes("session not found") ||
|
||||
message.includes("cannot find session")
|
||||
);
|
||||
};
|
||||
|
||||
const pruneStaleTargetsFromCommandFailure = (cmd: CdpCommand, err: unknown) => {
|
||||
if (!isMissingTargetError(err)) {
|
||||
return;
|
||||
}
|
||||
if (cmd.sessionId) {
|
||||
const removed = dropConnectedTargetSession(cmd.sessionId);
|
||||
if (removed) {
|
||||
broadcastDetachedTarget(removed);
|
||||
return;
|
||||
}
|
||||
}
|
||||
const params = (cmd.params ?? {}) as { targetId?: unknown };
|
||||
const targetId = typeof params.targetId === "string" ? params.targetId : undefined;
|
||||
if (!targetId) {
|
||||
return;
|
||||
}
|
||||
const removedTargets = dropConnectedTargetsByTargetId(targetId);
|
||||
for (const removed of removedTargets) {
|
||||
broadcastDetachedTarget(removed, targetId);
|
||||
}
|
||||
};
|
||||
|
||||
const ensureTargetEventsForClient = (ws: WebSocket, mode: "autoAttach" | "discover") => {
|
||||
for (const target of connectedTargets.values()) {
|
||||
if (mode === "autoAttach") {
|
||||
@@ -762,7 +826,18 @@ export async function ensureChromeExtensionRelayServer(opts: {
|
||||
if (method === "Target.detachedFromTarget") {
|
||||
const detached = (params ?? {}) as DetachedFromTargetEvent;
|
||||
if (detached?.sessionId) {
|
||||
connectedTargets.delete(detached.sessionId);
|
||||
dropConnectedTargetSession(detached.sessionId);
|
||||
} else if (detached?.targetId) {
|
||||
dropConnectedTargetsByTargetId(detached.targetId);
|
||||
}
|
||||
broadcastToCdpClients({ method, params, sessionId });
|
||||
return;
|
||||
}
|
||||
|
||||
if (method === "Target.targetDestroyed" || method === "Target.targetCrashed") {
|
||||
const targetEvent = (params ?? {}) as { targetId?: string };
|
||||
if (targetEvent.targetId) {
|
||||
dropConnectedTargetsByTargetId(targetEvent.targetId);
|
||||
}
|
||||
broadcastToCdpClients({ method, params, sessionId });
|
||||
return;
|
||||
@@ -871,6 +946,7 @@ export async function ensureChromeExtensionRelayServer(opts: {
|
||||
|
||||
sendResponseToCdp(ws, { id: cmd.id, sessionId: cmd.sessionId, result });
|
||||
} catch (err) {
|
||||
pruneStaleTargetsFromCommandFailure(cmd, err);
|
||||
sendResponseToCdp(ws, {
|
||||
id: cmd.id,
|
||||
sessionId: cmd.sessionId,
|
||||
|
||||
Reference in New Issue
Block a user