diff --git a/packages/puppeteer-core/src/common/Browser.ts b/packages/puppeteer-core/src/common/Browser.ts index fc3222b1..dff1dd53 100644 --- a/packages/puppeteer-core/src/common/Browser.ts +++ b/packages/puppeteer-core/src/common/Browser.ts @@ -360,7 +360,10 @@ export class CDPBrowser extends BrowserBase { }; #onAttachedToTarget = async (target: Target) => { - if ((await target._initializedPromise) === InitializationStatus.SUCCESS) { + if ( + (await target._initializedPromise.valueOrThrow()) === + InitializationStatus.SUCCESS + ) { this.emit(BrowserEmittedEvents.TargetCreated, target); target .browserContext() @@ -371,7 +374,10 @@ export class CDPBrowser extends BrowserBase { #onDetachedFromTarget = async (target: Target): Promise => { target._initializedPromise.resolve(InitializationStatus.ABORTED); target._isClosedPromise.resolve(); - if ((await target._initializedPromise) === InitializationStatus.SUCCESS) { + if ( + (await target._initializedPromise.valueOrThrow()) === + InitializationStatus.SUCCESS + ) { this.emit(BrowserEmittedEvents.TargetDestroyed, target); target .browserContext() @@ -432,7 +438,8 @@ export class CDPBrowser extends BrowserBase { throw new Error(`Missing target for page (id = ${targetId})`); } const initialized = - (await target._initializedPromise) === InitializationStatus.SUCCESS; + (await target._initializedPromise.valueOrThrow()) === + InitializationStatus.SUCCESS; if (!initialized) { throw new Error(`Failed to create target for page (id = ${targetId})`); } @@ -501,9 +508,13 @@ export class CDPBrowser extends BrowserBase { try { this.targets().forEach(check); if (!timeout) { - return await targetPromise; + return await targetPromise.valueOrThrow(); } - return await waitWithTimeout(targetPromise, 'target', timeout); + return await waitWithTimeout( + targetPromise.valueOrThrow(), + 'target', + timeout + ); } finally { this.off(BrowserEmittedEvents.TargetCreated, check); this.off(BrowserEmittedEvents.TargetChanged, check); diff --git a/packages/puppeteer-core/src/common/ChromeTargetManager.ts b/packages/puppeteer-core/src/common/ChromeTargetManager.ts index 65987279..b62c65a4 100644 --- a/packages/puppeteer-core/src/common/ChromeTargetManager.ts +++ b/packages/puppeteer-core/src/common/ChromeTargetManager.ts @@ -131,7 +131,7 @@ export class ChromeTargetManager extends EventEmitter implements TargetManager { autoAttach: true, }); this.#finishInitializationIfReady(); - await this.#initializePromise; + await this.#initializePromise.valueOrThrow(); } dispose(): void { diff --git a/packages/puppeteer-core/src/common/Connection.ts b/packages/puppeteer-core/src/common/Connection.ts index 0ee372b4..8188c93a 100644 --- a/packages/puppeteer-core/src/common/Connection.ts +++ b/packages/puppeteer-core/src/common/Connection.ts @@ -18,12 +18,13 @@ import {Protocol} from 'devtools-protocol'; import {ProtocolMapping} from 'devtools-protocol/types/protocol-mapping.js'; import {assert} from '../util/assert.js'; -import {createDeferredPromise} from '../util/util.js'; +import {createDeferredPromise, DeferredPromise} from '../util/util.js'; import {ConnectionTransport} from './ConnectionTransport.js'; import {debug} from './Debug.js'; import {TargetCloseError, ProtocolError} from './Errors.js'; import {EventEmitter} from './EventEmitter.js'; +import {debugError} from './util.js'; const debugProtocolSend = debug('puppeteer:protocol:SEND ►'); const debugProtocolReceive = debug('puppeteer:protocol:RECV ◀'); @@ -96,7 +97,7 @@ class Callback { return this.#id; } - get promise(): Promise { + get promise(): DeferredPromise { return this.#promise; } @@ -130,14 +131,17 @@ export class CallbackRegistry { } catch (error) { // We still throw sync errors synchronously and clean up the scheduled // callback. - callback.promise.catch(() => { - this.#callbacks.delete(callback.id); - }); + callback.promise + .valueOrThrow() + .catch(debugError) + .finally(() => { + this.#callbacks.delete(callback.id); + }); callback.reject(error as Error); throw error; } // Must only have sync code up until here. - return callback.promise.finally(() => { + return callback.promise.valueOrThrow().finally(() => { this.#callbacks.delete(callback.id); }); } diff --git a/packages/puppeteer-core/src/common/DeviceRequestPrompt.ts b/packages/puppeteer-core/src/common/DeviceRequestPrompt.ts index aa3b1264..75e12f0b 100644 --- a/packages/puppeteer-core/src/common/DeviceRequestPrompt.ts +++ b/packages/puppeteer-core/src/common/DeviceRequestPrompt.ts @@ -161,7 +161,7 @@ export class DeviceRequestPrompt { const handle = {filter, promise}; this.#waitForDevicePromises.add(handle); try { - return await promise; + return await promise.valueOrThrow(); } finally { this.#waitForDevicePromises.delete(handle); } @@ -262,7 +262,10 @@ export class DeviceRequestPromptManager { this.#deviceRequestPromptPromises.add(promise); try { - const [result] = await Promise.all([promise, enablePromise]); + const [result] = await Promise.all([ + promise.valueOrThrow(), + enablePromise, + ]); return result; } finally { this.#deviceRequestPromptPromises.delete(promise); diff --git a/packages/puppeteer-core/src/common/FirefoxTargetManager.ts b/packages/puppeteer-core/src/common/FirefoxTargetManager.ts index 745c37d9..4628291f 100644 --- a/packages/puppeteer-core/src/common/FirefoxTargetManager.ts +++ b/packages/puppeteer-core/src/common/FirefoxTargetManager.ts @@ -169,7 +169,7 @@ export class FirefoxTargetManager filter: [{}], }); this.#targetsIdsForInit = new Set(this.#discoveredTargetsByTargetId.keys()); - await this.#initializePromise; + await this.#initializePromise.valueOrThrow(); } #onTargetCreated = async ( diff --git a/packages/puppeteer-core/src/common/Frame.ts b/packages/puppeteer-core/src/common/Frame.ts index eaee3995..836d9592 100644 --- a/packages/puppeteer-core/src/common/Frame.ts +++ b/packages/puppeteer-core/src/common/Frame.ts @@ -419,7 +419,7 @@ export class Frame extends BaseFrame { script.id = id; } document.head.appendChild(script); - await promise; + await promise.valueOrThrow(); return script; }, LazyArg.create(context => { @@ -488,7 +488,7 @@ export class Frame extends BaseFrame { {once: true} ); document.head.appendChild(element); - await promise; + await promise.valueOrThrow(); return element; }, LazyArg.create(context => { diff --git a/packages/puppeteer-core/src/common/FrameTree.ts b/packages/puppeteer-core/src/common/FrameTree.ts index 5dc50e3d..34effcc5 100644 --- a/packages/puppeteer-core/src/common/FrameTree.ts +++ b/packages/puppeteer-core/src/common/FrameTree.ts @@ -57,7 +57,7 @@ export class FrameTree { const callbacks = this.#waitRequests.get(frameId) || new Set>(); callbacks.add(deferred); - return deferred; + return deferred.valueOrThrow(); } frames(): Frame[] { diff --git a/packages/puppeteer-core/src/common/HTTPResponse.ts b/packages/puppeteer-core/src/common/HTTPResponse.ts index faf7c157..e4c5ff16 100644 --- a/packages/puppeteer-core/src/common/HTTPResponse.ts +++ b/packages/puppeteer-core/src/common/HTTPResponse.ts @@ -136,31 +136,37 @@ export class HTTPResponse extends BaseHTTPResponse { override buffer(): Promise { if (!this.#contentPromise) { - this.#contentPromise = this.#bodyLoadedPromise.then(async error => { - if (error) { - throw error; - } - try { - const response = await this.#client.send('Network.getResponseBody', { - requestId: this.#request._requestId, - }); - return Buffer.from( - response.body, - response.base64Encoded ? 'base64' : 'utf8' - ); - } catch (error) { - if ( - error instanceof ProtocolError && - error.originalMessage === 'No resource with given identifier found' - ) { - throw new ProtocolError( - 'Could not load body for this request. This might happen if the request is a preflight request.' - ); + this.#contentPromise = this.#bodyLoadedPromise + .valueOrThrow() + .then(async error => { + if (error) { + throw error; } + try { + const response = await this.#client.send( + 'Network.getResponseBody', + { + requestId: this.#request._requestId, + } + ); + return Buffer.from( + response.body, + response.base64Encoded ? 'base64' : 'utf8' + ); + } catch (error) { + if ( + error instanceof ProtocolError && + error.originalMessage === + 'No resource with given identifier found' + ) { + throw new ProtocolError( + 'Could not load body for this request. This might happen if the request is a preflight request.' + ); + } - throw error; - } - }); + throw error; + } + }); } return this.#contentPromise; } diff --git a/packages/puppeteer-core/src/common/IsolatedWorld.ts b/packages/puppeteer-core/src/common/IsolatedWorld.ts index d2f9004b..2b8c235e 100644 --- a/packages/puppeteer-core/src/common/IsolatedWorld.ts +++ b/packages/puppeteer-core/src/common/IsolatedWorld.ts @@ -174,7 +174,7 @@ export class IsolatedWorld { if (this.#context === null) { throw new Error(`Execution content promise is missing`); } - return this.#context; + return this.#context.valueOrThrow(); } async evaluateHandle< @@ -425,13 +425,17 @@ export class IsolatedWorld { return; } - const context = await this.#context; - if (event.executionContextId !== context._contextId) { - return; - } + try { + const context = await this.#context.valueOrThrow(); + if (event.executionContextId !== context._contextId) { + return; + } - const binding = this._bindings.get(name); - await binding?.run(context, seq, args, isTrivial); + const binding = this._bindings.get(name); + await binding?.run(context, seq, args, isTrivial); + } catch (err) { + debugError(err); + } }; waitForFunction< diff --git a/packages/puppeteer-core/src/common/LifecycleWatcher.ts b/packages/puppeteer-core/src/common/LifecycleWatcher.ts index 06899eb0..12e0ce03 100644 --- a/packages/puppeteer-core/src/common/LifecycleWatcher.ts +++ b/packages/puppeteer-core/src/common/LifecycleWatcher.ts @@ -204,7 +204,7 @@ export class LifecycleWatcher { async navigationResponse(): Promise { // Continue with a possibly null response. - await this.#navigationResponseReceived?.catch(() => {}); + await this.#navigationResponseReceived?.valueOrThrow(); return this.#navigationRequest ? this.#navigationRequest.response() : null; } @@ -213,19 +213,22 @@ export class LifecycleWatcher { } sameDocumentNavigationPromise(): Promise { - return this.#sameDocumentNavigationPromise; + return this.#sameDocumentNavigationPromise.valueOrThrow(); } newDocumentNavigationPromise(): Promise { - return this.#newDocumentNavigationPromise; + return this.#newDocumentNavigationPromise.valueOrThrow(); } lifecyclePromise(): Promise { - return this.#lifecyclePromise; + return this.#lifecyclePromise.valueOrThrow(); } timeoutOrTerminationPromise(): Promise { - return Promise.race([this.#timeoutPromise, this.#terminationPromise]); + return Promise.race([ + this.#timeoutPromise, + this.#terminationPromise.valueOrThrow(), + ]); } async #createTimeoutPromise(): Promise { diff --git a/packages/puppeteer-core/src/common/NetworkManager.ts b/packages/puppeteer-core/src/common/NetworkManager.ts index d9a46be5..46817868 100644 --- a/packages/puppeteer-core/src/common/NetworkManager.ts +++ b/packages/puppeteer-core/src/common/NetworkManager.ts @@ -131,7 +131,7 @@ export class NetworkManager extends EventEmitter { */ initialize(): Promise { if (this.#deferredInitPromise) { - return this.#deferredInitPromise; + return this.#deferredInitPromise.valueOrThrow(); } this.#deferredInitPromise = createDebuggableDeferredPromise( 'NetworkManager initialization timed out' @@ -152,7 +152,7 @@ export class NetworkManager extends EventEmitter { .catch(err => { deferredInitPromise.reject(err); }); - return this.#deferredInitPromise; + return this.#deferredInitPromise.valueOrThrow(); } async authenticate(credentials?: Credentials): Promise { diff --git a/packages/puppeteer-core/src/common/Page.ts b/packages/puppeteer-core/src/common/Page.ts index 193133db..aaa4767c 100644 --- a/packages/puppeteer-core/src/common/Page.ts +++ b/packages/puppeteer-core/src/common/Page.ts @@ -256,17 +256,23 @@ export class CDPPage extends Page { client.on('Page.fileChooserOpened', event => { return this.#onFileChooser(event); }); - void this.#target._isClosedPromise.then(() => { - this.#target - ._targetManager() - .removeTargetInterceptor(this.#client, this.#onAttachedToTarget); + this.#target._isClosedPromise + .valueOrThrow() + .then(() => { + this.#target + ._targetManager() + .removeTargetInterceptor(this.#client, this.#onAttachedToTarget); - this.#target - ._targetManager() - .off(TargetManagerEmittedEvents.TargetGone, this.#onDetachedFromTarget); - this.emit(PageEmittedEvents.Close); - this.#closed = true; - }); + this.#target + ._targetManager() + .off( + TargetManagerEmittedEvents.TargetGone, + this.#onDetachedFromTarget + ); + this.emit(PageEmittedEvents.Close); + this.#closed = true; + }) + .catch(debugError); } #onDetachedFromTarget = (target: Target) => { @@ -376,7 +382,7 @@ export class CDPPage extends Page { enabled: true, }); } - return Promise.all([promise, enablePromise]) + return Promise.all([promise.valueOrThrow(), enablePromise]) .then(([result]) => { return result; }) @@ -1048,7 +1054,7 @@ export class CDPPage extends Page { ]; await Promise.race([ - idlePromise, + idlePromise.valueOrThrow(), ...eventPromises, this.#sessionClosePromise(), ]).then( @@ -1556,7 +1562,7 @@ export class CDPPage extends Page { await connection.send('Target.closeTarget', { targetId: this.#target._targetId, }); - await this.#target._isClosedPromise; + await this.#target._isClosedPromise.valueOrThrow(); } } diff --git a/packages/puppeteer-core/src/common/Target.ts b/packages/puppeteer-core/src/common/Target.ts index 4a3008a2..39613368 100644 --- a/packages/puppeteer-core/src/common/Target.ts +++ b/packages/puppeteer-core/src/common/Target.ts @@ -249,6 +249,7 @@ export class PageTarget extends Target { protected override _initialize(): void { this._initializedPromise + .valueOrThrow() .then(async result => { if (result === InitializationStatus.ABORTED) { return; diff --git a/packages/puppeteer-core/src/common/WaitTask.ts b/packages/puppeteer-core/src/common/WaitTask.ts index 30155d4a..cabeadb5 100644 --- a/packages/puppeteer-core/src/common/WaitTask.ts +++ b/packages/puppeteer-core/src/common/WaitTask.ts @@ -18,6 +18,7 @@ import {ElementHandle} from '../api/ElementHandle.js'; import {JSHandle} from '../api/JSHandle.js'; import type {Poller} from '../injected/Poller.js'; import {createDeferredPromise} from '../util/DeferredPromise.js'; +import {isErrorLike} from '../util/ErrorLike.js'; import {stringifyFunction} from '../util/Function.js'; import {TimeoutError} from './Errors.js'; @@ -97,7 +98,7 @@ export class WaitTask { } get result(): Promise> { - return this.#result; + return this.#result.valueOrThrow(); } async rerun(): Promise { @@ -170,7 +171,7 @@ export class WaitTask { } } - async terminate(error?: unknown): Promise { + async terminate(error?: Error): Promise { this.#world.taskManager.delete(this); if (this.#timeout) { @@ -199,8 +200,8 @@ export class WaitTask { /** * Not all errors lead to termination. They usually imply we need to rerun the task. */ - getBadError(error: unknown): unknown { - if (error instanceof Error) { + getBadError(error: unknown): Error | undefined { + if (isErrorLike(error)) { // When frame is detached the task should have been terminated by the IsolatedWorld. // This can fail if we were adding this task while the frame was detached, // so we terminate here instead. @@ -223,9 +224,14 @@ export class WaitTask { if (error.message.includes('Cannot find context with specified id')) { return; } + + return error; } - return error; + // @ts-expect-error TODO: uncomment once cause is supported in Node types. + return new Error('WaitTask failed with an error', { + cause: error, + }); } } diff --git a/packages/puppeteer-core/src/common/WebWorker.ts b/packages/puppeteer-core/src/common/WebWorker.ts index d4c04c9d..beab09e3 100644 --- a/packages/puppeteer-core/src/common/WebWorker.ts +++ b/packages/puppeteer-core/src/common/WebWorker.ts @@ -91,14 +91,18 @@ export class WebWorker extends EventEmitter { this.#executionContext.resolve(context); }); this.#client.on('Runtime.consoleAPICalled', async event => { - const context = await this.#executionContext; - return consoleAPICalled( - event.type, - event.args.map((object: Protocol.Runtime.RemoteObject) => { - return new CDPJSHandle(context, object); - }), - event.stackTrace - ); + try { + const context = await this.#executionContext.valueOrThrow(); + return consoleAPICalled( + event.type, + event.args.map((object: Protocol.Runtime.RemoteObject) => { + return new CDPJSHandle(context, object); + }), + event.stackTrace + ); + } catch (err) { + debugError(err); + } }); this.#client.on('Runtime.exceptionThrown', exception => { return exceptionThrown(exception.exceptionDetails); @@ -112,7 +116,7 @@ export class WebWorker extends EventEmitter { * @internal */ async executionContext(): Promise { - return this.#executionContext; + return this.#executionContext.valueOrThrow(); } /** @@ -154,7 +158,7 @@ export class WebWorker extends EventEmitter { this.evaluate.name, pageFunction ); - const context = await this.#executionContext; + const context = await this.#executionContext.valueOrThrow(); return context.evaluate(pageFunction, ...args); } @@ -181,7 +185,7 @@ export class WebWorker extends EventEmitter { this.evaluateHandle.name, pageFunction ); - const context = await this.#executionContext; + const context = await this.#executionContext.valueOrThrow(); return context.evaluateHandle(pageFunction, ...args); } } diff --git a/packages/puppeteer-core/src/injected/Poller.ts b/packages/puppeteer-core/src/injected/Poller.ts index d7f4eb39..685f3eee 100644 --- a/packages/puppeteer-core/src/injected/Poller.ts +++ b/packages/puppeteer-core/src/injected/Poller.ts @@ -80,7 +80,7 @@ export class MutationPoller implements Poller { result(): Promise { assert(this.#promise, 'Polling never started.'); - return this.#promise; + return this.#promise.valueOrThrow(); } } @@ -126,7 +126,7 @@ export class RAFPoller implements Poller { result(): Promise { assert(this.#promise, 'Polling never started.'); - return this.#promise; + return this.#promise.valueOrThrow(); } } @@ -176,6 +176,6 @@ export class IntervalPoller implements Poller { result(): Promise { assert(this.#promise, 'Polling never started.'); - return this.#promise; + return this.#promise.valueOrThrow(); } } diff --git a/packages/puppeteer-core/src/util/DeferredPromise.ts b/packages/puppeteer-core/src/util/DeferredPromise.ts index f240e362..4f8f2ca7 100644 --- a/packages/puppeteer-core/src/util/DeferredPromise.ts +++ b/packages/puppeteer-core/src/util/DeferredPromise.ts @@ -3,12 +3,13 @@ import {TimeoutError} from '../common/Errors.js'; /** * @internal */ -export interface DeferredPromise extends Promise { +export interface DeferredPromise { finished: () => boolean; resolved: () => boolean; resolve: (value: T) => void; - reject: (reason?: unknown) => void; - value: () => T | undefined; + reject: (error: Error) => void; + value: () => T | Error | undefined; + valueOrThrow: () => Promise; } /** @@ -20,10 +21,10 @@ export interface DeferredPromiseOptions { } /** - * Creates and returns a promise along with the resolve/reject functions. + * Creates and returns a deferred object along with the resolve/reject functions. * - * If the promise has not been resolved/rejected within the `timeout` period, - * the promise gets rejected with a timeout error. `timeout` has to be greater than 0 or + * If the deferred has not been resolved/rejected within the `timeout` period, + * the deferred gets resolves with a timeout error. `timeout` has to be greater than 0 or * it is ignored. * * @internal @@ -33,42 +34,58 @@ export function createDeferredPromise( ): DeferredPromise { let isResolved = false; let isRejected = false; - let _value: T | undefined; - let resolver: (value: T) => void; - let rejector: (reason?: unknown) => void; - const taskPromise = new Promise((resolve, reject) => { + let _value: T | Error | undefined; + let resolver: (value: void) => void; + const taskPromise = new Promise(resolve => { resolver = resolve; - rejector = reject; }); const timeoutId = opts && opts.timeout > 0 ? setTimeout(() => { - isRejected = true; - rejector(new TimeoutError(opts.message)); + reject(new TimeoutError(opts.message)); }, opts.timeout) : undefined; - return Object.assign(taskPromise, { + + function finish(value: T | Error) { + clearTimeout(timeoutId); + _value = value; + resolver(); + } + + function resolve(value: T) { + if (isRejected || isResolved) { + return; + } + isResolved = true; + finish(value); + } + + function reject(error: Error) { + if (isRejected || isResolved) { + return; + } + isRejected = true; + finish(error); + } + + return { resolved: () => { return isResolved; }, finished: () => { return isResolved || isRejected; }, - resolve: (value: T) => { - if (timeoutId) { - clearTimeout(timeoutId); - } - isResolved = true; - _value = value; - resolver(value); - }, - reject: (err?: unknown) => { - clearTimeout(timeoutId); - isRejected = true; - rejector(err); - }, + resolve, + reject, value: () => { return _value; }, - }); + async valueOrThrow() { + await taskPromise; + if (isRejected) { + throw _value; + } + return _value as T; + }, + }; } diff --git a/test/src/DeferredPromise.spec.ts b/test/src/DeferredPromise.spec.ts new file mode 100644 index 00000000..aef93b9f --- /dev/null +++ b/test/src/DeferredPromise.spec.ts @@ -0,0 +1,51 @@ +/** + * Copyright 2023 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import expect from 'expect'; +import { + DeferredPromise, + createDeferredPromise, +} from 'puppeteer-core/internal/util/DeferredPromise.js'; + +describe('DeferredPromise', function () { + it('should catch errors', async () => { + // Async function before try/catch. + async function task() { + await new Promise(resolve => { + return setTimeout(resolve, 50); + }); + } + // Async function that fails. + function fails(): DeferredPromise { + const deferred = createDeferredPromise(); + setTimeout(() => { + deferred.reject(new Error('test')); + }, 25); + return deferred; + } + + const expectedToFail = fails(); + await task(); + let caught = false; + try { + await expectedToFail.valueOrThrow(); + } catch (err) { + expect((err as Error).message).toEqual('test'); + caught = true; + } + expect(caught).toBeTruthy(); + }); +}); diff --git a/test/src/DeviceRequestPrompt.spec.ts b/test/src/DeviceRequestPrompt.spec.ts index b898b209..0d19f9a7 100644 --- a/test/src/DeviceRequestPrompt.spec.ts +++ b/test/src/DeviceRequestPrompt.spec.ts @@ -1,3 +1,19 @@ +/** + * Copyright 2022 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + import expect from 'expect'; import {TimeoutError} from 'puppeteer'; import {