✨ Show network logs on navigation errors (#1105)

* 🔊 Show network logs on navigation errors * 🔇 Suppress discovery logs caused by page cleanup * 💚 Add coverage for timeout logs Co-authored-by: Wil Wilsman <[email protected]>
percy · Oct 20, 2022 · 3f18b69 · 3f18b69
1 parent d83865b
commit 3f18b69
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 15 deletions.
diff --git a/packages/core/src/network.js b/packages/core/src/network.js
@@ -76,23 +76,26 @@ export class Network {
       timeout: Network.TIMEOUT,
       idle: timeout
     }).catch(error => {
-      // throw a better timeout error
       if (error.message.startsWith('Timeout')) {
-        let msg = 'Timed out waiting for network requests to idle.';
-
-        if (this.log.shouldLog('debug')) {
-          msg += `\n\n  ${['Active requests:',
-            ...requests.map(r => r.url)
-          ].join('\n  - ')}\n`;
-        }
-
-        throw new Error(msg);
+        this._throwTimeoutError((
+          'Timed out waiting for network requests to idle.'
+        ), filter);
       } else {
         throw error;
       }
     });
   }
 
+  // Throw a better network timeout error
+  _throwTimeoutError(msg, filter = () => true) {
+    if (this.log.shouldLog('debug')) {
+      let reqs = Array.from(this.#requests.values()).filter(filter).map(r => r.url);
+      msg += `\n\n  ${['Active requests:', ...reqs].join('\n  - ')}\n`;
+    }
+
+    throw new Error(msg);
+  }
+
   // Called when a request should be removed from various trackers
   _forgetRequest({ requestId, interceptId }, keepPending) {
     this.#requests.delete(requestId);
@@ -276,6 +279,9 @@ async function sendResponseResource(network, request, session) {
       });
     }
   } catch (error) {
+    /* istanbul ignore next: too hard to test (create race condition) */
+    if (session.closing && error.message.includes('close')) return;
+
     log.debug(`Encountered an error handling request: ${url}`, meta);
     log.debug(error);
 

diff --git a/packages/core/src/page.js b/packages/core/src/page.js
@@ -88,12 +88,18 @@ export class Page {
         return handlers.every(handler => handler.finished);
       }, Page.TIMEOUT)]);
     } catch (error) {
-      // remove handlers and modify the error message
+      // remove any unused handlers
       for (let handler of handlers) handler.off();
 
-      throw Object.assign(error, {
-        message: `Navigation failed: ${error.message}`
-      });
+      // assign context to unknown errors
+      if (!error.message.startsWith('Timeout')) {
+        throw Object.assign(error, { message: `Navigation failed: ${error.message}` });
+      }
+
+      // throw a network error to show active requests
+      this.network._throwTimeoutError(
+        `Navigation failed: Timed out waiting for the page ${waitUntil} event`
+      );
     }
 
     this.log.debug('Page navigated', this.meta);

diff --git a/packages/core/src/session.js b/packages/core/src/session.js
@@ -22,7 +22,8 @@ export class Session extends EventEmitter {
   }
 
   async close() {
-    if (!this.browser) return;
+    if (!this.browser || this.closing) return;
+    this.closing = true;
 
     await this.browser.send('Target.closeTarget', {
       targetId: this.targetId

diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js
@@ -807,6 +807,43 @@ describe('Discovery', () => {
     });
   });
 
+  describe('navigation timeout', () => {
+    let Page, timeout;
+
+    beforeEach(async () => {
+      ({ Page } = await import('../src/page.js'));
+      timeout = Page.TIMEOUT;
+      Page.TIMEOUT = 500;
+
+      server.reply('/', () => [200, 'text/html', testDOM]);
+      // trigger navigation fail
+      server.reply('/img.gif', () => new Promise(r => (
+        setTimeout(r, 3000, [200, 'image/gif', pixel]))));
+    });
+
+    afterEach(() => {
+      Page.TIMEOUT = timeout;
+    });
+
+    it('shows debug info when navigation fails within the timeout', async () => {
+      percy.loglevel('debug');
+
+      await percy.snapshot({
+        name: 'navigation idle',
+        url: 'http://localhost:8000'
+      });
+
+      expect(logger.stderr).toContain(jasmine.stringMatching([
+        '^\\[percy:core] Error: Navigation failed: Timed out waiting for the page load event',
+        '',
+        '  Active requests:',
+        '  - http://localhost:8000/img.gif',
+        '',
+        '(?<stack>(.|\n)*)$'
+      ].join('\n')));
+    });
+  });
+
   describe('cookies', () => {
     let cookie;