Implement browser restart functionality on disconnection and enhance retry logic in discovery

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);

packages/core/src/config.js

@@ -417,7 +417,7 @@ export const configSchema = {
       },
       retry: {
         type: 'boolean',
-        default: false
+        default: true
       },
       launchOptions: {
         type: 'object',

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']

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;

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 = '<html><body><p>Test</p></body></html>';
+      // 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

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 += '<img src="/img.png"/>';
-      }
+      },
+      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);