From 9f5cb670e81d67218a84848f87408eaa743ff8e6 Mon Sep 17 00:00:00 2001 From: jrandolf <101637635+jrandolf@users.noreply.github.com> Date: Tue, 30 Aug 2022 16:24:51 +0200 Subject: [PATCH] chore: consolidate deferred promise (#8863) * fix: forward timeout waitForFileChooser * chore: consolidate deferred promise Co-authored-by: Alex Rudenko --- src/common/FrameManager.ts | 14 ++-- src/common/NetworkManager.ts | 8 +-- src/common/Page.ts | 9 +-- src/environment.ts | 2 +- src/util/DeferredPromise.ts | 133 +++++++++++++---------------------- 5 files changed, 65 insertions(+), 101 deletions(-) diff --git a/src/common/FrameManager.ts b/src/common/FrameManager.ts index e027cc62cb3..2f2e011d45e 100644 --- a/src/common/FrameManager.ts +++ b/src/common/FrameManager.ts @@ -17,7 +17,7 @@ import {Protocol} from 'devtools-protocol'; import {assert} from '../util/assert.js'; import { - createDebuggableDeferredPromise, + createDeferredPromise, DeferredPromise, } from '../util/DeferredPromise.js'; import {isErrorLike} from '../util/ErrorLike.js'; @@ -150,9 +150,9 @@ export class FrameManager extends EventEmitter { if (!this.#framesPendingTargetInit.has(targetId)) { this.#framesPendingTargetInit.set( targetId, - createDebuggableDeferredPromise( - `Waiting for target frame ${targetId} failed` - ) + createDeferredPromise({ + message: `Waiting for target frame ${targetId} failed`, + }) ); } const result = await Promise.all([ @@ -318,9 +318,9 @@ export class FrameManager extends EventEmitter { if (!this.#framesPendingAttachment.has(frameId)) { this.#framesPendingAttachment.set( frameId, - createDebuggableDeferredPromise( - `Waiting for frame ${frameId} to attach failed` - ) + createDeferredPromise({ + message: `Waiting for frame ${frameId} to attach failed`, + }) ); } frame.then(() => { diff --git a/src/common/NetworkManager.ts b/src/common/NetworkManager.ts index 4aaacd4c9fa..2d0a757af58 100644 --- a/src/common/NetworkManager.ts +++ b/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 { - createDebuggableDeferredPromise, + createDeferredPromise, DeferredPromise, } from '../util/DeferredPromise.js'; @@ -144,9 +144,9 @@ export class NetworkManager extends EventEmitter { if (this.#deferredInitPromise) { return this.#deferredInitPromise; } - this.#deferredInitPromise = createDebuggableDeferredPromise( - 'NetworkManager initialization timed out' - ); + this.#deferredInitPromise = createDeferredPromise({ + message: 'NetworkManager initialization timed out', + }); const init = Promise.all([ this.#ignoreHTTPSErrors ? this.#client.send('Security.setIgnoreCertificateErrors', { diff --git a/src/common/Page.ts b/src/common/Page.ts index f7e8a769efb..b4f187ec3fb 100644 --- a/src/common/Page.ts +++ b/src/common/Page.ts @@ -69,7 +69,7 @@ import { } from './util.js'; import {isErrorLike} from '../util/ErrorLike.js'; import { - createDeferredPromiseWithTimer, + createDeferredPromise, DeferredPromise, } from '../util/DeferredPromise.js'; import {WebWorker} from './WebWorker.js'; @@ -769,9 +769,10 @@ export class Page extends EventEmitter { } const {timeout = this.#timeoutSettings.timeout()} = options; - const promise = createDeferredPromiseWithTimer( - `Waiting for \`FileChooser\` failed: ${timeout}ms exceeded` - ); + const promise = createDeferredPromise({ + message: `Waiting for \`FileChooser\` failed: ${timeout}ms exceeded`, + timeout, + }); this.#fileChooserPromises.add(promise); return promise.catch(error => { this.#fileChooserPromises.delete(promise); diff --git a/src/environment.ts b/src/environment.ts index 876eac8f15d..80258c67fed 100644 --- a/src/environment.ts +++ b/src/environment.ts @@ -22,7 +22,7 @@ export const isNode = !!(typeof process !== 'undefined' && process.version); /** * @internal */ -export const deferredPromiseDebugTimeout = +export const DEFERRED_PROMISE_DEBUG_TIMEOUT = typeof process !== 'undefined' && typeof process.env['PUPPETEER_DEFERRED_PROMISE_DEBUG_TIMEOUT'] !== 'undefined' ? Number(process.env['PUPPETEER_DEFERRED_PROMISE_DEBUG_TIMEOUT']) diff --git a/src/util/DeferredPromise.ts b/src/util/DeferredPromise.ts index b6be161e379..e2350c07c2c 100644 --- a/src/util/DeferredPromise.ts +++ b/src/util/DeferredPromise.ts @@ -1,5 +1,5 @@ import {TimeoutError} from '../common/Errors.js'; -import {deferredPromiseDebugTimeout} from '../environment.js'; +import {DEFERRED_PROMISE_DEBUG_TIMEOUT} from '../environment.js'; /** * @internal @@ -11,96 +11,59 @@ export interface DeferredPromise extends Promise { reject: (_: Error) => void; } +interface DeferredPromiseOptions { + message?: string; + timeout?: number; + isDebug?: boolean; +} + /** * Creates an returns a promise along with the resolve/reject functions. * - * If the promise has not been resolved/rejected withing the `timeout` period, + * If the promise has not been resolved/rejected within the `timeout` period, * the promise gets rejected with a timeout error. * * @internal */ -export function createDeferredPromiseWithTimer( - timeoutMessage: string, - timeout = 5000 -): DeferredPromise { - let isResolved = false; - let isRejected = false; - let resolver = (_: T): void => {}; - let rejector = (_: Error) => {}; - const taskPromise = new Promise((resolve, reject) => { - resolver = resolve; - rejector = reject; - }); - const timeoutId = - timeout > 0 - ? setTimeout(() => { - isRejected = true; - rejector(new TimeoutError(timeoutMessage)); - }, timeout) - : undefined; - return Object.assign(taskPromise, { - resolved: () => { - return isResolved; - }, - finished: () => { - return isResolved || isRejected; - }, - resolve: (value: T) => { - clearTimeout(timeoutId); - isResolved = true; - resolver(value); - }, - reject: (err: Error) => { - clearTimeout(timeoutId); - isRejected = true; - rejector(err); - }, - }); -} - -/** - * Creates an returns a promise along with the resolve/reject functions. - * - * @internal - */ -export function createDeferredPromise(): DeferredPromise { - let isResolved = false; - let isRejected = false; - let resolver = (_: T): void => {}; - let rejector = (_: Error) => {}; - const taskPromise = new Promise((resolve, reject) => { - resolver = resolve; - rejector = reject; - }); - return Object.assign(taskPromise, { - resolved: () => { - return isResolved; - }, - finished: () => { - return isResolved || isRejected; - }, - resolve: (value: T) => { - isResolved = true; - resolver(value); - }, - reject: (err: Error) => { - isRejected = true; - rejector(err); - }, - }); -} - -/** - * @internal - */ -export function createDebuggableDeferredPromise( - timeoutMessage: string -): DeferredPromise { - if (deferredPromiseDebugTimeout > 0) { - return createDeferredPromiseWithTimer( - timeoutMessage, - deferredPromiseDebugTimeout - ); +export function createDeferredPromise({ + message, + timeout = 5000, +}: DeferredPromiseOptions = {}): DeferredPromise { + if (DEFERRED_PROMISE_DEBUG_TIMEOUT > 0 && !timeout) { + timeout = DEFERRED_PROMISE_DEBUG_TIMEOUT; } - return createDeferredPromise(); + let isResolved = false; + let isRejected = false; + let resolver = (_: T): void => {}; + let rejector = (_: Error) => {}; + const taskPromise = new Promise((resolve, reject) => { + resolver = resolve; + rejector = reject; + }); + const timeoutId = message + ? setTimeout(() => { + isRejected = true; + rejector(new TimeoutError(message)); + }, timeout) + : undefined; + return Object.assign(taskPromise, { + resolved: () => { + return isResolved; + }, + finished: () => { + return isResolved || isRejected; + }, + resolve: (value: T) => { + if (timeoutId) { + clearTimeout(timeoutId); + } + isResolved = true; + resolver(value); + }, + reject: (err: Error) => { + clearTimeout(timeoutId); + isRejected = true; + rejector(err); + }, + }); }