diff --git a/clients/static-site/README.md b/clients/static-site/README.md index 51bad105..4c013b40 100644 --- a/clients/static-site/README.md +++ b/clients/static-site/README.md @@ -141,6 +141,7 @@ Configuration is merged in this order (later overrides earlier): - `--browser-args ` - Additional Puppeteer browser arguments - `--headless` - Run browser in headless mode (default: true) - `--full-page` - Capture full page screenshots (default: false) +- `--timeout ` - Screenshot timeout in milliseconds (default: 45000) - `--dry-run` - Print discovered pages and task count without capturing screenshots - `--use-sitemap` - Use sitemap.xml for page discovery (default: true) - `--sitemap-path ` - Path to sitemap.xml relative to build directory diff --git a/clients/static-site/src/browser.js b/clients/static-site/src/browser.js index 5c139d27..aae67852 100644 --- a/clients/static-site/src/browser.js +++ b/clients/static-site/src/browser.js @@ -5,6 +5,45 @@ import puppeteer from 'puppeteer'; +/** + * Default browser args optimized for CI environments + * These reduce memory usage and improve stability in resource-constrained environments + */ +let CI_OPTIMIZED_ARGS = [ + // Required for running in containers/CI + '--no-sandbox', + '--disable-setuid-sandbox', + + // Reduce memory usage + '--disable-dev-shm-usage', // Use /tmp instead of /dev/shm (often too small in Docker) + '--disable-gpu', // No GPU in CI + '--disable-software-rasterizer', + + // Disable unnecessary features + '--disable-extensions', + '--disable-background-networking', + '--disable-background-timer-throttling', + '--disable-backgrounding-occluded-windows', + '--disable-breakpad', // Crash reporting + '--disable-component-update', + '--disable-default-apps', + '--disable-hang-monitor', + '--disable-ipc-flooding-protection', + '--disable-popup-blocking', + '--disable-prompt-on-repost', + '--disable-renderer-backgrounding', + '--disable-sync', + '--disable-translate', + + // Reduce resource usage + '--metrics-recording-only', + '--no-first-run', + '--safebrowsing-disable-auto-update', + + // Memory optimizations + '--js-flags=--max-old-space-size=512', // Limit V8 heap +]; + /** * Launch a Puppeteer browser instance * @param {Object} options - Browser launch options @@ -17,7 +56,9 @@ export async function launchBrowser(options = {}) { let browser = await puppeteer.launch({ headless, - args: ['--no-sandbox', '--disable-setuid-sandbox', ...args], + args: [...CI_OPTIMIZED_ARGS, ...args], + // Reduce protocol timeout for faster failure detection + protocolTimeout: 60_000, // 60s instead of default 180s }); return browser; diff --git a/clients/static-site/src/config-schema.js b/clients/static-site/src/config-schema.js index 9e6d2362..9482dcef 100644 --- a/clients/static-site/src/config-schema.js +++ b/clients/static-site/src/config-schema.js @@ -44,6 +44,7 @@ let browserSchema = z.object({ let screenshotSchema = z.object({ fullPage: z.boolean().default(false), omitBackground: z.boolean().default(false), + timeout: z.number().int().positive().default(45_000), // 45 seconds }); /** @@ -95,6 +96,7 @@ export let staticSiteConfigSchema = z screenshot: screenshotSchema.default({ fullPage: false, omitBackground: false, + timeout: 45_000, }), concurrency: z.number().int().positive().default(getDefaultConcurrency()), include: z.string().nullable().optional(), @@ -110,7 +112,7 @@ export let staticSiteConfigSchema = z .default({ viewports: [{ name: 'default', width: 1920, height: 1080 }], browser: { headless: true, args: [] }, - screenshot: { fullPage: false, omitBackground: false }, + screenshot: { fullPage: false, omitBackground: false, timeout: 45_000 }, concurrency: getDefaultConcurrency(), pageDiscovery: { useSitemap: true, diff --git a/clients/static-site/src/config.js b/clients/static-site/src/config.js index 1a04dc8a..1c083b62 100644 --- a/clients/static-site/src/config.js +++ b/clients/static-site/src/config.js @@ -73,6 +73,10 @@ export function parseCliOptions(options) { config.screenshot = { ...config.screenshot, fullPage: options.fullPage }; } + if (options.timeout !== undefined) { + config.screenshot = { ...config.screenshot, timeout: options.timeout }; + } + if (options.useSitemap !== undefined) { config.pageDiscovery = { ...config.pageDiscovery, diff --git a/clients/static-site/src/index.js b/clients/static-site/src/index.js index 6715ad87..3f622bdb 100644 --- a/clients/static-site/src/index.js +++ b/clients/static-site/src/index.js @@ -241,19 +241,26 @@ export async function run(buildPath, options = {}, context = {}) { } if (!isTdd && !hasToken) { - logger.error('❌ No TDD server or API token found'); - logger.info(''); - logger.info(' To capture screenshots, you need either:'); - logger.info(''); - logger.info(' 1. Start TDD server first (recommended for local dev):'); - logger.info(' vizzly tdd start'); - logger.info(' npx vizzly static-site ./dist'); - logger.info(''); - logger.info(' 2. Or set VIZZLY_TOKEN for cloud uploads:'); - logger.info( - ' VIZZLY_TOKEN=your-token npx vizzly static-site ./dist' - ); - logger.info(''); + // Use output module methods for clean formatting + let out = logger.print ? logger : null; + if (out) { + out.blank(); + out.warn('No TDD server or API token found'); + out.blank(); + out.print(' To capture screenshots, you need either:'); + out.blank(); + out.print(' 1. Start TDD server first (recommended for local dev):'); + out.hint(' vizzly tdd start'); + out.hint(' npx vizzly static-site ./dist'); + out.blank(); + out.print(' 2. Or set VIZZLY_TOKEN for cloud uploads:'); + out.hint(' VIZZLY_TOKEN=your-token npx vizzly static-site ./dist'); + out.blank(); + } else { + // Fallback for testing or when output module not available + logger.warn('No TDD server or API token found'); + logger.info('Run "vizzly tdd start" first, or set VIZZLY_TOKEN'); + } return; } diff --git a/clients/static-site/src/plugin.js b/clients/static-site/src/plugin.js index 39ec264d..803c4feb 100644 --- a/clients/static-site/src/plugin.js +++ b/clients/static-site/src/plugin.js @@ -66,6 +66,11 @@ export default { .option('--browser-args ', 'Additional Puppeteer browser arguments') .option('--headless', 'Run browser in headless mode') .option('--full-page', 'Capture full page screenshots') + .option( + '--timeout ', + 'Screenshot timeout in milliseconds (default: 45000)', + parseInt + ) .option( '--dry-run', 'Print discovered pages without capturing screenshots' diff --git a/clients/static-site/src/pool.js b/clients/static-site/src/pool.js index d11b954e..f558d112 100644 --- a/clients/static-site/src/pool.js +++ b/clients/static-site/src/pool.js @@ -4,15 +4,37 @@ */ /** - * Create a tab pool that manages browser tabs with reuse + * Default number of uses before recycling a tab + * After this many uses, the tab is closed and a fresh one created + * This prevents memory leaks from accumulating + */ +let DEFAULT_RECYCLE_AFTER = 10; + +/** + * Create a tab pool that manages browser tabs with reuse and recycling * @param {Object} browser - Puppeteer browser instance * @param {number} size - Maximum number of concurrent tabs + * @param {Object} [options] - Pool options + * @param {number} [options.recycleAfter=10] - Recycle tab after N uses * @returns {Object} Pool operations: { acquire, release, drain, stats } */ -export function createTabPool(browser, size) { +export function createTabPool(browser, size, options = {}) { + let { recycleAfter = DEFAULT_RECYCLE_AFTER } = options; + + // Track tabs with their use counts: { tab, useCount } let available = []; let waiting = []; let totalTabs = 0; + let recycledCount = 0; + + /** + * Create a fresh tab entry + * @returns {Promise} Tab entry { tab, useCount } + */ + let createTabEntry = async () => { + let tab = await browser.newPage(); + return { tab, useCount: 0 }; + }; /** * Acquire a tab from the pool @@ -27,13 +49,19 @@ export function createTabPool(browser, size) { let acquire = async () => { // Reuse existing tab if available if (available.length > 0) { - return available.pop(); + let entry = available.pop(); + entry.useCount++; + return entry.tab; } // Create new tab if under limit if (totalTabs < size) { totalTabs++; - return await browser.newPage(); + let entry = await createTabEntry(); + entry.useCount = 1; + // Store entry reference on tab for release lookup + entry.tab._poolEntry = entry; + return entry.tab; } // Wait for a tab to become available @@ -65,21 +93,58 @@ export function createTabPool(browser, size) { /** * Release a tab back to the pool * Resets tab state before reuse to prevent cross-contamination. + * Recycles (closes and replaces) tabs that have been used too many times. * If workers are waiting, hand off directly; otherwise add to available. * @param {Object} tab - Puppeteer page instance to release */ let release = async tab => { if (!tab) return; + let entry = tab._poolEntry; + + // Check if tab needs recycling + if (entry && entry.useCount >= recycleAfter) { + recycledCount++; + + // Close the old tab + try { + await tab.close(); + } catch { + // Ignore close errors + } + + // Create a fresh replacement + try { + let newEntry = await createTabEntry(); + newEntry.tab._poolEntry = newEntry; + + // Hand off to waiting worker or add to available + if (waiting.length > 0) { + newEntry.useCount = 1; + let next = waiting.shift(); + next(newEntry.tab); + } else { + available.push(newEntry); + } + } catch { + // Failed to create new tab - reduce total count + totalTabs--; + } + return; + } + // Reset tab state before reuse await resetTab(tab); // If someone is waiting, give them the tab directly if (waiting.length > 0) { + if (entry) entry.useCount++; let next = waiting.shift(); next(tab); + } else if (entry) { + available.push(entry); } else { - available.push(tab); + available.push({ tab, useCount: 0 }); } }; @@ -95,8 +160,8 @@ export function createTabPool(browser, size) { let drain = async () => { // Close all available tabs await Promise.all( - available.map(tab => - tab.close().catch(() => { + available.map(entry => + entry.tab.close().catch(() => { // Ignore close errors (tab may already be closed) }) ) @@ -114,13 +179,14 @@ export function createTabPool(browser, size) { /** * Get current pool statistics - * @returns {Object} { available, waiting, total, size } + * @returns {Object} { available, waiting, total, size, recycled } */ let stats = () => ({ available: available.length, waiting: waiting.length, total: totalTabs, size, + recycled: recycledCount, }); return { acquire, release, drain, stats }; diff --git a/clients/static-site/src/screenshot.js b/clients/static-site/src/screenshot.js index 85551d64..dbca150a 100644 --- a/clients/static-site/src/screenshot.js +++ b/clients/static-site/src/screenshot.js @@ -55,20 +55,32 @@ export function generateScreenshotProperties(viewport) { }; } +/** + * Default screenshot timeout in milliseconds (45 seconds) + * If a page can't render within this time, something is likely wrong + */ +let DEFAULT_SCREENSHOT_TIMEOUT = 45_000; + /** * Capture a screenshot from a page * @param {Object} page - Puppeteer page instance * @param {Object} options - Screenshot options * @param {boolean} [options.fullPage=false] - Capture full page * @param {boolean} [options.omitBackground=false] - Omit background + * @param {number} [options.timeout=45000] - Screenshot timeout in ms * @returns {Promise} Screenshot buffer */ export async function captureScreenshot(page, options = {}) { - let { fullPage = false, omitBackground = false } = options; + let { + fullPage = false, + omitBackground = false, + timeout = DEFAULT_SCREENSHOT_TIMEOUT, + } = options; let screenshot = await page.screenshot({ fullPage, omitBackground, + timeout, }); return screenshot; diff --git a/clients/static-site/src/tasks.js b/clients/static-site/src/tasks.js index 86615f29..42fbb62c 100644 --- a/clients/static-site/src/tasks.js +++ b/clients/static-site/src/tasks.js @@ -223,6 +223,42 @@ export async function processAllTasks(tasks, pool, config, logger, deps = {}) { // Merge deps for processTask let taskDeps = { ...defaultDeps, ...deps }; + /** + * Attempt a task with optional retry on failure + * On timeout/crash, closes the tab and retries with a fresh one + */ + let attemptTask = async (task, tab, isRetry = false) => { + try { + await processTask(tab, task, taskDeps); + return { success: true, tab }; + } catch (error) { + let isTimeout = + error.message.includes('timeout') || + error.message.includes('Timeout') || + error.message.includes('Target closed') || + error.message.includes('Protocol error'); + + // If timeout on first attempt, close bad tab and retry with fresh one + if (isTimeout && !isRetry) { + try { + await tab.close(); + } catch { + // Ignore close errors + } + + // Get a fresh tab for retry + let freshTab = await pool.acquire(); + if (!freshTab) { + return { success: false, error, tab: null }; + } + + return attemptTask(task, freshTab, true); + } + + return { success: false, error, tab, isRetry }; + } + }; + await mapWithConcurrency( tasks, async task => { @@ -239,11 +275,12 @@ export async function processAllTasks(tasks, pool, config, logger, deps = {}) { return; } - try { - await processTask(tab, task, taskDeps); + let result = await attemptTask(task, tab); + + if (result.success) { completed++; - // Track task duration for ETA calculation + // Track task duration for ETA calculation (only successful tasks) let taskDuration = Date.now() - taskStart; taskTimes.push(taskDuration); @@ -272,20 +309,27 @@ export async function processAllTasks(tasks, pool, config, logger, deps = {}) { ` ✓ [${completed}/${total}] ${task.page.path}@${task.viewport.name} ${eta}` ); } - } catch (error) { + + if (result.tab) { + pool.release(result.tab); + } + } else { completed++; + let retryNote = result.isRetry ? ' (after retry)' : ''; errors.push({ page: task.page.path, viewport: task.viewport.name, - error: error.message, + error: result.error.message + retryNote, }); output.logError( - ` ✗ ${task.page.path}@${task.viewport.name}: ${error.message}`, + ` ✗ ${task.page.path}@${task.viewport.name}: ${result.error.message}${retryNote}`, logger ); - } finally { - pool.release(tab); + + if (result.tab) { + pool.release(result.tab); + } } }, config.concurrency diff --git a/clients/static-site/tests/pool.test.js b/clients/static-site/tests/pool.test.js index a7e368d3..96605be8 100644 --- a/clients/static-site/tests/pool.test.js +++ b/clients/static-site/tests/pool.test.js @@ -279,4 +279,190 @@ describe('createTabPool', () => { assert.ok(browser.getPageCount() <= 3); }); }); + + describe('tab recycling', () => { + it('recycles tab after N uses', async () => { + let browser = createMockBrowser(); + let pool = createTabPool(browser, 1, { recycleAfter: 3 }); + + // First tab created + let tab1 = await pool.acquire(); + let originalId = tab1.id; + assert.strictEqual(browser.getNewPageCalls(), 1); + + // Use 1 + await pool.release(tab1); + + // Use 2 + let tab2 = await pool.acquire(); + assert.strictEqual(tab2.id, originalId); + await pool.release(tab2); + + // Use 3 - triggers recycling + let tab3 = await pool.acquire(); + assert.strictEqual(tab3.id, originalId); + await pool.release(tab3); + + // Now acquire should get a fresh tab (recycled) + let tab4 = await pool.acquire(); + assert.notStrictEqual(tab4.id, originalId); + assert.strictEqual(browser.getNewPageCalls(), 2); + }); + + it('tracks recycled count in stats', async () => { + let browser = createMockBrowser(); + let pool = createTabPool(browser, 1, { recycleAfter: 2 }); + + assert.strictEqual(pool.stats().recycled, 0); + + let tab = await pool.acquire(); + await pool.release(tab); // use 1 + + tab = await pool.acquire(); + await pool.release(tab); // use 2 - triggers recycle + + assert.strictEqual(pool.stats().recycled, 1); + + // Do it again + tab = await pool.acquire(); + await pool.release(tab); // use 1 + + tab = await pool.acquire(); + await pool.release(tab); // use 2 - triggers recycle + + assert.strictEqual(pool.stats().recycled, 2); + }); + + it('closes old tab during recycling', async () => { + let browser = createMockBrowser(); + let pool = createTabPool(browser, 1, { recycleAfter: 2 }); + + let tab = await pool.acquire(); + await pool.release(tab); // use 1 + + tab = await pool.acquire(); + assert.strictEqual(tab.close.mock.callCount(), 0); + + await pool.release(tab); // use 2 - triggers recycle + + assert.strictEqual(tab.close.mock.callCount(), 1); + }); + + it('hands off fresh tab to waiting acquirer during recycling', async () => { + let browser = createMockBrowser(); + let pool = createTabPool(browser, 1, { recycleAfter: 2 }); + + let tab = await pool.acquire(); + await pool.release(tab); // use 1 + + tab = await pool.acquire(); + let originalId = tab.id; + + // Someone is waiting + let acquirePromise = pool.acquire(); + + // Release triggers recycle + await pool.release(tab); // use 2 + + let newTab = await acquirePromise; + assert.notStrictEqual(newTab.id, originalId); + }); + + it('reduces total count when new tab creation fails during recycling', async () => { + let callCount = 0; + let browser = { + newPage: mock.fn(async () => { + callCount++; + if (callCount === 2) { + throw new Error('Failed to create tab'); + } + return createMockTab(callCount); + }), + }; + + let pool = createTabPool(browser, 1, { recycleAfter: 2 }); + + let tab = await pool.acquire(); + assert.strictEqual(pool.stats().total, 1); + + await pool.release(tab); // use 1 + + tab = await pool.acquire(); + await pool.release(tab); // use 2 - triggers recycle, new tab fails + + // Total should be reduced since we couldn't create replacement + assert.strictEqual(pool.stats().total, 0); + }); + + it('ignores close errors during recycling', async () => { + let browser = createMockBrowser(); + let pool = createTabPool(browser, 1, { recycleAfter: 2 }); + + let tab = await pool.acquire(); + tab.close = mock.fn(async () => { + throw new Error('Close failed'); + }); + + await pool.release(tab); // use 1 + + tab = await pool.acquire(); + tab.close = mock.fn(async () => { + throw new Error('Close failed'); + }); + + // Should not throw despite close error + await pool.release(tab); // use 2 - triggers recycle + + assert.strictEqual(pool.stats().recycled, 1); + }); + }); + + describe('_poolEntry metadata', () => { + it('preserves _poolEntry reference on tab', async () => { + let browser = createMockBrowser(); + let pool = createTabPool(browser, 2); + + let tab = await pool.acquire(); + + assert.ok(tab._poolEntry); + assert.strictEqual(tab._poolEntry.tab, tab); + assert.strictEqual(tab._poolEntry.useCount, 1); + }); + + it('increments useCount on each acquire', async () => { + let browser = createMockBrowser(); + let pool = createTabPool(browser, 1); + + let tab = await pool.acquire(); + assert.strictEqual(tab._poolEntry.useCount, 1); + + await pool.release(tab); + + tab = await pool.acquire(); + assert.strictEqual(tab._poolEntry.useCount, 2); + + await pool.release(tab); + + tab = await pool.acquire(); + assert.strictEqual(tab._poolEntry.useCount, 3); + }); + + it('increments useCount when handing off to waiter', async () => { + let browser = createMockBrowser(); + let pool = createTabPool(browser, 1); + + let tab = await pool.acquire(); + assert.strictEqual(tab._poolEntry.useCount, 1); + + // Someone is waiting + let acquirePromise = pool.acquire(); + + // Release hands off directly to waiter + pool.release(tab); + + let sameTab = await acquirePromise; + assert.strictEqual(sameTab, tab); + assert.strictEqual(tab._poolEntry.useCount, 2); + }); + }); }); diff --git a/clients/static-site/tests/tasks.test.js b/clients/static-site/tests/tasks.test.js index 949f115a..466ff44d 100644 --- a/clients/static-site/tests/tasks.test.js +++ b/clients/static-site/tests/tasks.test.js @@ -323,4 +323,375 @@ describe('processAllTasks', () => { assert.ok(maxConcurrent <= 2); }); + + describe('retry logic', () => { + it('retries with fresh tab on timeout error', async () => { + let acquireCount = 0; + let releaseCalls = []; + let closeCalls = []; + let screenshotAttempts = 0; + + let pool = { + acquire: async () => { + acquireCount++; + return { + id: acquireCount, + close: async () => closeCalls.push(acquireCount), + }; + }, + release: tab => { + releaseCalls.push(tab.id); + }, + }; + + let config = { concurrency: 1 }; + let logger = { + info: mock.fn(), + error: mock.fn(), + }; + + let deps = { + setViewport: async () => {}, + navigateToUrl: async () => {}, + captureAndSendScreenshot: async () => { + screenshotAttempts++; + if (screenshotAttempts === 1) { + throw new Error('Navigation timeout exceeded'); + } + // Second attempt succeeds + }, + }; + + let tasks = [ + { + page: { path: '/test' }, + viewport: { name: 'desktop', width: 1920, height: 1080 }, + hook: null, + url: 'http://localhost:3000/test', + screenshotOptions: {}, + }, + ]; + + let errors = await processAllTasks(tasks, pool, config, logger, deps); + + assert.strictEqual(errors.length, 0); + assert.strictEqual(screenshotAttempts, 2); + assert.strictEqual(acquireCount, 2); // Original + retry + assert.deepStrictEqual(closeCalls, [1]); // First tab closed + assert.deepStrictEqual(releaseCalls, [2]); // Second tab released + }); + + it('does not retry on non-timeout errors', async () => { + let screenshotAttempts = 0; + + let pool = { + acquire: async () => ({ id: 1, close: async () => {} }), + release: mock.fn(), + }; + + let config = { concurrency: 1 }; + let logger = { + info: mock.fn(), + error: mock.fn(), + warn: mock.fn(), + }; + + let deps = { + setViewport: async () => {}, + navigateToUrl: async () => {}, + captureAndSendScreenshot: async () => { + screenshotAttempts++; + throw new Error('Network error: DNS resolution failed'); + }, + }; + + let tasks = [ + { + page: { path: '/test' }, + viewport: { name: 'desktop', width: 1920, height: 1080 }, + hook: null, + url: 'http://localhost:3000/test', + screenshotOptions: {}, + }, + ]; + + let errors = await processAllTasks(tasks, pool, config, logger, deps); + + assert.strictEqual(errors.length, 1); + assert.strictEqual(screenshotAttempts, 1); // No retry + assert.ok(!errors[0].error.includes('after retry')); + }); + + it('does not retry if retry also fails', async () => { + let screenshotAttempts = 0; + let acquireCount = 0; + + let pool = { + acquire: async () => { + acquireCount++; + return { id: acquireCount, close: async () => {} }; + }, + release: mock.fn(), + }; + + let config = { concurrency: 1 }; + let logger = { + info: mock.fn(), + error: mock.fn(), + warn: mock.fn(), + }; + + let deps = { + setViewport: async () => {}, + navigateToUrl: async () => {}, + captureAndSendScreenshot: async () => { + screenshotAttempts++; + throw new Error('Navigation timeout exceeded'); + }, + }; + + let tasks = [ + { + page: { path: '/test' }, + viewport: { name: 'desktop', width: 1920, height: 1080 }, + hook: null, + url: 'http://localhost:3000/test', + screenshotOptions: {}, + }, + ]; + + let errors = await processAllTasks(tasks, pool, config, logger, deps); + + assert.strictEqual(errors.length, 1); + assert.strictEqual(screenshotAttempts, 2); // Original + one retry + assert.strictEqual(acquireCount, 2); + assert.ok(errors[0].error.includes('after retry')); + }); + + it('appends retry note to error message', async () => { + let pool = { + acquire: async () => ({ id: 1, close: async () => {} }), + release: mock.fn(), + }; + + let config = { concurrency: 1 }; + let logger = { + info: mock.fn(), + error: mock.fn(), + warn: mock.fn(), + }; + + let deps = { + setViewport: async () => {}, + navigateToUrl: async () => {}, + captureAndSendScreenshot: async () => { + throw new Error('Target closed'); + }, + }; + + let tasks = [ + { + page: { path: '/test' }, + viewport: { name: 'desktop', width: 1920, height: 1080 }, + hook: null, + url: 'http://localhost:3000/test', + screenshotOptions: {}, + }, + ]; + + let errors = await processAllTasks(tasks, pool, config, logger, deps); + + assert.strictEqual(errors.length, 1); + assert.ok(errors[0].error.includes('Target closed')); + assert.ok(errors[0].error.includes('(after retry)')); + }); + + it('handles Protocol error with retry', async () => { + let screenshotAttempts = 0; + + let pool = { + acquire: async () => ({ id: 1, close: async () => {} }), + release: mock.fn(), + }; + + let config = { concurrency: 1 }; + let logger = { + info: mock.fn(), + error: mock.fn(), + }; + + let deps = { + setViewport: async () => {}, + navigateToUrl: async () => {}, + captureAndSendScreenshot: async () => { + screenshotAttempts++; + if (screenshotAttempts === 1) { + throw new Error('Protocol error: Session closed'); + } + }, + }; + + let tasks = [ + { + page: { path: '/test' }, + viewport: { name: 'desktop', width: 1920, height: 1080 }, + hook: null, + url: 'http://localhost:3000/test', + screenshotOptions: {}, + }, + ]; + + let errors = await processAllTasks(tasks, pool, config, logger, deps); + + assert.strictEqual(errors.length, 0); + assert.strictEqual(screenshotAttempts, 2); + }); + + it('closes bad tab before getting fresh one', async () => { + let events = []; + + let pool = { + acquire: async () => { + events.push('acquire'); + return { + id: events.filter(e => e === 'acquire').length, + close: async () => events.push('close'), + }; + }, + release: () => events.push('release'), + }; + + let config = { concurrency: 1 }; + let logger = { + info: mock.fn(), + error: mock.fn(), + }; + + let firstAttempt = true; + let deps = { + setViewport: async () => {}, + navigateToUrl: async () => {}, + captureAndSendScreenshot: async () => { + if (firstAttempt) { + firstAttempt = false; + throw new Error('Timeout waiting for function'); + } + }, + }; + + let tasks = [ + { + page: { path: '/test' }, + viewport: { name: 'desktop', width: 1920, height: 1080 }, + hook: null, + url: 'http://localhost:3000/test', + screenshotOptions: {}, + }, + ]; + + await processAllTasks(tasks, pool, config, logger, deps); + + // Verify order: acquire -> close (bad tab) -> acquire (fresh) -> release + assert.strictEqual(events[0], 'acquire'); + assert.strictEqual(events[1], 'close'); + assert.strictEqual(events[2], 'acquire'); + assert.strictEqual(events[3], 'release'); + }); + + it('handles pool returning null during retry', async () => { + let acquireCount = 0; + + let pool = { + acquire: async () => { + acquireCount++; + if (acquireCount === 2) { + return null; // Pool drained during retry + } + return { id: acquireCount, close: async () => {} }; + }, + release: mock.fn(), + }; + + let config = { concurrency: 1 }; + let logger = { + info: mock.fn(), + error: mock.fn(), + warn: mock.fn(), + }; + + let deps = { + setViewport: async () => {}, + navigateToUrl: async () => {}, + captureAndSendScreenshot: async () => { + throw new Error('Navigation timeout'); + }, + }; + + let tasks = [ + { + page: { path: '/test' }, + viewport: { name: 'desktop', width: 1920, height: 1080 }, + hook: null, + url: 'http://localhost:3000/test', + screenshotOptions: {}, + }, + ]; + + let errors = await processAllTasks(tasks, pool, config, logger, deps); + + assert.strictEqual(errors.length, 1); + assert.ok(errors[0].error.includes('Navigation timeout')); + }); + + it('ignores tab close errors before retry', async () => { + let acquireCount = 0; + + let pool = { + acquire: async () => { + acquireCount++; + return { + id: acquireCount, + close: async () => { + throw new Error('Tab already closed'); + }, + }; + }, + release: mock.fn(), + }; + + let config = { concurrency: 1 }; + let logger = { + info: mock.fn(), + error: mock.fn(), + }; + + let firstAttempt = true; + let deps = { + setViewport: async () => {}, + navigateToUrl: async () => {}, + captureAndSendScreenshot: async () => { + if (firstAttempt) { + firstAttempt = false; + throw new Error('timeout exceeded'); + } + }, + }; + + let tasks = [ + { + page: { path: '/test' }, + viewport: { name: 'desktop', width: 1920, height: 1080 }, + hook: null, + url: 'http://localhost:3000/test', + screenshotOptions: {}, + }, + ]; + + // Should not throw despite close error + let errors = await processAllTasks(tasks, pool, config, logger, deps); + + assert.strictEqual(errors.length, 0); + assert.strictEqual(acquireCount, 2); + }); + }); });