Added env var support for timeouts (#1296)

* Added env var support for timeouts PERCY_NETWORK_IDLE_WAIT_TIMEOUT for Network PERCY_PAGE_LOAD_TIMEOUT for Page load * Improved tests
percy · Jul 4, 2023 · 2c42dfa · 2c42dfa
1 parent 14f2a4e
commit 2c42dfa
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 10 deletions.
diff --git a/packages/core/src/network.js b/packages/core/src/network.js
@@ -10,7 +10,7 @@ const ALLOWED_RESOURCES = ['Document', 'Stylesheet', 'Image', 'Media', 'Font', '
 // The Interceptor class creates common handlers for dealing with intercepting asset requests
 // for a given page using various devtools protocol events and commands.
 export class Network {
-  static TIMEOUT = 30000;
+  static TIMEOUT = undefined;
 
   log = logger('core:discovery');
 
@@ -29,6 +29,7 @@ export class Network {
       page.session.browser.version.userAgent.replace('Headless', '');
     this.intercept = options.intercept;
     this.meta = options.meta;
+    this._initializeNetworkIdleWaitTimeout();
   }
 
   watch(session) {
@@ -237,6 +238,18 @@ export class Network {
 
     this._forgetRequest(request);
   }
+
+  _initializeNetworkIdleWaitTimeout() {
+    if (Network.TIMEOUT) return;
+
+    Network.TIMEOUT = parseInt(process.env.PERCY_NETWORK_IDLE_WAIT_TIMEOUT) || 30000;
+
+    if (Network.TIMEOUT > 60000) {
+      this.log.warn('Setting PERCY_NETWORK_IDLE_WAIT_TIMEOUT over 60000ms is not recommended. ' +
+        'If your page needs more than 60000ms to idle due to CPU/Network load, ' +
+        'its recommended to increase CI resources where this cli is running.');
+    }
+  }
 }
 
 // Returns the normalized origin URL of a request

diff --git a/packages/core/src/page.js b/packages/core/src/page.js
@@ -10,7 +10,7 @@ import {
 } from './utils.js';
 
 export class Page {
-  static TIMEOUT = 30000;
+  static TIMEOUT = undefined;
 
   log = logger('core:page');
 
@@ -20,6 +20,7 @@ export class Page {
     this.enableJavaScript = options.enableJavaScript ?? true;
     this.network = new Network(this, options);
     this.meta = options.meta;
+    this._initializeLoadTimeout();
 
     session.on('Runtime.executionContextCreated', this._handleExecutionContextCreated);
     session.on('Runtime.executionContextDestroyed', this._handleExecutionContextDestroyed);
@@ -236,6 +237,18 @@ export class Page {
   _handleExecutionContextsCleared = () => {
     this.contextId = null;
   }
+
+  _initializeLoadTimeout() {
+    if (Page.TIMEOUT) return;
+
+    Page.TIMEOUT = parseInt(process.env.PERCY_PAGE_LOAD_TIMEOUT) || 30000;
+
+    if (Page.TIMEOUT > 60000) {
+      this.log.warn('Setting PERCY_PAGE_LOAD_TIMEOUT over 60000ms is not recommended. ' +
+        'If your page needs more than 60000ms to load due to CPU/Network load, ' +
+        'its recommended to increase CI resources where this cli is running.');
+    }
+  }
 }
 
 export default Page;
diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js
@@ -790,12 +790,12 @@ describe('Discovery', () => {
   });
 
   describe('idle timeout', () => {
-    let Network, timeout;
+    let Network;
 
     beforeEach(async () => {
       ({ Network } = await import('../src/network.js'));
-      timeout = Network.TIMEOUT;
-      Network.TIMEOUT = 500;
+      Network.TIMEOUT = undefined;
+      process.env.PERCY_NETWORK_IDLE_WAIT_TIMEOUT = 500;
 
       // some async request that takes a while
       server.reply('/img.gif', () => new Promise(r => (
@@ -806,7 +806,8 @@ describe('Discovery', () => {
     });
 
     afterEach(() => {
-      Network.TIMEOUT = timeout;
+      Network.TIMEOUT = undefined;
+      process.env.PERCY_NETWORK_IDLE_WAIT_TIMEOUT = undefined;
     });
 
     it('throws an error when requests fail to idle in time', async () => {
@@ -837,15 +838,32 @@ describe('Discovery', () => {
         '(?<stack>(.|\n)*)$'
       ].join('\n')));
     });
+
+    it('shows a warning when idle wait timeout is set over 60000ms', async () => {
+      percy.loglevel('debug');
+      process.env.PERCY_NETWORK_IDLE_WAIT_TIMEOUT = 80000;
+
+      await percy.snapshot({
+        name: 'test idle',
+        url: 'http://localhost:8000'
+      });
+
+      expect(logger.stderr).toContain(jasmine.stringMatching(
+        '^\\[percy:core:discovery] Wait for 100ms idle'
+      ));
+      expect(logger.stderr).toContain(jasmine.stringMatching(
+        '^\\[percy:core:discovery] Setting PERCY_NETWORK_IDLE_WAIT_TIMEOUT over 60000ms is not'
+      ));
+    });
   });
 
   describe('navigation timeout', () => {
-    let Page, timeout;
+    let Page;
 
     beforeEach(async () => {
       ({ Page } = await import('../src/page.js'));
-      timeout = Page.TIMEOUT;
-      Page.TIMEOUT = 500;
+      Page.TIMEOUT = undefined;
+      process.env.PERCY_PAGE_LOAD_TIMEOUT = 500;
 
       server.reply('/', () => [200, 'text/html', testDOM]);
       // trigger navigation fail
@@ -854,7 +872,8 @@ describe('Discovery', () => {
     });
 
     afterEach(() => {
-      Page.TIMEOUT = timeout;
+      Page.TIMEOUT = undefined;
+      process.env.PERCY_PAGE_LOAD_TIMEOUT = undefined;
     });
 
     it('shows debug info when navigation fails within the timeout', async () => {
@@ -874,6 +893,23 @@ describe('Discovery', () => {
         '(?<stack>(.|\n)*)$'
       ].join('\n')));
     });
+
+    it('shows a warning when page load timeout is set over 60000ms', async () => {
+      percy.loglevel('debug');
+      process.env.PERCY_PAGE_LOAD_TIMEOUT = 80000;
+
+      await percy.snapshot({
+        name: 'navigation idle',
+        url: 'http://localhost:8000'
+      });
+
+      expect(logger.stderr).toContain(jasmine.stringMatching(
+        '^\\[percy:core:discovery] Wait for 100ms idle'
+      ));
+      expect(logger.stderr).toContain(jasmine.stringMatching(
+        '^\\[percy:core:page] Setting PERCY_PAGE_LOAD_TIMEOUT over 60000ms is not recommended.'
+      ));
+    });
   });
 
   describe('cookies', () => {