From 36c029b38d64a10590bfc74ecea255a58914b0d2 Mon Sep 17 00:00:00 2001
From: Nikolay Vitkov <34244704+Lightning00Blade@users.noreply.github.com>
Date: Tue, 21 Mar 2023 10:22:57 +0100
Subject: [PATCH] fix: waitForNavigation issue with aborted events (#9883)

---
 .../puppeteer-core/src/common/NetworkManager.ts   |  6 +-----
 packages/puppeteer-core/src/common/Page.ts        | 12 +++---------
 test/assets/abort-request.html                    | 13 +++++++++++++
 test/src/page.spec.ts                             | 15 +++++++++++++++
 4 files changed, 32 insertions(+), 14 deletions(-)
 create mode 100644 test/assets/abort-request.html

diff --git a/packages/puppeteer-core/src/common/NetworkManager.ts b/packages/puppeteer-core/src/common/NetworkManager.ts
index b4750bef9d0..2dbcf572536 100644
--- a/packages/puppeteer-core/src/common/NetworkManager.ts
+++ b/packages/puppeteer-core/src/common/NetworkManager.ts
@@ -302,11 +302,7 @@ export class NetworkManager extends EventEmitter {
   }
 
   #onAuthRequired(event: Protocol.Fetch.AuthRequiredEvent): void {
-    /* TODO(jacktfranklin): This is defined in protocol.d.ts but not
-     * in an easily referrable way - we should look at exposing it.
-     */
-    type AuthResponse = 'Default' | 'CancelAuth' | 'ProvideCredentials';
-    let response: AuthResponse = 'Default';
+    let response: Protocol.Fetch.AuthChallengeResponse['response'] = 'Default';
     if (this.#attemptedAuthentications.has(event.requestId)) {
       response = 'CancelAuth';
     } else if (this.#credentials) {
diff --git a/packages/puppeteer-core/src/common/Page.ts b/packages/puppeteer-core/src/common/Page.ts
index fb5c00819c1..21c13514d71 100644
--- a/packages/puppeteer-core/src/common/Page.ts
+++ b/packages/puppeteer-core/src/common/Page.ts
@@ -991,10 +991,7 @@ export class CDPPage extends Page {
 
     const networkManager = this.#frameManager.networkManager;
 
-    let idleResolveCallback: () => void;
-    const idlePromise = new Promise<void>(resolve => {
-      idleResolveCallback = resolve;
-    });
+    const idlePromise = createDeferredPromise<void>();
 
     let abortRejectCallback: (error: Error) => void;
     const abortPromise = new Promise<Error>((_, reject) => {
@@ -1002,10 +999,6 @@ export class CDPPage extends Page {
     });
 
     let idleTimer: NodeJS.Timeout;
-    const onIdle = () => {
-      return idleResolveCallback();
-    };
-
     const cleanup = () => {
       idleTimer && clearTimeout(idleTimer);
       abortRejectCallback(new Error('abort'));
@@ -1014,7 +1007,7 @@ export class CDPPage extends Page {
     const evaluate = () => {
       idleTimer && clearTimeout(idleTimer);
       if (networkManager.numRequestsInProgress() === 0) {
-        idleTimer = setTimeout(onIdle, idleTime);
+        idleTimer = setTimeout(idlePromise.resolve, idleTime);
       }
     };
 
@@ -1038,6 +1031,7 @@ export class CDPPage extends Page {
     const eventPromises = [
       listenToEvent(NetworkManagerEmittedEvents.Request),
       listenToEvent(NetworkManagerEmittedEvents.Response),
+      listenToEvent(NetworkManagerEmittedEvents.RequestFailed),
     ];
 
     await Promise.race([
diff --git a/test/assets/abort-request.html b/test/assets/abort-request.html
new file mode 100644
index 00000000000..77c056a422d
--- /dev/null
+++ b/test/assets/abort-request.html
@@ -0,0 +1,13 @@
+<button id="abort"></button>
+
+<script>
+  const button = document.getElementById('abort');
+  button.addEventListener('click', getJson)
+  async function getJson() {
+    const abort = new AbortController();
+    const result = fetch("/simple.json", {
+      signal: abort.signal
+    });
+    abort.abort();
+  }
+</script>
\ No newline at end of file
diff --git a/test/src/page.spec.ts b/test/src/page.spec.ts
index 49f6e88b857..31722363809 100644
--- a/test/src/page.spec.ts
+++ b/test/src/page.spec.ts
@@ -1163,6 +1163,21 @@ describe('Page', function () {
       ]);
       expect(result).toBe(undefined);
     });
+    it('should work with aborted requests', async () => {
+      const {page, server} = getTestState();
+
+      await page.goto(server.PREFIX + '/abort-request.html');
+
+      const element = await page.$(`#abort`);
+      await element!.click();
+
+      let error = false;
+      await page.waitForNetworkIdle().catch(() => {
+        return (error = true);
+      });
+
+      expect(error).toBe(false);
+    });
   });
 
   describe('Page.exposeFunction', function () {