Merge pull request #39 from hobostay/fix/console-capture-retry-on-failure

fix(extension): retry console capture when a domain enable fails
LGTM

apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts

@@ -156,11 +156,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", {});
-    await cdp.ensureConsoleCapture(12);
-    const enableCalls = (api.sendCommand as ReturnType<typeof vi.fn>).mock.calls.filter(
+  });
+
+  it("retries console capture after a domain enable fails during attach", async () => {
+    const { api } = fakeApi();
+    (api.sendCommand as ReturnType<typeof vi.fn>).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<typeof vi.fn>).mock.calls.filter(
       ([, method]) => method === "Runtime.enable" || method === "Log.enable",
     );
-    expect(enableCalls).toHaveLength(2);
+    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 afterRetry = (api.sendCommand as ReturnType<typeof vi.fn>).mock.calls.filter(
+      ([, method]) => method === "Runtime.enable" || method === "Log.enable",
+    );
+    expect(afterRetry).toHaveLength(4);
   });
 
   it("retries and surfaces Network.enable failures for explicit capture", async () => {

apps/extension/src/browser-driver/chromium-cdp.ts

@@ -144,7 +144,7 @@ export class ChromiumCdp {
   private readonly dialogSequences = new Map<number, number>();
   private readonly consoleBuffers = new Map<number, ConsoleEntry[]>();
   private readonly consoleSequences = new Map<number, number>();
-  private readonly consoleDomainsAttemptedTabs = new Set<number>();
+  private readonly consoleDomainsEnabledTabs = new Set<number>();
   private readonly networkBuffers = new Map<number, NetworkEntry[]>();
   private readonly networkSequences = new Map<number, number>();
   private readonly networkDomainsEnabledTabs = new Set<number>();
@@ -373,7 +373,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();
@@ -394,15 +394,24 @@ export class ChromiumCdp {
   }
 
   private async enableConsoleDomains(tabId: number): Promise<void> {
-    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<void> {
@@ -505,7 +514,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 {