figmar/BrowserSkill
1
0
Fork 0
mirror of https://github.com/Tencent/BrowserSkill.git synced 2026-09-10 13:01:23 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(extension): roll back CDP attach when a domain enable fails

Browse Source 
If `chrome.debugger.attach` succeeds but a subsequent CDP domain enable
rejects — most notably `Page.enable`, when the tab navigates to a
chrome:// URL or the Web Store in the gap between the two calls — the
driver left the tab in an unrecoverable state. `attachedTabs` never
recorded the tabId, so the next `ensureAttached` re-attached and failed
permanently with "Another debugger is already attached" until the
extension was reloaded.

Detach the debuggee directly in the failure path (the public `detach()`
is a no-op here because the id is not cached yet) so the next attempt
starts from a clean slate. Adds a regression test that fails on the
previous code, where the rollback never ran and `detach` was never
called.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Qiaochu Hu
2026-07-22 15:43:29 +08:00
co-authored by Claude
parent ccf3c32140
commit 0de67c537a
2 changed files with 44 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts
+24
View File
@@ -67,6 +67,30 @@ describe("ChromiumCdp", () => {
expect(cdp.isAttached(5)).toBe(false);
});
it("rolls back the raw attach when a domain enable fails so the tab is not left stuck", async () => {
const { api } = fakeApi();
(api.sendCommand as ReturnType<typeof vi.fn>).mockImplementation(
async (_target, method: string) => {
if (method === "Page.enable") throw new Error("Page.enable rejected");
return {};
},
);
const cdp = new ChromiumCdp(api);
// Raw attach succeeds, but Page.enable fails. Without the rollback
// the debugger would stay attached while `attachedTabs` omits the id,
// leaving the tab stuck on "Another debugger is already attached" for
// every later call until the extension is reloaded.
await expect(cdp.ensureAttached(42)).rejects.toThrow(/Page\.enable rejected/);
expect(cdp.isAttached(42)).toBe(false);
expect(api.detach).toHaveBeenCalledWith({ tabId: 42 });
// The rollback detached, so a fresh attach can succeed afterwards.
(api.sendCommand as ReturnType<typeof vi.fn>).mockResolvedValue({});
await expect(cdp.ensureAttached(42)).resolves.toBeUndefined();
expect(cdp.isAttached(42)).toBe(true);
});
it("send() rejects on chrome.runtime.lastError-style failures", async () => {
const { api } = fakeApi();
(api.sendCommand as ReturnType<typeof vi.fn>).mockImplementation(