Merge pull request #9 from ArchiveBox/fix/chrome-lifecycle-isolation

Fix chrome lifecycle consistency and test isolation

Author: pirate

abx_plugins/plugins/accessibility/on_Snapshot__39_accessibility.js

@@ -27,7 +27,7 @@ const {
 ensureNodeModuleResolution(module);
 const puppeteer = require('puppeteer-core');
 const {
-    readCdpUrl,
+    waitForChromeSession,
     connectToPage,
     waitForPageLoaded,
 } = require('../chrome/chrome_utils.js');
@@ -52,7 +52,7 @@ async function extractAccessibility(url, timeoutMs) {
     let browser = null;
 
     try {
-        if (!readCdpUrl(CHROME_SESSION_DIR)) {
+        if (!(await waitForChromeSession(CHROME_SESSION_DIR, Math.min(timeoutMs, 1000), true))) {
             return { success: false, error: 'No Chrome session found (chrome plugin must run first)' };
         }
 

abx_plugins/plugins/chrome/README.md

@@ -95,6 +95,8 @@ This directory does **not** own a separate browser. It stores the per-snapshot t
 
 The snapshot-level `cdp_url.txt` and `chrome.pid` are copies of the crawl session values. The snapshot-level `target_id.txt` is unique per snapshot.
 
+Snapshot hooks should treat `SNAP_DIR/chrome/` as their entire Chrome state surface. They should not reach back into `CRAWL_DIR/chrome/` directly; if they need a tab, target, navigation record, or copied extension metadata, they should consume the snapshot-level markers and let the core Chrome hooks handle any crawl-to-snapshot propagation.
+
 ## Readiness Lifecycle
 
 ### 1. Extension install hooks run before Chrome launch

abx_plugins/plugins/chrome/chrome_utils.js

@@ -524,14 +524,19 @@ async function launchChromium(options = {}) {
     // Write command script for debugging
     writeCmdScript(path.join(outputDir, 'cmd.sh'), binary, chromiumArgs);
 
+    let chromiumProcess = null;
+    let chromePid = null;
+    let recentStderr = '';
+    let recentStdout = '';
+
     try {
         console.error(`[*] Spawning Chromium (headless=${headless})...`);
-        const chromiumProcess = spawn(binary, chromiumArgs, {
+        chromiumProcess = spawn(binary, chromiumArgs, {
             stdio: ['ignore', 'pipe', 'pipe'],
             detached: true,
         });
 
-        const chromePid = chromiumProcess.pid;
+        chromePid = chromiumProcess.pid;
         const chromeStartTime = Date.now() / 1000;
 
         if (chromePid) {
@@ -541,16 +546,35 @@ async function launchChromium(options = {}) {
 
         // Pipe Chrome output to stderr
         chromiumProcess.stdout.on('data', (data) => {
+            recentStdout = `${recentStdout}${String(data)}`.slice(-4000);
             process.stderr.write(`[chromium:stdout] ${data}`);
         });
         chromiumProcess.stderr.on('data', (data) => {
+            recentStderr = `${recentStderr}${String(data)}`.slice(-4000);
             process.stderr.write(`[chromium:stderr] ${data}`);
         });
 
+        const chromiumExit = new Promise((_, reject) => {
+            chromiumProcess.once('error', (error) => {
+                reject(new Error(`Chromium process failed to start: ${error.message}`));
+            });
+            chromiumProcess.once('exit', (code, signal) => {
+                reject(new Error(
+                    `Chromium exited before opening the debug port (code=${code ?? 'null'}, signal=${signal || 'none'})`
+                ));
+            });
+        });
+        // Suppress unhandled rejection if chromiumExit loses the race but
+        // fires later when the browser eventually shuts down.
+        chromiumExit.catch(() => {});
+
         // Wait for debug port
         console.error(`[*] Waiting for debug port ${debugPort}...`);
         const debugProbeTimeoutMs = getEnvInt('CHROME_DEBUG_PORT_TIMEOUT_MS', 30000);
-        const versionInfo = await waitForDebugPort(debugPort, debugProbeTimeoutMs);
+        const versionInfo = await Promise.race([
+            waitForDebugPort(debugPort, debugProbeTimeoutMs),
+            chromiumExit,
+        ]);
         const wsUrl = versionInfo.webSocketDebuggerUrl;
 
         console.error(`[+] Chromium ready: ${wsUrl}`);
@@ -584,7 +608,19 @@ async function launchChromium(options = {}) {
 
         return result;
     } catch (e) {
-        return { success: false, error: `${e.name}: ${e.message}` };
+        if (chromePid) {
+            await cleanupLaunchArtifacts(outputDir, chromePid);
+        }
+        const extraOutput = [
+            recentStdout ? `stdout=${recentStdout.trim()}` : '',
+            recentStderr ? `stderr=${recentStderr.trim()}` : '',
+        ].filter(Boolean).join(' ');
+        return {
+            success: false,
+            error: extraOutput
+                ? `${e.name}: ${e.message} (${extraOutput})`
+                : `${e.name}: ${e.message}`,
+        };
     }
 }
 
@@ -1591,10 +1627,15 @@ function findChromium() {
             if (validateBinary(c)) return c;
         }
         // Also search puppeteer cache under LIB_DIR
-        const libPuppeteerDir = path.join(libDir, 'puppeteer', 'chrome');
-        const libPuppeteerBinary = findInPuppeteerDir(libPuppeteerDir);
-        if (libPuppeteerBinary && validateBinary(libPuppeteerBinary)) {
-            return libPuppeteerBinary;
+        const libPuppeteerDirs = [
+            path.join(libDir, 'puppeteer', 'chromium'),
+            path.join(libDir, 'puppeteer', 'chrome'),
+        ];
+        for (const libPuppeteerDir of libPuppeteerDirs) {
+            const libPuppeteerBinary = findInPuppeteerDir(libPuppeteerDir);
+            if (libPuppeteerBinary && validateBinary(libPuppeteerBinary)) {
+                return libPuppeteerBinary;
+            }
         }
     }
 
@@ -2212,14 +2253,24 @@ async function withConnectedBrowser(options, operation) {
 async function setBrowserDownloadBehavior(options = {}) {
     const {
         browser,
+        page,
         downloadPath,
     } = options;
 
-    if (!browser || !downloadPath) return false;
+    if (!browser && !page) {
+        throw new Error('setBrowserDownloadBehavior requires a browser or page');
+    }
+    if (!downloadPath) {
+        throw new Error('setBrowserDownloadBehavior requires downloadPath');
+    }
 
     await fs.promises.mkdir(downloadPath, { recursive: true });
-    const session = await browser.target().createCDPSession();
+    const sessionTarget = page ? page.target() : browser.target();
+    const session = await sessionTarget.createCDPSession();
 
+    // Keep the CDP session alive for the lifetime of the caller's browser/page
+    // connection. Extension-driven downloads regress if we detach immediately
+    // after configuring download behavior.
     try {
         await session.send('Browser.setDownloadBehavior', {
             behavior: 'allow',
@@ -2242,7 +2293,7 @@ async function setBrowserDownloadBehavior(options = {}) {
     }
 }
 
-function fetchDevtoolsTargets(cdpUrl) {
+function fetchDevtoolsTargets(cdpUrl, timeoutMs = 5000) {
     const port = getChromeDebugPortFromCdpUrl(cdpUrl);
     if (!port) {
         return Promise.resolve([]);
@@ -2264,11 +2315,14 @@ function fetchDevtoolsTargets(cdpUrl) {
                 });
             }
         );
+        req.setTimeout(timeoutMs, () => {
+            req.destroy(new Error(`Timed out fetching DevTools targets after ${timeoutMs}ms`));
+        });
         req.on('error', reject);
     });
 }
 
-function devtoolsHttpRequest(cdpUrl, requestPath, method = 'GET') {
+function devtoolsHttpRequest(cdpUrl, requestPath, method = 'GET', timeoutMs = 5000) {
     const port = getChromeDebugPortFromCdpUrl(cdpUrl);
     if (!port) {
         return Promise.reject(new Error(`Invalid CDP URL: ${cdpUrl}`));
@@ -2289,14 +2343,18 @@ function devtoolsHttpRequest(cdpUrl, requestPath, method = 'GET') {
                 });
             }
         );
+        req.setTimeout(timeoutMs, () => {
+            req.destroy(new Error(`Timed out waiting for DevTools ${method} ${requestPath} after ${timeoutMs}ms`));
+        });
         req.on('error', reject);
         req.end();
     });
 }
 
-async function createDevtoolsPageTarget(cdpUrl, initialUrl = 'about:blank') {
+async function createDevtoolsPageTarget(cdpUrl, initialUrl = 'about:blank', options = {}) {
+    const timeoutMs = options.timeoutMs || 5000;
     const encodedUrl = encodeURIComponent(initialUrl);
-    const response = await devtoolsHttpRequest(cdpUrl, `/json/new?${encodedUrl}`, 'PUT');
+    const response = await devtoolsHttpRequest(cdpUrl, `/json/new?${encodedUrl}`, 'PUT', timeoutMs);
     const target = JSON.parse(response || '{}');
     if (!target?.id) {
         throw new Error('Failed to create DevTools page target');
@@ -2648,15 +2706,38 @@ async function getBrowserServerUrl(chromeSessionDir = '../chrome', options = {})
  * @returns {Promise<{targetId: string}>}
  */
 async function openTabInChromeSession(options = {}) {
-    const { cdpUrl, puppeteer } = options;
+    const {
+        cdpUrl,
+        puppeteer,
+        timeoutMs = 10000,
+        intervalMs = 250,
+    } = options;
     if (!cdpUrl) {
         throw new Error(CHROME_SESSION_REQUIRED_ERROR);
     }
     if (puppeteer) {
         requirePuppeteerModule(puppeteer, 'openTabInChromeSession');
     }
-    const target = await createDevtoolsPageTarget(cdpUrl, 'about:blank');
-    return { targetId: target.id };
+    const deadline = Date.now() + Math.max(timeoutMs, 0);
+    let lastError = null;
+
+    while (Date.now() <= deadline) {
+        try {
+            const requestTimeoutMs = Math.max(1000, Math.min(5000, deadline - Date.now()));
+            const target = await createDevtoolsPageTarget(cdpUrl, 'about:blank', {
+                timeoutMs: requestTimeoutMs,
+            });
+            return { targetId: target.id };
+        } catch (error) {
+            lastError = error;
+            if (Date.now() >= deadline) {
+                break;
+            }
+            await sleep(intervalMs);
+        }
+    }
+
+    throw lastError || new Error('Failed to create DevTools page target');
 }
 
 /**

abx_plugins/plugins/chrome/on_Snapshot__10_chrome_tab.daemon.bg.js

@@ -246,6 +246,7 @@ async function main() {
         const opened = await openTabInChromeSession({
             cdpUrl: crawlSession.cdpUrl,
             puppeteer,
+            timeoutMs: timeoutSeconds * 1000,
         });
         targetId = opened.targetId;
         if (!targetId) {

abx_plugins/plugins/chrome/tests/chrome_test_helpers.py

@@ -409,12 +409,15 @@ def _coerce_upstream_urls(value: Any) -> Optional[Dict[str, str]]:
 @pytest.fixture(scope="session")
 def ensure_chromium_and_puppeteer_installed(tmp_path_factory):
     """Install Chromium and Puppeteer once for test sessions that require Chrome."""
-    if not os.environ.get("SNAP_DIR"):
-        os.environ["SNAP_DIR"] = str(tmp_path_factory.mktemp("chrome_test_data"))
-    if not os.environ.get("PERSONAS_DIR"):
-        os.environ["PERSONAS_DIR"] = str(
-            tmp_path_factory.mktemp("chrome_test_personas")
-        )
+    os.environ["SNAP_DIR"] = str(tmp_path_factory.mktemp("chrome_test_data"))
+    os.environ["PERSONAS_DIR"] = str(tmp_path_factory.mktemp("chrome_test_personas"))
+    os.environ["HOME"] = str(tmp_path_factory.mktemp("chrome_test_home"))
+    os.environ["XDG_CONFIG_HOME"] = str(Path(os.environ["HOME"]) / ".config")
+    os.environ["XDG_CACHE_HOME"] = str(Path(os.environ["HOME"]) / ".cache")
+    os.environ["XDG_DATA_HOME"] = str(Path(os.environ["HOME"]) / ".local" / "share")
+
+    for key in ("HOME", "XDG_CONFIG_HOME", "XDG_CACHE_HOME", "XDG_DATA_HOME"):
+        Path(os.environ[key]).mkdir(parents=True, exist_ok=True)
 
     env = get_test_env()
     chromium_binary = install_chromium_with_hooks(env)
@@ -1026,12 +1029,14 @@ def install_chromium_with_hooks(env: dict, timeout: int = 300) -> str:
         records = parse_jsonl_records(result.stdout)
         chromium_record = None
         for record in records:
-            if record.get("type") == "Binary" and record.get("name") in (
-                "chromium",
-                "chrome",
-            ):
+            if record.get("type") == "Binary" and record.get("name") == "chromium":
                 chromium_record = record
                 break
+        if not chromium_record:
+            for record in records:
+                if record.get("type") == "Binary" and record.get("name") == "chrome":
+                    chromium_record = record
+                    break
         if not chromium_record:
             chromium_record = parse_jsonl_output(result.stdout, record_type="Binary")
         if not chromium_record:
@@ -1116,13 +1121,21 @@ def setup_test_env(tmpdir: Path) -> dict:
     node_modules_dir = npm_dir / "node_modules"
 
     personas_dir = tmpdir / "personas"
+    home_dir = tmpdir / "home"
+    xdg_config_home = home_dir / ".config"
+    xdg_cache_home = home_dir / ".cache"
+    xdg_data_home = home_dir / ".local" / "share"
     chrome_extensions_dir = personas_dir / "Default" / "chrome_extensions"
     chrome_downloads_dir = personas_dir / "Default" / "chrome_downloads"
     chrome_user_data_dir = personas_dir / "Default" / "chrome_user_data"
 
     # Create all directories
     node_modules_dir.mkdir(parents=True, exist_ok=True)
     npm_bin_dir.mkdir(parents=True, exist_ok=True)
+    home_dir.mkdir(parents=True, exist_ok=True)
+    xdg_config_home.mkdir(parents=True, exist_ok=True)
+    xdg_cache_home.mkdir(parents=True, exist_ok=True)
+    xdg_data_home.mkdir(parents=True, exist_ok=True)
     chrome_extensions_dir.mkdir(parents=True, exist_ok=True)
     chrome_downloads_dir.mkdir(parents=True, exist_ok=True)
     chrome_user_data_dir.mkdir(parents=True, exist_ok=True)
@@ -1141,6 +1154,10 @@ def setup_test_env(tmpdir: Path) -> dict:
             "MACHINE_TYPE": machine_type,
             "NPM_BIN_DIR": str(npm_bin_dir),
             "NODE_MODULES_DIR": str(node_modules_dir),
+            "HOME": str(home_dir),
+            "XDG_CONFIG_HOME": str(xdg_config_home),
+            "XDG_CACHE_HOME": str(xdg_cache_home),
+            "XDG_DATA_HOME": str(xdg_data_home),
             "CHROME_EXTENSIONS_DIR": str(chrome_extensions_dir),
             "CHROME_DOWNLOADS_DIR": str(chrome_downloads_dir),
             "CHROME_USER_DATA_DIR": str(chrome_user_data_dir),
@@ -1481,6 +1498,10 @@ def chrome_session(
         crawl_dir = tmpdir / "crawl" / crawl_id
         snap_dir = tmpdir / "snap" / snapshot_id
         personas_dir = tmpdir / "personas"
+        home_dir = tmpdir / "home"
+        xdg_config_home = home_dir / ".config"
+        xdg_cache_home = home_dir / ".cache"
+        xdg_data_home = home_dir / ".local" / "share"
         env = os.environ.copy()
 
         # Prefer an already-provisioned NODE_MODULES_DIR (set by session-level chrome fixture)
@@ -1507,6 +1528,10 @@ def chrome_session(
         # Build env with tmpdir-specific paths
         snap_dir.mkdir(parents=True, exist_ok=True)
         personas_dir.mkdir(parents=True, exist_ok=True)
+        home_dir.mkdir(parents=True, exist_ok=True)
+        xdg_config_home.mkdir(parents=True, exist_ok=True)
+        xdg_cache_home.mkdir(parents=True, exist_ok=True)
+        xdg_data_home.mkdir(parents=True, exist_ok=True)
 
         env.update(
             {
@@ -1518,6 +1543,10 @@ def chrome_session(
                 "NODE_MODULES_DIR": str(node_modules_dir),
                 "NODE_PATH": str(node_modules_dir),
                 "NPM_BIN_DIR": str(npm_dir / ".bin"),
+                "HOME": str(home_dir),
+                "XDG_CONFIG_HOME": str(xdg_config_home),
+                "XDG_CACHE_HOME": str(xdg_cache_home),
+                "XDG_DATA_HOME": str(xdg_data_home),
                 "CHROME_HEADLESS": "true",
                 "PUPPETEER_CACHE_DIR": str(puppeteer_cache_dir),
             }