diff --git a/src/browser.test.ts b/src/browser.test.ts index 55e483df..37a3fea3 100644 --- a/src/browser.test.ts +++ b/src/browser.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach, vi } from 'vitest'; import { BrowserManager } from './browser.js'; import { chromium } from 'playwright-core'; @@ -422,6 +422,158 @@ describe('BrowserManager', () => { }); }); + describe('CDP connection resilience', () => { + let connectSpy: ReturnType; + + const createMockBrowser = ( + options: { + contexts?: Array<{ + pages: () => Array<{ url: () => string; on: ReturnType }>; + on: ReturnType; + }>; + } = {} + ) => ({ + contexts: () => + options.contexts ?? [ + { + pages: () => [{ url: () => 'http://example.com', on: vi.fn() }], + on: vi.fn(), + }, + ], + close: vi.fn().mockResolvedValue(undefined), + isConnected: vi.fn().mockReturnValue(true), + }); + + afterEach(() => { + if (connectSpy) { + connectSpy.mockRestore(); + } + }); + + it('should accept cdpUrl as full WebSocket URL', async () => { + const mockBrowser = createMockBrowser(); + connectSpy = vi.spyOn(chromium, 'connectOverCDP').mockResolvedValue(mockBrowser as any); + + const cdpBrowser = new BrowserManager(); + await cdpBrowser.launch({ + id: 'test', + action: 'launch', + cdpUrl: 'ws://localhost:9222/devtools/browser/abc123', + }); + + expect(connectSpy).toHaveBeenCalledWith('ws://localhost:9222/devtools/browser/abc123'); + await cdpBrowser.close(); + }); + + it('should accept cdpUrl as HTTP URL', async () => { + const mockBrowser = createMockBrowser(); + connectSpy = vi.spyOn(chromium, 'connectOverCDP').mockResolvedValue(mockBrowser as any); + + const cdpBrowser = new BrowserManager(); + await cdpBrowser.launch({ id: 'test', action: 'launch', cdpUrl: 'http://remote-host:9222' }); + + expect(connectSpy).toHaveBeenCalledWith('http://remote-host:9222'); + await cdpBrowser.close(); + }); + + it('should convert numeric cdpPort to localhost URL', async () => { + const mockBrowser = createMockBrowser(); + connectSpy = vi.spyOn(chromium, 'connectOverCDP').mockResolvedValue(mockBrowser as any); + + const cdpBrowser = new BrowserManager(); + await cdpBrowser.launch({ id: 'test', action: 'launch', cdpPort: 9222 }); + + expect(connectSpy).toHaveBeenCalledWith('http://localhost:9222'); + await cdpBrowser.close(); + }); + + it('should retry on transient connection failures', async () => { + const mockBrowser = createMockBrowser(); + + let callCount = 0; + connectSpy = vi.spyOn(chromium, 'connectOverCDP').mockImplementation(async () => { + callCount++; + if (callCount < 3) { + throw new Error('Connection refused'); + } + return mockBrowser as any; + }); + + const cdpBrowser = new BrowserManager(); + await cdpBrowser.launch({ id: 'test', action: 'launch', cdpPort: 9222 }); + + // Should have retried and succeeded on 3rd attempt + expect(callCount).toBe(3); + expect(cdpBrowser.isLaunched()).toBe(true); + + await cdpBrowser.close(); + }); + + it('should not retry on validation errors (no browser context)', async () => { + const mockBrowser = createMockBrowser({ contexts: [] }); + + connectSpy = vi.spyOn(chromium, 'connectOverCDP').mockResolvedValue(mockBrowser as any); + + const cdpBrowser = new BrowserManager(); + await expect( + cdpBrowser.launch({ id: 'test', action: 'launch', cdpPort: 9222 }) + ).rejects.toThrow('No browser context'); + + // Should only have tried once (no retries for validation errors) + expect(connectSpy).toHaveBeenCalledTimes(1); + }); + + it('should not retry on validation errors (no page found)', async () => { + const mockBrowser = createMockBrowser({ + contexts: [ + { + pages: () => [], // No pages - validation error + on: vi.fn(), + }, + ], + }); + + connectSpy = vi.spyOn(chromium, 'connectOverCDP').mockResolvedValue(mockBrowser as any); + + const cdpBrowser = new BrowserManager(); + await expect( + cdpBrowser.launch({ id: 'test', action: 'launch', cdpPort: 9222 }) + ).rejects.toThrow('No page found'); + + // Should only have tried once + expect(connectSpy).toHaveBeenCalledTimes(1); + }); + + it('should provide helpful error message after all retries fail', async () => { + connectSpy = vi + .spyOn(chromium, 'connectOverCDP') + .mockRejectedValue(new Error('Connection refused')); + + const cdpBrowser = new BrowserManager(); + await expect( + cdpBrowser.launch({ id: 'test', action: 'launch', cdpPort: 9222 }) + ).rejects.toThrow(/Failed to connect via CDP.*after 3 attempts/); + + // Should have tried 3 times (default retries) + expect(connectSpy).toHaveBeenCalledTimes(3); + }); + + it('should mention remote URL in error message for non-localhost connections', async () => { + connectSpy = vi + .spyOn(chromium, 'connectOverCDP') + .mockRejectedValue(new Error('Connection refused')); + + const cdpBrowser = new BrowserManager(); + await expect( + cdpBrowser.launch({ + id: 'test', + action: 'launch', + cdpUrl: 'ws://remote-server:9222/devtools', + }) + ).rejects.toThrow(/remote browser is accessible/); + }); + }); + describe('screencast', () => { it('should report screencasting state correctly', () => { expect(browser.isScreencasting()).toBe(false); diff --git a/src/browser.ts b/src/browser.ts index 04cfa66a..5c4e1fd1 100644 --- a/src/browser.ts +++ b/src/browser.ts @@ -17,6 +17,10 @@ import { import path from 'node:path'; import os from 'node:os'; import { existsSync, mkdirSync, rmSync } from 'node:fs'; +import { exec } from 'node:child_process'; +import { promisify } from 'node:util'; + +const execAsync = promisify(exec); import type { LaunchCommand } from './types.js'; import { type RefMap, type EnhancedSnapshot, getEnhancedSnapshot, parseRef } from './snapshot.js'; @@ -923,11 +927,46 @@ export class BrowserManager { this.setupPageTracking(page); } + /** + * Check if Chrome/Chromium browser is currently running + * (not just Chrome-related helper processes like crashpad handlers) + */ + private async isChromeRunning(): Promise { + try { + if (process.platform === 'win32') { + const { stdout } = await execAsync('tasklist /FI "IMAGENAME eq chrome.exe"'); + return stdout.toLowerCase().includes('chrome.exe'); + } else { + // Look for actual Chrome/Chromium browser processes, excluding helpers + // -x matches exact process name (not substrings in command line) + const { stdout } = await execAsync( + 'pgrep -x "chrome|chromium|chromium-browser|google-chrome" || true' + ); + return stdout.trim().length > 0; + } + } catch { + return false; // Can't determine, assume not running + } + } + + /** + * Sleep for a given number of milliseconds + */ + private sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); + } + /** * Connect to a running browser via CDP (Chrome DevTools Protocol) * @param cdpEndpoint Either a port number (as string) or a full WebSocket URL (ws:// or wss://) + * @param retries - Number of retry attempts (default: 3) + * @param retryDelayMs - Delay between retries in milliseconds (default: 1000) */ - private async connectViaCDP(cdpEndpoint: string | undefined): Promise { + private async connectViaCDP( + cdpEndpoint: string | undefined, + retries: number = 3, + retryDelayMs: number = 1000 + ): Promise { if (!cdpEndpoint) { throw new Error('CDP endpoint is required for CDP connection'); } @@ -952,15 +991,63 @@ export class BrowserManager { cdpUrl = `http://localhost:${cdpEndpoint}`; } - const browser = await chromium.connectOverCDP(cdpUrl).catch(() => { - throw new Error( - `Failed to connect via CDP to ${cdpUrl}. ` + - (cdpUrl.includes('localhost') - ? `Make sure the app is running with --remote-debugging-port=${cdpEndpoint}` - : 'Make sure the remote browser is accessible and the URL is correct.') - ); - }); + let lastError: Error | null = null; + + for (let attempt = 1; attempt <= retries; attempt++) { + try { + const browser = await chromium.connectOverCDP(cdpUrl); + // Connection successful - validate and set up state + await this.setupCDPConnection(browser, cdpEndpoint); + return; + } catch (error) { + lastError = error instanceof Error ? error : new Error(String(error)); + + // Don't retry on validation errors (browser connected but no contexts/pages) + if ( + lastError.message.includes('No browser context') || + lastError.message.includes('No page found') + ) { + throw lastError; + } + + if (attempt < retries) { + // Wait before retrying + await this.sleep(retryDelayMs); + } + } + } + + // All retries failed - provide helpful error message + const chromeRunning = await this.isChromeRunning(); + const isLocalhost = cdpUrl.includes('localhost'); + + let errorMessage = `Failed to connect via CDP to ${cdpUrl} after ${retries} attempts.\n\n`; + + if (isLocalhost && chromeRunning) { + errorMessage += + `Chrome is already running but likely not with remote debugging enabled.\n` + + `This happens when Chrome opens your request in an existing session.\n\n` + + `Solutions:\n` + + ` 1. Start a separate Chrome instance with a temp profile:\n` + + ` google-chrome --remote-debugging-port=${cdpEndpoint} --user-data-dir=/tmp/chrome-cdp\n\n` + + ` 2. Or close all Chrome windows first, then:\n` + + ` google-chrome --remote-debugging-port=${cdpEndpoint}`; + } else if (isLocalhost) { + errorMessage += + `Make sure Chrome is running with remote debugging:\n` + + ` google-chrome --remote-debugging-port=${cdpEndpoint}`; + } else { + errorMessage += 'Make sure the remote browser is accessible and the URL is correct.'; + } + + throw new Error(errorMessage); + } + + /** + * Set up the CDP connection after successful connect + */ + private async setupCDPConnection(browser: Browser, cdpEndpoint: string): Promise { // Validate and set up state, cleaning up browser connection if anything fails try { const contexts = browser.contexts();