fix: remove deferred promise timeouts (#8835)

Issue #8832 makes a good point that we should not be making
implicit assumptions about the client's performance
when waiting for internal events. At the same time,
we want to be able to get the debug info if some promises
never resolve because of missing backend events.

This PR adds a variable to turn on timeouts for deferred
promises created using `createDebuggableDeferredPromise`.
We can use it in our tests to catch never-resolving
promises or when reproducing bug reports where Puppeteer
hangs indefinitely.

Closes #8832

package.json

@@ -30,11 +30,11 @@
     "test": "c8 --check-coverage --lines 93 run-s test:chrome:* test:firefox",
     "test:types": "tsd",
     "test:install": "scripts/test-install.sh",
-    "test:firefox": "cross-env PUPPETEER_PRODUCT=firefox MOZ_WEBRENDER=0 mocha",
+    "test:firefox": "cross-env PUPPETEER_PRODUCT=firefox MOZ_WEBRENDER=0 PUPPETEER_DEFERRED_PROMISE_DEBUG_TIMEOUT=20000 mocha",
     "test:chrome": "run-s test:chrome:*",
-    "test:chrome:headless": "cross-env HEADLESS=true mocha",
-    "test:chrome:headless-chrome": "cross-env HEADLESS=chrome mocha",
-    "test:chrome:headful": "cross-env HEADLESS=false mocha",
+    "test:chrome:headless": "cross-env HEADLESS=true PUPPETEER_DEFERRED_PROMISE_DEBUG_TIMEOUT=20000 mocha",
+    "test:chrome:headless-chrome": "cross-env HEADLESS=chrome PUPPETEER_DEFERRED_PROMISE_DEBUG_TIMEOUT=20000 mocha",
+    "test:chrome:headful": "cross-env HEADLESS=false PUPPETEER_DEFERRED_PROMISE_DEBUG_TIMEOUT=20000 mocha",
     "prepublishOnly": "npm run build",
     "prepare": "node typescript-if-required.js && husky install",
     "lint": "run-s lint:prettier lint:eslint",

src/common/FrameManager.ts

@@ -17,7 +17,7 @@
 import {Protocol} from 'devtools-protocol';
 import {assert} from '../util/assert.js';
 import {
-  createDeferredPromiseWithTimer,
+  createDebuggableDeferredPromise,
   DeferredPromise,
 } from '../util/DeferredPromise.js';
 import {isErrorLike} from '../util/ErrorLike.js';
@@ -150,7 +150,7 @@ export class FrameManager extends EventEmitter {
       if (!this.#framesPendingTargetInit.has(targetId)) {
         this.#framesPendingTargetInit.set(
           targetId,
-          createDeferredPromiseWithTimer(
+          createDebuggableDeferredPromise(
             `Waiting for target frame ${targetId} failed`
           )
         );
@@ -318,7 +318,7 @@ export class FrameManager extends EventEmitter {
       if (!this.#framesPendingAttachment.has(frameId)) {
         this.#framesPendingAttachment.set(
           frameId,
-          createDeferredPromiseWithTimer(
+          createDebuggableDeferredPromise(
             `Waiting for frame ${frameId} to attach failed`
           )
         );

src/common/NetworkManager.ts

@@ -24,7 +24,7 @@ import {HTTPResponse} from './HTTPResponse.js';
 import {FetchRequestId, NetworkEventManager} from './NetworkEventManager.js';
 import {debugError, isString} from './util.js';
 import {
-  createDeferredPromiseWithTimer,
+  createDebuggableDeferredPromise,
   DeferredPromise,
 } from '../util/DeferredPromise.js';
 
@@ -144,9 +144,8 @@ export class NetworkManager extends EventEmitter {
     if (this.#deferredInitPromise) {
       return this.#deferredInitPromise;
     }
-    this.#deferredInitPromise = createDeferredPromiseWithTimer<void>(
-      'NetworkManager initialization timed out',
-      30000
+    this.#deferredInitPromise = createDebuggableDeferredPromise<void>(
+      'NetworkManager initialization timed out'
     );
     const init = Promise.all([
       this.#ignoreHTTPSErrors

src/environment.ts

@@ -18,3 +18,12 @@
  * @internal
  */
 export const isNode = !!(typeof process !== 'undefined' && process.version);
+
+/**
+ * @internal
+ */
+export const deferredPromiseDebugTimeout =
+  typeof process !== 'undefined' &&
+  typeof process.env['PUPPETEER_DEFERRED_PROMISE_DEBUG_TIMEOUT'] !== 'undefined'
+    ? Number(process.env['PUPPETEER_DEFERRED_PROMISE_DEBUG_TIMEOUT'])
+    : -1;

src/util/DeferredPromise.ts

@@ -1,15 +1,16 @@
 import {TimeoutError} from '../common/Errors.js';
+import {deferredPromiseDebugTimeout} from '../environment.js';
 
 /**
  * @internal
  */
-
 export interface DeferredPromise<T> extends Promise<T> {
   finished: () => boolean;
   resolved: () => boolean;
   resolve: (_: T) => void;
   reject: (_: Error) => void;
 }
+
 /**
  * Creates an returns a promise along with the resolve/reject functions.
  *
@@ -18,7 +19,6 @@ export interface DeferredPromise<T> extends Promise<T> {
  *
  * @internal
  */
-
 export function createDeferredPromiseWithTimer<T>(
   timeoutMessage: string,
   timeout = 5000
@@ -54,12 +54,12 @@ export function createDeferredPromiseWithTimer<T>(
     },
   });
 }
+
 /**
  * Creates an returns a promise along with the resolve/reject functions.
  *
  * @internal
  */
-
 export function createDeferredPromise<T>(): DeferredPromise<T> {
   let isResolved = false;
   let isRejected = false;
@@ -86,3 +86,18 @@ export function createDeferredPromise<T>(): DeferredPromise<T> {
     },
   });
 }
+
+/**
+ * @internal
+ */
+export function createDebuggableDeferredPromise<T>(
+  timeoutMessage: string
+): DeferredPromise<T> {
+  if (deferredPromiseDebugTimeout > 0) {
+    return createDeferredPromiseWithTimer(
+      timeoutMessage,
+      deferredPromiseDebugTimeout
+    );
+  }
+  return createDeferredPromise();
+}