From 2dad162a870819449a9862937d008018778fc1b2 Mon Sep 17 00:00:00 2001 From: Qiaochu Hu <110803307+hobostay@users.noreply.github.com> Date: Wed, 22 Jul 2026 20:12:51 +0800 Subject: [PATCH] fix(extension): retry console capture when a domain enable fails `enableConsoleDomains` marked the tab in `consoleDomainsAttemptedTabs` before issuing `Runtime.enable`/`Log.enable` and swallowed any errors, so a transient failure (e.g. a restricted page during attach) permanently disabled console capture for that tab: every later `ensureConsoleCapture` was a no-op and the agent silently got an empty console buffer with no indication that capture was broken. This was inconsistent with `enableNetworkDomain`, which records the tab only after `Network.enable` succeeds (and is explicitly tested as "retries and surfaces failures"). Bring `enableConsoleDomains` in line: mark the tab only after both domains enable successfully, so a later `ensureConsoleCapture` can recover from a transient failure. Renamed the flag `consoleDomainsAttemptedTabs` -> `consoleDomainsEnabledTabs` to match `networkDomainsEnabledTabs` and reflect the new semantics. Attach remains best-effort (it still succeeds when a domain can't be enabled). Adds a regression test asserting `ensureConsoleCapture` retries after a failed enable; it fails (no retry) on the previous code. Co-Authored-By: Claude --- .../__tests__/chromium-cdp.test.ts | 26 +++++++++++++++++-- .../src/browser-driver/chromium-cdp.ts | 19 ++++++++++---- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts b/apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts index f6d88d3..e7477a7 100644 --- a/apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts +++ b/apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts @@ -132,11 +132,33 @@ describe("ChromiumCdp", () => { expect(cdp.isAttached(12)).toBe(true); expect(api.sendCommand).toHaveBeenCalledWith({ tabId: 12 }, "Runtime.enable", {}); expect(api.sendCommand).toHaveBeenCalledWith({ tabId: 12 }, "Log.enable", {}); + }); + + it("retries console capture after a domain enable fails during attach", async () => { + const { api } = fakeApi(); + (api.sendCommand as ReturnType).mockImplementation( + async (_target, method: string) => { + if (method === "Log.enable") throw new Error("restricted"); + return {}; + }, + ); + const cdp = new ChromiumCdp(api); + // Attach is best-effort: Log.enable fails but attach still succeeds, + // leaving the tab unmarked so capture can be retried. + await cdp.ensureAttached(12); + const afterAttach = (api.sendCommand as ReturnType).mock.calls.filter( + ([, method]) => method === "Runtime.enable" || method === "Log.enable", + ); + expect(afterAttach).toHaveLength(2); + + // Before the fix the "attempted" flag was set before success, so this + // was a no-op and the tab silently returned no console output forever. + // Now it retries both domains. await cdp.ensureConsoleCapture(12); - const enableCalls = (api.sendCommand as ReturnType).mock.calls.filter( + const afterRetry = (api.sendCommand as ReturnType).mock.calls.filter( ([, method]) => method === "Runtime.enable" || method === "Log.enable", ); - expect(enableCalls).toHaveLength(2); + expect(afterRetry).toHaveLength(4); }); it("retries and surfaces Network.enable failures for explicit capture", async () => { diff --git a/apps/extension/src/browser-driver/chromium-cdp.ts b/apps/extension/src/browser-driver/chromium-cdp.ts index 3848e97..bdf2e38 100644 --- a/apps/extension/src/browser-driver/chromium-cdp.ts +++ b/apps/extension/src/browser-driver/chromium-cdp.ts @@ -127,7 +127,7 @@ export class ChromiumCdp { private readonly dialogSequences = new Map(); private readonly consoleBuffers = new Map(); private readonly consoleSequences = new Map(); - private readonly consoleDomainsAttemptedTabs = new Set(); + private readonly consoleDomainsEnabledTabs = new Set(); private readonly networkBuffers = new Map(); private readonly networkSequences = new Map(); private readonly networkDomainsEnabledTabs = new Set(); @@ -313,7 +313,7 @@ export class ChromiumCdp { this.dialogSequences.clear(); this.consoleBuffers.clear(); this.consoleSequences.clear(); - this.consoleDomainsAttemptedTabs.clear(); + this.consoleDomainsEnabledTabs.clear(); this.networkBuffers.clear(); this.networkSequences.clear(); this.networkDomainsEnabledTabs.clear(); @@ -334,15 +334,24 @@ export class ChromiumCdp { } private async enableConsoleDomains(tabId: number): Promise { - if (this.consoleDomainsAttemptedTabs.has(tabId)) return; - this.consoleDomainsAttemptedTabs.add(tabId); + if (this.consoleDomainsEnabledTabs.has(tabId)) return; + // Mark the tab only after both domains enable successfully — a + // transient failure (e.g. restricted page during attach) must leave + // the tab unmarked so a later `ensureConsoleCapture` can retry, + // instead of silently returning no console output forever. Mirrors + // `enableNetworkDomain`, which records the tab after success only. + let failed = false; for (const method of ["Runtime.enable", "Log.enable"]) { try { await this.api.sendCommand({ tabId }, method, {}); } catch (err) { + failed = true; console.debug("[bsk cdp] console domain enable failed", { tabId, method, err }); } } + if (!failed) { + this.consoleDomainsEnabledTabs.add(tabId); + } } private async enableNetworkDomain(tabId: number): Promise { @@ -445,7 +454,7 @@ export class ChromiumCdp { private clearConsoleState(tabId: number): void { this.consoleBuffers.delete(tabId); this.consoleSequences.delete(tabId); - this.consoleDomainsAttemptedTabs.delete(tabId); + this.consoleDomainsEnabledTabs.delete(tabId); } private bindNetworkHandler(): void {