From 39e973723224685763070dd4d79a80cc78afd6e9 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov <34244704+Lightning00Blade@users.noreply.github.com> Date: Wed, 31 May 2023 23:36:19 +0200 Subject: [PATCH] refactor: Deferred to a class (#10282) --- .eslintrc.js | 6 + packages/puppeteer-core/src/api/Page.ts | 15 +- packages/puppeteer-core/src/common/Browser.ts | 4 +- .../src/common/ChromeTargetManager.ts | 4 +- .../puppeteer-core/src/common/Connection.ts | 4 +- .../src/common/DeviceRequestPrompt.ts | 6 +- .../src/common/FirefoxTargetManager.ts | 4 +- packages/puppeteer-core/src/common/Frame.ts | 15 +- .../puppeteer-core/src/common/FrameTree.ts | 4 +- .../puppeteer-core/src/common/HTTPResponse.ts | 4 +- .../src/common/IsolatedWorld.ts | 8 +- .../src/common/LifecycleWatcher.ts | 19 ++- packages/puppeteer-core/src/common/Page.ts | 8 +- packages/puppeteer-core/src/common/Target.ts | 6 +- .../puppeteer-core/src/common/WaitTask.ts | 4 +- .../puppeteer-core/src/common/WebWorker.ts | 4 +- .../puppeteer-core/src/common/bidi/Page.ts | 4 +- packages/puppeteer-core/src/common/util.ts | 36 +++-- .../puppeteer-core/src/injected/Poller.ts | 8 +- .../puppeteer-core/src/injected/injected.ts | 4 +- .../src/util/DebuggableDeferred.ts | 6 +- packages/puppeteer-core/src/util/Deferred.ts | 133 ++++++++++-------- test/src/Deferred.spec.ts | 7 +- 23 files changed, 164 insertions(+), 149 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 7de1595db11..9ece15fe086 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -194,6 +194,12 @@ module.exports = { selector: "CallExpression[callee.name='require']", message: '`require` statements are not allowed. Use `import`.', }, + { + // We need this as NodeJS will run until all the timers have resolved + message: 'Use method `Deferred.race()` instead.', + selector: + 'MemberExpression[object.name="Promise"][property.name="race"]', + }, ], '@typescript-eslint/no-floating-promises': [ 'error', diff --git a/packages/puppeteer-core/src/api/Page.ts b/packages/puppeteer-core/src/api/Page.ts index 0c3621ad0e7..a48f574c770 100644 --- a/packages/puppeteer-core/src/api/Page.ts +++ b/packages/puppeteer-core/src/api/Page.ts @@ -61,7 +61,6 @@ import { import type {WebWorker} from '../common/WebWorker.js'; import {assert} from '../util/assert.js'; import {Deferred} from '../util/Deferred.js'; -import {createDeferred} from '../util/Deferred.js'; import type {Browser} from './Browser.js'; import type {BrowserContext} from './BrowserContext.js'; @@ -1638,8 +1637,8 @@ export class Page extends EventEmitter { timeout: number, closedDeferred: Deferred ): Promise { - const idleDeferred = createDeferred(); - const abortDeferred = createDeferred(); + const idleDeferred = Deferred.create(); + const abortDeferred = Deferred.create(); let idleTimer: NodeJS.Timeout | undefined; const cleanup = () => { @@ -1651,7 +1650,9 @@ export class Page extends EventEmitter { clearTimeout(idleTimer); if (networkManager.inFlightRequestsCount() === 0) { - idleTimer = setTimeout(idleDeferred.resolve, idleTime); + idleTimer = setTimeout(() => { + return idleDeferred.resolve(); + }, idleTime); } }; @@ -1676,11 +1677,7 @@ export class Page extends EventEmitter { evaluate(); - await Promise.race([ - idleDeferred.valueOrThrow(), - ...eventPromises, - closedDeferred.valueOrThrow(), - ]).then( + await Deferred.race([idleDeferred, ...eventPromises, closedDeferred]).then( r => { cleanup(); return r; diff --git a/packages/puppeteer-core/src/common/Browser.ts b/packages/puppeteer-core/src/common/Browser.ts index 3a0c835101f..9dd83035515 100644 --- a/packages/puppeteer-core/src/common/Browser.ts +++ b/packages/puppeteer-core/src/common/Browser.ts @@ -33,7 +33,7 @@ import { import {BrowserContext} from '../api/BrowserContext.js'; import {Page} from '../api/Page.js'; import {assert} from '../util/assert.js'; -import {createDeferred} from '../util/Deferred.js'; +import {Deferred} from '../util/Deferred.js'; import {ChromeTargetManager} from './ChromeTargetManager.js'; import {CDPSession, Connection, ConnectionEmittedEvents} from './Connection.js'; @@ -501,7 +501,7 @@ export class CDPBrowser extends BrowserBase { options: WaitForTargetOptions = {} ): Promise { const {timeout = 30000} = options; - const targetDeferred = createDeferred>(); + const targetDeferred = Deferred.create>(); this.on(BrowserEmittedEvents.TargetCreated, check); this.on(BrowserEmittedEvents.TargetChanged, check); diff --git a/packages/puppeteer-core/src/common/ChromeTargetManager.ts b/packages/puppeteer-core/src/common/ChromeTargetManager.ts index ee7537eb8dd..98869c578c3 100644 --- a/packages/puppeteer-core/src/common/ChromeTargetManager.ts +++ b/packages/puppeteer-core/src/common/ChromeTargetManager.ts @@ -18,7 +18,7 @@ import {Protocol} from 'devtools-protocol'; import {TargetFilterCallback} from '../api/Browser.js'; import {assert} from '../util/assert.js'; -import {createDeferred} from '../util/Deferred.js'; +import {Deferred} from '../util/Deferred.js'; import {CDPSession, Connection} from './Connection.js'; import {EventEmitter} from './EventEmitter.js'; @@ -81,7 +81,7 @@ export class ChromeTargetManager extends EventEmitter implements TargetManager { (event: Protocol.Target.DetachedFromTargetEvent) => void > = new WeakMap(); - #initializeDeferred = createDeferred(); + #initializeDeferred = Deferred.create(); #targetsIdsForInit: Set = new Set(); constructor( diff --git a/packages/puppeteer-core/src/common/Connection.ts b/packages/puppeteer-core/src/common/Connection.ts index ac09a1b5bed..def84d0fef7 100644 --- a/packages/puppeteer-core/src/common/Connection.ts +++ b/packages/puppeteer-core/src/common/Connection.ts @@ -18,7 +18,7 @@ import {Protocol} from 'devtools-protocol'; import {ProtocolMapping} from 'devtools-protocol/types/protocol-mapping.js'; import {assert} from '../util/assert.js'; -import {createDeferred, Deferred} from '../util/util.js'; +import {Deferred} from '../util/util.js'; import {ConnectionTransport} from './ConnectionTransport.js'; import {debug} from './Debug.js'; @@ -64,7 +64,7 @@ function createIncrementalIdGenerator(): GetIdFn { export class Callback { #id: number; #error = new ProtocolError(); - #deferred = createDeferred(); + #deferred = Deferred.create(); #timer?: ReturnType; #label: string; diff --git a/packages/puppeteer-core/src/common/DeviceRequestPrompt.ts b/packages/puppeteer-core/src/common/DeviceRequestPrompt.ts index 36af82c684e..9b320dfa2cb 100644 --- a/packages/puppeteer-core/src/common/DeviceRequestPrompt.ts +++ b/packages/puppeteer-core/src/common/DeviceRequestPrompt.ts @@ -18,7 +18,7 @@ import Protocol from 'devtools-protocol'; import {WaitTimeoutOptions} from '../api/Page.js'; import {assert} from '../util/assert.js'; -import {createDeferred, Deferred} from '../util/Deferred.js'; +import {Deferred} from '../util/Deferred.js'; import {CDPSession} from './Connection.js'; import {TimeoutSettings} from './TimeoutSettings.js'; @@ -151,7 +151,7 @@ export class DeviceRequestPrompt { } const {timeout = this.#timeoutSettings.timeout()} = options; - const deferred = createDeferred({ + const deferred = Deferred.create({ message: `Waiting for \`DeviceRequestPromptDevice\` failed: ${timeout}ms exceeded`, timeout, }); @@ -250,7 +250,7 @@ export class DeviceRequestPromptManager { } const {timeout = this.#timeoutSettings.timeout()} = options; - const deferred = createDeferred({ + const deferred = Deferred.create({ message: `Waiting for \`DeviceRequestPrompt\` failed: ${timeout}ms exceeded`, timeout, }); diff --git a/packages/puppeteer-core/src/common/FirefoxTargetManager.ts b/packages/puppeteer-core/src/common/FirefoxTargetManager.ts index aee0ef96b5d..384453d729e 100644 --- a/packages/puppeteer-core/src/common/FirefoxTargetManager.ts +++ b/packages/puppeteer-core/src/common/FirefoxTargetManager.ts @@ -18,7 +18,7 @@ import {Protocol} from 'devtools-protocol'; import {TargetFilterCallback} from '../api/Browser.js'; import {assert} from '../util/assert.js'; -import {createDeferred} from '../util/Deferred.js'; +import {Deferred} from '../util/Deferred.js'; import {CDPSession, Connection} from './Connection.js'; import {EventEmitter} from './EventEmitter.js'; @@ -88,7 +88,7 @@ export class FirefoxTargetManager (event: Protocol.Target.AttachedToTargetEvent) => Promise > = new WeakMap(); - #initializeDeferred = createDeferred(); + #initializeDeferred = Deferred.create(); #targetsIdsForInit: Set = new Set(); constructor( diff --git a/packages/puppeteer-core/src/common/Frame.ts b/packages/puppeteer-core/src/common/Frame.ts index 9032d53fe78..ef2c5d944f8 100644 --- a/packages/puppeteer-core/src/common/Frame.ts +++ b/packages/puppeteer-core/src/common/Frame.ts @@ -26,6 +26,7 @@ import { import {HTTPResponse} from '../api/HTTPResponse.js'; import {Page, WaitTimeoutOptions} from '../api/Page.js'; import {assert} from '../util/assert.js'; +import {Deferred} from '../util/Deferred.js'; import {isErrorLike} from '../util/ErrorLike.js'; import {CDPSession} from './Connection.js'; @@ -123,7 +124,7 @@ export class Frame extends BaseFrame { waitUntil, timeout ); - let error = await Promise.race([ + let error = await Deferred.race([ navigate( this.#client, url, @@ -134,7 +135,7 @@ export class Frame extends BaseFrame { watcher.timeoutOrTerminationPromise(), ]); if (!error) { - error = await Promise.race([ + error = await Deferred.race([ watcher.timeoutOrTerminationPromise(), ensureNewDocumentNavigation ? watcher.newDocumentNavigationPromise() @@ -197,7 +198,7 @@ export class Frame extends BaseFrame { waitUntil, timeout ); - const error = await Promise.race([ + const error = await Deferred.race([ watcher.timeoutOrTerminationPromise(), watcher.sameDocumentNavigationPromise(), watcher.newDocumentNavigationPromise(), @@ -389,8 +390,8 @@ export class Frame extends BaseFrame { return this.worlds[MAIN_WORLD].transferHandle( await this.worlds[PUPPETEER_WORLD].evaluateHandle( - async ({createDeferred}, {url, id, type, content}) => { - const deferred = createDeferred(); + async ({Deferred}, {url, id, type, content}) => { + const deferred = Deferred.create(); const script = document.createElement('script'); script.type = type; script.text = content; @@ -457,8 +458,8 @@ export class Frame extends BaseFrame { return this.worlds[MAIN_WORLD].transferHandle( await this.worlds[PUPPETEER_WORLD].evaluateHandle( - async ({createDeferred}, {url, content}) => { - const deferred = createDeferred(); + async ({Deferred}, {url, content}) => { + const deferred = Deferred.create(); let element: HTMLStyleElement | HTMLLinkElement; if (!url) { element = document.createElement('style'); diff --git a/packages/puppeteer-core/src/common/FrameTree.ts b/packages/puppeteer-core/src/common/FrameTree.ts index ca32755f3c8..6457014594b 100644 --- a/packages/puppeteer-core/src/common/FrameTree.ts +++ b/packages/puppeteer-core/src/common/FrameTree.ts @@ -15,7 +15,7 @@ */ import {Frame as BaseFrame} from '../api/Frame.js'; -import {createDeferred, Deferred} from '../util/Deferred.js'; +import {Deferred} from '../util/Deferred.js'; /** * Keeps track of the page frame tree and it's is managed by @@ -50,7 +50,7 @@ export class FrameTree { if (frame) { return Promise.resolve(frame); } - const deferred = createDeferred(); + const deferred = Deferred.create(); const callbacks = this.#waitRequests.get(frameId) || new Set>(); callbacks.add(deferred); diff --git a/packages/puppeteer-core/src/common/HTTPResponse.ts b/packages/puppeteer-core/src/common/HTTPResponse.ts index 84d5be168d1..0a01fff4a12 100644 --- a/packages/puppeteer-core/src/common/HTTPResponse.ts +++ b/packages/puppeteer-core/src/common/HTTPResponse.ts @@ -20,7 +20,7 @@ import { HTTPResponse as BaseHTTPResponse, RemoteAddress, } from '../api/HTTPResponse.js'; -import {createDeferred} from '../util/Deferred.js'; +import {Deferred} from '../util/Deferred.js'; import {CDPSession} from './Connection.js'; import {ProtocolError} from './Errors.js'; @@ -34,7 +34,7 @@ export class HTTPResponse extends BaseHTTPResponse { #client: CDPSession; #request: HTTPRequest; #contentPromise: Promise | null = null; - #bodyLoadedDeferred = createDeferred(); + #bodyLoadedDeferred = Deferred.create(); #remoteAddress: RemoteAddress; #status: number; #statusText: string; diff --git a/packages/puppeteer-core/src/common/IsolatedWorld.ts b/packages/puppeteer-core/src/common/IsolatedWorld.ts index 5481cb730c0..34610845f1c 100644 --- a/packages/puppeteer-core/src/common/IsolatedWorld.ts +++ b/packages/puppeteer-core/src/common/IsolatedWorld.ts @@ -19,7 +19,7 @@ import {Protocol} from 'devtools-protocol'; import type {ClickOptions, ElementHandle} from '../api/ElementHandle.js'; import {JSHandle} from '../api/JSHandle.js'; import {assert} from '../util/assert.js'; -import {createDeferred} from '../util/Deferred.js'; +import {Deferred} from '../util/Deferred.js'; import {Binding} from './Binding.js'; import {CDPSession} from './Connection.js'; @@ -101,7 +101,7 @@ export interface IsolatedWorldChart { export class IsolatedWorld { #frame: Frame; #document?: ElementHandle; - #context = createDeferred(); + #context = Deferred.create(); #detached = false; // Set of bindings that have been registered in the current context. @@ -144,7 +144,7 @@ export class IsolatedWorld { clearContext(): void { this.#document = undefined; - this.#context = createDeferred(); + this.#context = Deferred.create(); } setContext(context: ExecutionContext): void { @@ -304,7 +304,7 @@ export class IsolatedWorld { waitUntil, timeout ); - const error = await Promise.race([ + const error = await Deferred.race([ watcher.timeoutOrTerminationPromise(), watcher.lifecyclePromise(), ]); diff --git a/packages/puppeteer-core/src/common/LifecycleWatcher.ts b/packages/puppeteer-core/src/common/LifecycleWatcher.ts index 9cd5191bb8d..f2dfb0d1444 100644 --- a/packages/puppeteer-core/src/common/LifecycleWatcher.ts +++ b/packages/puppeteer-core/src/common/LifecycleWatcher.ts @@ -16,7 +16,7 @@ import {HTTPResponse} from '../api/HTTPResponse.js'; import {assert} from '../util/assert.js'; -import {Deferred, createDeferred} from '../util/Deferred.js'; +import {Deferred} from '../util/Deferred.js'; import {CDPSessionEmittedEvents} from './Connection.js'; import {TimeoutError} from './Errors.js'; @@ -71,10 +71,10 @@ export class LifecycleWatcher { #eventListeners: PuppeteerEventListener[]; #initialLoaderId: string; - #sameDocumentNavigationDeferred = createDeferred(); - #lifecycleDeferred = createDeferred(); - #newDocumentNavigationDeferred = createDeferred(); - #terminationDeferred = createDeferred(); + #sameDocumentNavigationDeferred = Deferred.create(); + #lifecycleDeferred = Deferred.create(); + #newDocumentNavigationDeferred = Deferred.create(); + #terminationDeferred = Deferred.create(); #timeoutPromise: Promise; @@ -169,7 +169,7 @@ export class LifecycleWatcher { // navigation requests reported by the backend. This generally should not // happen by it looks like it's possible. this.#navigationResponseReceived?.resolve(); - this.#navigationResponseReceived = createDeferred(); + this.#navigationResponseReceived = Deferred.create(); if (request.response() !== null) { this.#navigationResponseReceived?.resolve(); } @@ -222,10 +222,7 @@ export class LifecycleWatcher { } timeoutOrTerminationPromise(): Promise { - return Promise.race([ - this.#timeoutPromise, - this.#terminationDeferred.valueOrThrow(), - ]); + return Deferred.race([this.#timeoutPromise, this.#terminationDeferred]); } async #createTimeoutPromise(): Promise { @@ -299,6 +296,6 @@ export class LifecycleWatcher { dispose(): void { removeEventListeners(this.#eventListeners); - this.#maximumTimer !== undefined && clearTimeout(this.#maximumTimer); + clearTimeout(this.#maximumTimer); } } diff --git a/packages/puppeteer-core/src/common/Page.ts b/packages/puppeteer-core/src/common/Page.ts index 80ca119c289..c25767a184e 100644 --- a/packages/puppeteer-core/src/common/Page.ts +++ b/packages/puppeteer-core/src/common/Page.ts @@ -43,7 +43,7 @@ import { NewDocumentScriptEvaluation, } from '../api/Page.js'; import {assert} from '../util/assert.js'; -import {createDeferred, Deferred} from '../util/Deferred.js'; +import {Deferred} from '../util/Deferred.js'; import {isErrorLike} from '../util/ErrorLike.js'; import {Accessibility} from './Accessibility.js'; @@ -153,7 +153,7 @@ export class CDPPage extends Page { #screenshotTaskQueue: TaskQueue; #workers = new Map(); #fileChooserDeferreds = new Set>(); - #sessionCloseDeferred = createDeferred(); + #sessionCloseDeferred = Deferred.create(); #serviceWorkerBypassed = false; #userDragInterceptionEnabled = false; @@ -374,7 +374,7 @@ export class CDPPage extends Page { ): Promise { const needsEnable = this.#fileChooserDeferreds.size === 0; const {timeout = this.#timeoutSettings.timeout()} = options; - const deferred = createDeferred({ + const deferred = Deferred.create({ message: `Waiting for \`FileChooser\` failed: ${timeout}ms exceeded`, timeout, }); @@ -1029,7 +1029,7 @@ export class CDPPage extends Page { }; } - const eventRace: Promise = Promise.race([ + const eventRace: Promise = Deferred.race([ waitForEvent( this.#frameManager, FrameManagerEmittedEvents.FrameAttached, diff --git a/packages/puppeteer-core/src/common/Target.ts b/packages/puppeteer-core/src/common/Target.ts index 4089dfff3cb..4a9745e6741 100644 --- a/packages/puppeteer-core/src/common/Target.ts +++ b/packages/puppeteer-core/src/common/Target.ts @@ -19,7 +19,7 @@ import {Protocol} from 'devtools-protocol'; import type {Browser} from '../api/Browser.js'; import type {BrowserContext} from '../api/BrowserContext.js'; import {Page, PageEmittedEvents} from '../api/Page.js'; -import {createDeferred} from '../util/Deferred.js'; +import {Deferred} from '../util/Deferred.js'; import {CDPSession} from './Connection.js'; import {CDPPage} from './Page.js'; @@ -55,11 +55,11 @@ export class Target { /** * @internal */ - _initializedDeferred = createDeferred(); + _initializedDeferred = Deferred.create(); /** * @internal */ - _isClosedDeferred = createDeferred(); + _isClosedDeferred = Deferred.create(); /** * @internal */ diff --git a/packages/puppeteer-core/src/common/WaitTask.ts b/packages/puppeteer-core/src/common/WaitTask.ts index 18c5617f742..17595c10742 100644 --- a/packages/puppeteer-core/src/common/WaitTask.ts +++ b/packages/puppeteer-core/src/common/WaitTask.ts @@ -17,7 +17,7 @@ import {ElementHandle} from '../api/ElementHandle.js'; import {JSHandle} from '../api/JSHandle.js'; import type {Poller} from '../injected/Poller.js'; -import {createDeferred} from '../util/Deferred.js'; +import {Deferred} from '../util/Deferred.js'; import {isErrorLike} from '../util/ErrorLike.js'; import {stringifyFunction} from '../util/Function.js'; @@ -49,7 +49,7 @@ export class WaitTask { #timeout?: NodeJS.Timeout; - #result = createDeferred>(); + #result = Deferred.create>(); #poller?: JSHandle>; #signal?: AbortSignal; diff --git a/packages/puppeteer-core/src/common/WebWorker.ts b/packages/puppeteer-core/src/common/WebWorker.ts index 8b08233b876..9ff3d563614 100644 --- a/packages/puppeteer-core/src/common/WebWorker.ts +++ b/packages/puppeteer-core/src/common/WebWorker.ts @@ -15,7 +15,7 @@ */ import {Protocol} from 'devtools-protocol'; -import {createDeferred} from '../util/Deferred.js'; +import {Deferred} from '../util/Deferred.js'; import {CDPSession} from './Connection.js'; import {ConsoleMessageType} from './ConsoleMessage.js'; @@ -68,7 +68,7 @@ export type ExceptionThrownCallback = ( * @public */ export class WebWorker extends EventEmitter { - #executionContext = createDeferred(); + #executionContext = Deferred.create(); #client: CDPSession; #url: string; diff --git a/packages/puppeteer-core/src/common/bidi/Page.ts b/packages/puppeteer-core/src/common/bidi/Page.ts index fdef22f4fb4..c02d0bd6fbc 100644 --- a/packages/puppeteer-core/src/common/bidi/Page.ts +++ b/packages/puppeteer-core/src/common/bidi/Page.ts @@ -25,7 +25,7 @@ import { WaitForOptions, } from '../../api/Page.js'; import {assert} from '../../util/assert.js'; -import {createDeferred} from '../../util/Deferred.js'; +import {Deferred} from '../../util/Deferred.js'; import {ConsoleMessage, ConsoleMessageLocation} from '../ConsoleMessage.js'; import {TargetCloseError} from '../Errors.js'; import {Handler} from '../EventEmitter.js'; @@ -61,7 +61,7 @@ export class Page extends PageBase { #frameTree = new FrameTree(); #networkManager: NetworkManager; #viewport: Viewport | null = null; - #closedDeferred = createDeferred(); + #closedDeferred = Deferred.create(); #subscribedEvents = new Map>([ ['log.entryAdded', this.#onLogEntryAdded.bind(this)], [ diff --git a/packages/puppeteer-core/src/common/util.ts b/packages/puppeteer-core/src/common/util.ts index 8c51bbe60da..9ed8f7299a9 100644 --- a/packages/puppeteer-core/src/common/util.ts +++ b/packages/puppeteer-core/src/common/util.ts @@ -22,8 +22,8 @@ import type {ElementHandle} from '../api/ElementHandle.js'; import type {JSHandle} from '../api/JSHandle.js'; import {Page} from '../api/Page.js'; import {isNode} from '../environment.js'; +import {Deferred} from '../puppeteer-core.js'; import {assert} from '../util/assert.js'; -import {createDeferred} from '../util/Deferred.js'; import {isErrorLike} from '../util/ErrorLike.js'; import type {CDPSession} from './Connection.js'; @@ -382,7 +382,7 @@ export async function waitForEvent( timeout: number, abortPromise: Promise ): Promise { - const deferred = createDeferred({ + const deferred = Deferred.create({ message: `Timeout exceeded while waiting for event ${String(eventName)}`, timeout, }); @@ -391,23 +391,19 @@ export async function waitForEvent( deferred.resolve(event); } }); - return Promise.race([deferred.valueOrThrow(), abortPromise]) - .then( - r => { - removeEventListeners([listener]); - if (isErrorLike(r)) { - throw r; - } - return r; - }, - error => { - removeEventListeners([listener]); - throw error; + return Deferred.race([deferred, abortPromise]).then( + r => { + removeEventListeners([listener]); + if (isErrorLike(r)) { + throw r; } - ) - .finally(() => { - deferred.reject(new Error('Cleared')); - }); + return r; + }, + error => { + removeEventListeners([listener]); + throw error; + } + ); } /** @@ -509,12 +505,12 @@ export async function waitWithTimeout( taskName: string, timeout: number ): Promise { - const deferred = createDeferred({ + const deferred = Deferred.create({ message: `waiting for ${taskName} failed: timeout ${timeout}ms exceeded`, timeout, }); - return await Promise.race([promise, deferred.valueOrThrow()]).finally(() => { + return await Deferred.race([promise, deferred.valueOrThrow()]).finally(() => { deferred.reject(new Error('Cleared')); }); } diff --git a/packages/puppeteer-core/src/injected/Poller.ts b/packages/puppeteer-core/src/injected/Poller.ts index a175e3f62a6..d1851cd4472 100644 --- a/packages/puppeteer-core/src/injected/Poller.ts +++ b/packages/puppeteer-core/src/injected/Poller.ts @@ -15,7 +15,7 @@ */ import {assert} from '../util/assert.js'; -import {createDeferred, Deferred} from '../util/Deferred.js'; +import {Deferred} from '../util/Deferred.js'; /** * @internal @@ -42,7 +42,7 @@ export class MutationPoller implements Poller { } async start(): Promise { - const deferred = (this.#deferred = createDeferred()); + const deferred = (this.#deferred = Deferred.create()); const result = await this.#fn(); if (result) { deferred.resolve(result); @@ -92,7 +92,7 @@ export class RAFPoller implements Poller { } async start(): Promise { - const deferred = (this.#deferred = createDeferred()); + const deferred = (this.#deferred = Deferred.create()); const result = await this.#fn(); if (result) { deferred.resolve(result); @@ -143,7 +143,7 @@ export class IntervalPoller implements Poller { } async start(): Promise { - const deferred = (this.#deferred = createDeferred()); + const deferred = (this.#deferred = Deferred.create()); const result = await this.#fn(); if (result) { deferred.resolve(result); diff --git a/packages/puppeteer-core/src/injected/injected.ts b/packages/puppeteer-core/src/injected/injected.ts index e3107ebdfa9..f7408848378 100644 --- a/packages/puppeteer-core/src/injected/injected.ts +++ b/packages/puppeteer-core/src/injected/injected.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import {createDeferred} from '../util/Deferred.js'; +import {Deferred} from '../util/Deferred.js'; import {createFunction} from '../util/Function.js'; import * as ARIAQuerySelector from './ARIAQuerySelector.js'; @@ -41,7 +41,7 @@ const PuppeteerUtil = Object.freeze({ ...TextQuerySelector, ...util, ...XPathQuerySelector, - createDeferred, + Deferred, createFunction, createTextContent, IntervalPoller, diff --git a/packages/puppeteer-core/src/util/DebuggableDeferred.ts b/packages/puppeteer-core/src/util/DebuggableDeferred.ts index e35cb6eeda3..b8a7d551b70 100644 --- a/packages/puppeteer-core/src/util/DebuggableDeferred.ts +++ b/packages/puppeteer-core/src/util/DebuggableDeferred.ts @@ -1,6 +1,6 @@ import {DEFERRED_PROMISE_DEBUG_TIMEOUT} from '../environment.js'; -import {Deferred, createDeferred} from './Deferred.js'; +import {Deferred} from './Deferred.js'; /** * Creates and returns a deferred promise using DEFERRED_PROMISE_DEBUG_TIMEOUT @@ -10,10 +10,10 @@ import {Deferred, createDeferred} from './Deferred.js'; */ export function createDebuggableDeferred(message: string): Deferred { if (DEFERRED_PROMISE_DEBUG_TIMEOUT > 0) { - return createDeferred({ + return Deferred.create({ message, timeout: DEFERRED_PROMISE_DEBUG_TIMEOUT, }); } - return createDeferred(); + return Deferred.create(); } diff --git a/packages/puppeteer-core/src/util/Deferred.ts b/packages/puppeteer-core/src/util/Deferred.ts index 6a27a512570..424b7433775 100644 --- a/packages/puppeteer-core/src/util/Deferred.ts +++ b/packages/puppeteer-core/src/util/Deferred.ts @@ -1,17 +1,5 @@ import {TimeoutError} from '../common/Errors.js'; -/** - * @internal - */ -export interface Deferred { - finished: () => boolean; - resolved: () => boolean; - resolve: (value: T) => void; - reject: (error: Error) => void; - value: () => T | Error | undefined; - valueOrThrow: () => Promise; -} - /** * @internal */ @@ -29,61 +17,94 @@ export interface DeferredOptions { * * @internal */ -export function createDeferred(opts?: DeferredOptions): Deferred { - let isResolved = false; - let isRejected = false; - let _value: T | Error | undefined; - let resolver: (value: void) => void; - const taskPromise = new Promise(resolve => { - resolver = resolve; +export class Deferred { + #isResolved = false; + #isRejected = false; + #value: T | Error | undefined; + #resolver: (value: void) => void = () => {}; + #taskPromise = new Promise(resolve => { + this.#resolver = resolve; }); - const timeoutId = - opts && opts.timeout > 0 - ? setTimeout(() => { - reject(new TimeoutError(opts.message)); - }, opts.timeout) - : undefined; + #timeoutId: ReturnType | undefined; - function finish(value: T | Error) { - clearTimeout(timeoutId); - _value = value; - resolver(); + constructor(opts?: DeferredOptions) { + this.#timeoutId = + opts && opts.timeout > 0 + ? setTimeout(() => { + this.reject(new TimeoutError(opts.message)); + }, opts.timeout) + : undefined; } - function resolve(value: T) { - if (isRejected || isResolved) { + #finish(value: T | Error) { + clearTimeout(this.#timeoutId); + this.#value = value; + this.#resolver(); + } + + resolve(value: T): void { + if (this.#isRejected || this.#isResolved) { return; } - isResolved = true; - finish(value); + this.#isResolved = true; + this.#finish(value); } - function reject(error: Error) { - if (isRejected || isResolved) { + reject(error: Error): void { + if (this.#isRejected || this.#isResolved) { return; } - isRejected = true; - finish(error); + this.#isRejected = true; + this.#finish(error); } - return { - resolved: () => { - return isResolved; - }, - finished: () => { - return isResolved || isRejected; - }, - resolve, - reject, - value: () => { - return _value; - }, - async valueOrThrow() { - await taskPromise; - if (isRejected) { - throw _value; + resolved(): boolean { + return this.#isResolved; + } + + finished(): boolean { + return this.#isResolved || this.#isRejected; + } + + value(): T | Error | undefined { + return this.#value; + } + + async valueOrThrow(): Promise { + await this.#taskPromise; + if (this.#isRejected) { + throw this.#value; + } + return this.#value as T; + } + + static create(opts?: DeferredOptions): Deferred { + return new Deferred(opts); + } + + static async race( + awaitables: Array | Deferred> + ): Promise { + const deferredWithTimeout: Set> = new Set(); + try { + const promises = awaitables.map(value => { + if (value instanceof Deferred) { + deferredWithTimeout.add(value); + + return value.valueOrThrow(); + } + + return value; + }); + // eslint-disable-next-line no-restricted-syntax + return await Promise.race(promises); + } finally { + for (const deferred of deferredWithTimeout) { + // We need to stop the timeout else + // Node.JS will keep running the event loop till the + // timer executes + deferred.reject(new Error('Timeout cleared')); } - return _value as T; - }, - }; + } + } } diff --git a/test/src/Deferred.spec.ts b/test/src/Deferred.spec.ts index e991a4436fb..3299a97e540 100644 --- a/test/src/Deferred.spec.ts +++ b/test/src/Deferred.spec.ts @@ -15,10 +15,7 @@ */ import expect from 'expect'; -import { - Deferred, - createDeferred, -} from 'puppeteer-core/internal/util/Deferred.js'; +import {Deferred} from 'puppeteer-core/internal/util/Deferred.js'; describe('DeferredPromise', function () { it('should catch errors', async () => { @@ -30,7 +27,7 @@ describe('DeferredPromise', function () { } // Async function that fails. function fails(): Deferred { - const deferred = createDeferred(); + const deferred = Deferred.create(); setTimeout(() => { deferred.reject(new Error('test')); }, 25);