diff --git a/packages/core/src/browser.js b/packages/core/src/browser.js index c2aecc64f..e5828ece3 100644 --- a/packages/core/src/browser.js +++ b/packages/core/src/browser.js @@ -120,6 +120,28 @@ export class Browser extends EventEmitter { return this.ws?.readyState === WebSocket.OPEN; } + async restart() { + this.log.info('Restarting browser after disconnection'); + + // Force close the existing browser instance + /* istanbul ignore next: Hard to mock */ + if (this.readyState !== null) { + await this.close(true).catch(err => { + this.log.debug('Error during force close:', err); + }); + } + + // Reset state for fresh launch + this.readyState = null; + this._closed = null; + this.#callbacks.clear(); + this.sessions.clear(); + + // Launch a new browser instance + await this.launch(); + this.log.info('Browser restarted successfully'); + } + async close(force = false) { // Check for the new closeBrowser option if (!force && this.percy.config.discovery?.launchOptions?.closeBrowser === false) { @@ -195,6 +217,12 @@ export class Browser extends EventEmitter { } async page(options = {}) { + // Check if browser is connected, restart if needed + if (!this.isConnected()) { + this.log.warn('Browser disconnected, attempting restart'); + await this.restart(); + } + let { targetId } = await this.send('Target.createTarget', { url: '' }); let { sessionId } = await this.send('Target.attachToTarget', { targetId, flatten: true }); let page = new Page(this.sessions.get(sessionId), options); diff --git a/packages/core/src/config.js b/packages/core/src/config.js index 08a8964e8..6b58bc58f 100644 --- a/packages/core/src/config.js +++ b/packages/core/src/config.js @@ -417,7 +417,7 @@ export const configSchema = { }, retry: { type: 'boolean', - default: false + default: true }, launchOptions: { type: 'object', diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 44b7c7d21..dd916c2c7 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -488,8 +488,21 @@ export function createDiscoveryQueue(percy) { } }, { count: snapshot.discovery.retry ? 3 : 1, - onRetry: () => { + onRetry: async (error) => { percy.log.info(`Retrying snapshot: ${snapshotLogName(snapshot.name, snapshot.meta)}`, snapshot.meta); + // If browser disconnected or crashed, restart it before retrying + if (error?.message?.includes('Browser not connected') || + error?.message?.includes('Browser closed') || + error?.message?.includes('Session closed') || + error?.message?.includes('Session crashed')) { + percy.log.warn('Detected browser disconnection, restarting browser before retry'); + try { + await percy.browser.restart(); + } catch (restartError) { + percy.log.error(`Failed to restart browser: ${restartError}`); + throw restartError; + } + } }, signal: snapshot._ctrl.signal, throwOn: ['AbortError'] diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js index 3c5c3e171..34556029b 100644 --- a/packages/core/src/utils.js +++ b/packages/core/src/utils.js @@ -416,7 +416,7 @@ export async function withRetries(fn, { count, onRetry, signal, throwOn }) { // if this error should not be retried on, we want to skip errors let throwError = throwOn?.includes(e.name); if (!throwError && run < count) { - await onRetry?.(); + await onRetry?.(e); continue; } throw e; diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 5937b2404..16e8e6475 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -1461,14 +1461,13 @@ describe('Discovery', () => { delete process.env.PERCY_PAGE_LOAD_TIMEOUT; }); - it('should retry the snapshot discovery upto 3 times', async () => { + it('should retry by default on the snapshot discovery upto 3 times', async () => { // 3rd request will resolve instantly fastCount = 3; await percy.snapshot({ name: 'test navigation timeout', url: 'http://localhost:8000', - discovery: { retry: true }, widths: [400, 800] }); @@ -2603,6 +2602,187 @@ describe('Discovery', () => { }); }); + describe('Browser restart on disconnection', () => { + let testDOM; + + beforeEach(async () => { + testDOM = '
Test
'; + // ensure a new percy instance is used for each test + await percy?.stop(true); + }); + + it('restarts the browser when it is disconnected', async () => { + percy = await Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { concurrency: 1 } + }); + + const browserInstance = percy.browser; + const originalPid = browserInstance.process.pid; + + // Close the browser to simulate disconnection + await browserInstance.close(true); + expect(browserInstance.isConnected()).toBe(false); + + // Restart should launch a new browser + await browserInstance.restart(); + expect(browserInstance.isConnected()).toBe(true); + expect(browserInstance.process.pid).not.toBe(originalPid); + expect(browserInstance.readyState).toBe(1); // connected state + + await percy.stop(true); + }); + + it('restarts the browser automatically in page() when disconnected', async () => { + percy = await Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { concurrency: 1 } + }); + + const browserInstance = percy.browser; + spyOn(browserInstance, 'restart').and.callThrough(); + + // Close the browser to simulate disconnection + await browserInstance.close(true); + expect(browserInstance.isConnected()).toBe(false); + + // Creating a page should trigger auto-restart + const page = await browserInstance.page(); + expect(browserInstance.restart).toHaveBeenCalled(); + expect(browserInstance.isConnected()).toBe(true); + expect(page).toBeDefined(); + + await page.close(); + await percy.stop(true); + }); + + it('retries snapshot with browser restart on "Browser not connected" error', async () => { + percy = await Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { + concurrency: 1, + retry: true + } + }); + + const browserInstance = percy.browser; + spyOn(browserInstance, 'restart').and.callThrough(); + + let attemptCount = 0; + const originalPage = browserInstance.page.bind(browserInstance); + spyOn(browserInstance, 'page').and.callFake(async function(...args) { + attemptCount++; + if (attemptCount === 1) { + // Simulate browser crash on first attempt + await browserInstance.close(true); + throw new Error('Browser not connected'); + } + // Succeed on retry + return originalPage(...args); + }); + + await percy.snapshot({ + name: 'test snapshot', + url: 'http://localhost:8000', + domSnapshot: testDOM, + port: 0 + }); + + await percy.idle(); + + expect(browserInstance.restart).toHaveBeenCalled(); + expect(logger.stdout).toEqual(jasmine.arrayContaining([ + '[percy] Retrying snapshot: test snapshot' + ])); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Detected browser disconnection, restarting browser before retry' + ])); + await percy.stop(true); + }); + + it('retries snapshot with browser restart on "Session closed" error', async () => { + percy = await Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { + concurrency: 1 + } + }); + + const browserInstance = percy.browser; + spyOn(browserInstance, 'restart').and.callThrough(); + + let attemptCount = 0; + const originalPage = browserInstance.page.bind(browserInstance); + spyOn(browserInstance, 'page').and.callFake(async function(...args) { + attemptCount++; + if (attemptCount === 1) { + await browserInstance.close(true); + throw new Error('Session closed'); + } + return originalPage(...args); + }); + + await percy.snapshot({ + name: 'test snapshot', + url: 'http://localhost:8000', + domSnapshot: testDOM + }); + + await percy.idle(); + + expect(browserInstance.restart).toHaveBeenCalled(); + expect(logger.stdout).toEqual(jasmine.arrayContaining([ + '[percy] Retrying snapshot: test snapshot' + ])); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Detected browser disconnection, restarting browser before retry' + ])); + await percy.stop(true); + }); + + it('logs error and throws when browser restart fails', async () => { + percy = await Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { + concurrency: 1 + } + }); + + const browserInstance = percy.browser; + spyOn(browserInstance, 'restart').and.rejectWith(new Error('Restart failed')); + + let attemptCount = 0; + spyOn(browserInstance, 'page').and.callFake(async function(...args) { + attemptCount++; + if (attemptCount === 1) { + throw new Error('Browser not connected'); + } + }); + + await percy.snapshot({ + name: 'test snapshot', + url: 'http://localhost:8000', + domSnapshot: testDOM + }); + + await percy.idle(); + + expect(browserInstance.restart).toHaveBeenCalled(); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Detected browser disconnection, restarting browser before retry', + '[percy] Failed to restart browser: Error: Restart failed', + '[percy] Encountered an error taking snapshot: test snapshot', + jasmine.stringMatching(/Error: Restart failed/) + ])); + await percy.stop(true); + }); + }); + describe('Asset Discovery Page JS =>', () => { beforeEach(() => { // global defaults diff --git a/packages/core/test/snapshot.test.js b/packages/core/test/snapshot.test.js index 6c81fedf9..883020b9c 100644 --- a/packages/core/test/snapshot.test.js +++ b/packages/core/test/snapshot.test.js @@ -1074,7 +1074,8 @@ describe('Snapshot', () => { let snap = percy.snapshot({ name: 'test snapshot', - url: 'http://localhost:8000' + url: 'http://localhost:8000', + discovery: { retry: false } }); // wait until an asset has at least been requested @@ -1101,7 +1102,8 @@ describe('Snapshot', () => { url: 'http://localhost:8000', execute: () => { document.body.innerHTML += '
';
- }
+ },
+ discovery: { retry: false }
});
// wait until the asset is requested before exiting
@@ -1119,7 +1121,8 @@ describe('Snapshot', () => {
let snap = percy.snapshot({
name: 'crash snapshot',
url: 'http://localhost:8000',
- execute: () => new Promise(r => setTimeout(r, 1000))
+ execute: () => new Promise(r => setTimeout(r, 1000)),
+ discovery: { retry: false }
});
await waitFor(() => !!percy.browser.sessions.size);