From dae34aaa21f8a566872581ac0000d92b4d015a80 Mon Sep 17 00:00:00 2001 From: DCCA Date: Sat, 27 Jun 2026 17:59:23 -0300 Subject: [PATCH] fix(capture): retry tab activation when the tab strip is busy One-click auto-capture runs while the just-opened editor tab is still being inserted into the tab strip, so chrome.tabs.update(target, {active:true}) rejected with "Tabs cannot be edited right now (user may be dragging a tab)." and capture aborted. Manual capture avoids it because the strip has settled by the time the user clicks. Add activateTab(): retry chrome.tabs.update on this transient error with a short backoff, rethrowing unrelated errors. Use it to activate the target tab and to restore the previously-active tab (best-effort). isTabsBusyError is extracted and pure; both covered by unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/lib/capture.ts | 42 ++++++++++++++++++++++++++-- tests/capture.test.ts | 64 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 103 insertions(+), 3 deletions(-) diff --git a/src/lib/capture.ts b/src/lib/capture.ts index 3ac853e..529bd99 100644 --- a/src/lib/capture.ts +++ b/src/lib/capture.ts @@ -62,6 +62,43 @@ export function isNoReceiverError(error: unknown): boolean { return /receiving end does not exist|could not establish connection/i.test(message); } +/** + * True for the transient error Chrome throws from tab edits while the tab strip + * is mid-operation (a real drag, or — for one-click capture — the strip still + * settling right after the editor tab was created). + */ +export function isTabsBusyError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error); + return /tabs cannot be edited right now|user may be dragging a tab/i.test(message); +} + +/** + * Activate a tab, retrying while the tab strip is transiently locked. Without + * this, one-click auto-capture fails with "Tabs cannot be edited right now" + * because it runs while the just-opened editor tab is still being inserted. + */ +export async function activateTab( + tabId: number, + options: { retries?: number; delayMs?: number } = {} +): Promise { + const retries = options.retries ?? 8; + const delayMs = options.delayMs ?? 150; + + let lastError: unknown; + for (let attempt = 0; attempt <= retries; attempt += 1) { + try { + await chrome.tabs.update(tabId, { active: true }); + return; + } catch (error) { + lastError = error; + if (!isTabsBusyError(error)) throw error; + await wait(delayMs); + } + } + + throw lastError instanceof Error ? lastError : new Error("Could not activate tab"); +} + /** * Send a message to a tab's content script, re-injecting it and retrying while * the receiving end is not ready. This is the difference between manual capture @@ -99,7 +136,7 @@ export async function captureFullPage( const [activeTab] = await chrome.tabs.query({ active: true, windowId }); const previousActiveTabId = activeTab?.id; - await chrome.tabs.update(tabId, { active: true }); + await activateTab(tabId); await wait(150); try { @@ -147,7 +184,8 @@ export async function captureFullPage( }; } finally { if (previousActiveTabId && previousActiveTabId !== tabId) { - await chrome.tabs.update(previousActiveTabId, { active: true }); + // Best-effort: a failed restore must not mask the capture result. + await activateTab(previousActiveTabId).catch(() => undefined); } } } diff --git a/tests/capture.test.ts b/tests/capture.test.ts index d900446..4cbfcfb 100644 --- a/tests/capture.test.ts +++ b/tests/capture.test.ts @@ -1,5 +1,11 @@ import { describe, expect, it, vi } from "vitest"; -import { buildScrollSteps, isNoReceiverError, sendToContentScript } from "../src/lib/capture"; +import { + activateTab, + buildScrollSteps, + isNoReceiverError, + isTabsBusyError, + sendToContentScript +} from "../src/lib/capture"; describe("buildScrollSteps", () => { it("returns single step when content fits viewport", () => { @@ -82,3 +88,59 @@ describe("sendToContentScript", () => { expect(sendMessage).toHaveBeenCalledTimes(3); }); }); + +describe("isTabsBusyError", () => { + it("matches the transient tab-strip-locked errors", () => { + expect( + isTabsBusyError(new Error("Tabs cannot be edited right now (user may be dragging a tab).")) + ).toBe(true); + expect(isTabsBusyError(new Error("User may be dragging a tab"))).toBe(true); + }); + + it("does not match unrelated errors", () => { + expect(isTabsBusyError(new Error("No tab with id: 7"))).toBe(false); + }); +}); + +describe("activateTab", () => { + function stubTabsUpdate(update: ReturnType) { + (globalThis as unknown as { chrome: unknown }).chrome = { tabs: { update } }; + } + + it("activates on the first try", async () => { + const update = vi.fn().mockResolvedValue(undefined); + stubTabsUpdate(update); + await expect(activateTab(5, { delayMs: 0 })).resolves.toBeUndefined(); + expect(update).toHaveBeenCalledWith(5, { active: true }); + expect(update).toHaveBeenCalledTimes(1); + }); + + it("retries while the tab strip is busy, then succeeds", async () => { + const update = vi + .fn() + .mockRejectedValueOnce( + new Error("Tabs cannot be edited right now (user may be dragging a tab).") + ) + .mockRejectedValueOnce(new Error("Tabs cannot be edited right now")) + .mockResolvedValue(undefined); + stubTabsUpdate(update); + await expect(activateTab(5, { delayMs: 0 })).resolves.toBeUndefined(); + expect(update).toHaveBeenCalledTimes(3); + }); + + it("rethrows unrelated errors immediately", async () => { + const update = vi.fn().mockRejectedValue(new Error("No tab with id: 5")); + stubTabsUpdate(update); + await expect(activateTab(5, { delayMs: 0 })).rejects.toThrow("No tab with id"); + expect(update).toHaveBeenCalledTimes(1); + }); + + it("gives up after exhausting retries", async () => { + const update = vi.fn().mockRejectedValue(new Error("Tabs cannot be edited right now")); + stubTabsUpdate(update); + await expect(activateTab(5, { retries: 2, delayMs: 0 })).rejects.toThrow( + "Tabs cannot be edited right now" + ); + expect(update).toHaveBeenCalledTimes(3); + }); +});