From 60f1b788a6304504f504b0be9f02cb768e2803f8 Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Mon, 23 Oct 2023 11:02:19 +0200 Subject: [PATCH] fix: remove import cycle in connection (#11225) --- .../puppeteer-core/src/bidi/Connection.ts | 2 +- packages/puppeteer-core/src/cdp/CDPSession.ts | 24 ++- packages/puppeteer-core/src/cdp/Connection.ts | 188 +----------------- .../src/common/CallbackRegistry.ts | 174 ++++++++++++++++ packages/puppeteer-core/src/util/ErrorLike.ts | 55 ++++- 5 files changed, 249 insertions(+), 194 deletions(-) create mode 100644 packages/puppeteer-core/src/common/CallbackRegistry.ts diff --git a/packages/puppeteer-core/src/bidi/Connection.ts b/packages/puppeteer-core/src/bidi/Connection.ts index 40058bb1..5e191979 100644 --- a/packages/puppeteer-core/src/bidi/Connection.ts +++ b/packages/puppeteer-core/src/bidi/Connection.ts @@ -16,7 +16,7 @@ import type * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js'; -import {CallbackRegistry} from '../cdp/Connection.js'; +import {CallbackRegistry} from '../common/CallbackRegistry.js'; import type {ConnectionTransport} from '../common/ConnectionTransport.js'; import {debug} from '../common/Debug.js'; import {EventEmitter} from '../common/EventEmitter.js'; diff --git a/packages/puppeteer-core/src/cdp/CDPSession.ts b/packages/puppeteer-core/src/cdp/CDPSession.ts index 6b7dadac..1666f8ac 100644 --- a/packages/puppeteer-core/src/cdp/CDPSession.ts +++ b/packages/puppeteer-core/src/cdp/CDPSession.ts @@ -1,3 +1,19 @@ +/** + * Copyright 2017 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 type {ProtocolMapping} from 'devtools-protocol/types/protocol-mapping.js'; import { @@ -5,14 +21,12 @@ import { CDPSession, CDPSessionEvent, } from '../api/CDPSession.js'; +import {CallbackRegistry} from '../common/CallbackRegistry.js'; import {TargetCloseError} from '../common/Errors.js'; import {assert} from '../util/assert.js'; +import {createProtocolErrorMessage} from '../util/ErrorLike.js'; -import { - CallbackRegistry, - type Connection, - createProtocolErrorMessage, -} from './Connection.js'; +import type {Connection} from './Connection.js'; import type {CdpTarget} from './Target.js'; /** diff --git a/packages/puppeteer-core/src/cdp/Connection.ts b/packages/puppeteer-core/src/cdp/Connection.ts index 6916b1bf..e3ec333d 100644 --- a/packages/puppeteer-core/src/cdp/Connection.ts +++ b/packages/puppeteer-core/src/cdp/Connection.ts @@ -22,12 +22,12 @@ import { type CDPSession, type CDPSessionEvents, } from '../api/CDPSession.js'; +import {CallbackRegistry} from '../common/CallbackRegistry.js'; import type {ConnectionTransport} from '../common/ConnectionTransport.js'; import {debug} from '../common/Debug.js'; -import {ProtocolError, TargetCloseError} from '../common/Errors.js'; +import {TargetCloseError} from '../common/Errors.js'; import {EventEmitter} from '../common/EventEmitter.js'; -import {debugError} from '../common/util.js'; -import {Deferred} from '../util/Deferred.js'; +import {createProtocolErrorMessage} from '../util/ErrorLike.js'; import {CdpCDPSession} from './CDPSession.js'; @@ -39,159 +39,6 @@ const debugProtocolReceive = debug('puppeteer:protocol:RECV ◀'); */ export type {ConnectionTransport, ProtocolMapping}; -/** - * @internal - */ -type GetIdFn = () => number; - -/** - * @internal - */ -function createIncrementalIdGenerator(): GetIdFn { - let id = 0; - return (): number => { - return ++id; - }; -} - -/** - * @internal - */ -export class Callback { - #id: number; - #error = new ProtocolError(); - #deferred = Deferred.create(); - #timer?: ReturnType; - #label: string; - - constructor(id: number, label: string, timeout?: number) { - this.#id = id; - this.#label = label; - if (timeout) { - this.#timer = setTimeout(() => { - this.#deferred.reject( - rewriteError( - this.#error, - `${label} timed out. Increase the 'protocolTimeout' setting in launch/connect calls for a higher timeout if needed.` - ) - ); - }, timeout); - } - } - - resolve(value: unknown): void { - clearTimeout(this.#timer); - this.#deferred.resolve(value); - } - - reject(error: Error): void { - clearTimeout(this.#timer); - this.#deferred.reject(error); - } - - get id(): number { - return this.#id; - } - - get promise(): Deferred { - return this.#deferred; - } - - get error(): ProtocolError { - return this.#error; - } - - get label(): string { - return this.#label; - } -} - -/** - * Manages callbacks and their IDs for the protocol request/response communication. - * - * @internal - */ -export class CallbackRegistry { - #callbacks = new Map(); - #idGenerator = createIncrementalIdGenerator(); - - create( - label: string, - timeout: number | undefined, - request: (id: number) => void - ): Promise { - const callback = new Callback(this.#idGenerator(), label, timeout); - this.#callbacks.set(callback.id, callback); - try { - request(callback.id); - } catch (error) { - // We still throw sync errors synchronously and clean up the scheduled - // callback. - 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.valueOrThrow().finally(() => { - this.#callbacks.delete(callback.id); - }); - } - - reject(id: number, message: string, originalMessage?: string): void { - const callback = this.#callbacks.get(id); - if (!callback) { - return; - } - this._reject(callback, message, originalMessage); - } - - _reject( - callback: Callback, - errorMessage: string | ProtocolError, - originalMessage?: string - ): void { - let error: ProtocolError; - let message: string; - if (errorMessage instanceof ProtocolError) { - error = errorMessage; - error.cause = callback.error; - message = errorMessage.message; - } else { - error = callback.error; - message = errorMessage; - } - - callback.reject( - rewriteError( - error, - `Protocol error (${callback.label}): ${message}`, - originalMessage - ) - ); - } - - resolve(id: number, value: unknown): void { - const callback = this.#callbacks.get(id); - if (!callback) { - return; - } - callback.resolve(value); - } - - clear(): void { - for (const callback of this.#callbacks.values()) { - // TODO: probably we can accept error messages as params. - this._reject(callback, new TargetCloseError('Target closed')); - } - this.#callbacks.clear(); - } -} - /** * @public */ @@ -414,35 +261,6 @@ export class Connection extends EventEmitter { } } -/** - * @internal - */ -export function createProtocolErrorMessage(object: { - error: {message: string; data: any; code: number}; -}): string { - let message = `${object.error.message}`; - // TODO: remove the type checks when we stop connecting to BiDi with a CDP - // client. - if ( - object.error && - typeof object.error === 'object' && - 'data' in object.error - ) { - message += ` ${object.error.data}`; - } - return message; -} - -function rewriteError( - error: ProtocolError, - message: string, - originalMessage?: string -): Error { - error.message = message; - error.originalMessage = originalMessage ?? error.originalMessage; - return error; -} - /** * @internal */ diff --git a/packages/puppeteer-core/src/common/CallbackRegistry.ts b/packages/puppeteer-core/src/common/CallbackRegistry.ts new file mode 100644 index 00000000..0c4ef536 --- /dev/null +++ b/packages/puppeteer-core/src/common/CallbackRegistry.ts @@ -0,0 +1,174 @@ +/** + * 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 {Deferred} from '../util/Deferred.js'; +import {rewriteError} from '../util/ErrorLike.js'; + +import {ProtocolError, TargetCloseError} from './Errors.js'; +import {debugError} from './util.js'; + +/** + * Manages callbacks and their IDs for the protocol request/response communication. + * + * @internal + */ +export class CallbackRegistry { + #callbacks = new Map(); + #idGenerator = createIncrementalIdGenerator(); + + create( + label: string, + timeout: number | undefined, + request: (id: number) => void + ): Promise { + const callback = new Callback(this.#idGenerator(), label, timeout); + this.#callbacks.set(callback.id, callback); + try { + request(callback.id); + } catch (error) { + // We still throw sync errors synchronously and clean up the scheduled + // callback. + 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.valueOrThrow().finally(() => { + this.#callbacks.delete(callback.id); + }); + } + + reject(id: number, message: string, originalMessage?: string): void { + const callback = this.#callbacks.get(id); + if (!callback) { + return; + } + this._reject(callback, message, originalMessage); + } + + _reject( + callback: Callback, + errorMessage: string | ProtocolError, + originalMessage?: string + ): void { + let error: ProtocolError; + let message: string; + if (errorMessage instanceof ProtocolError) { + error = errorMessage; + error.cause = callback.error; + message = errorMessage.message; + } else { + error = callback.error; + message = errorMessage; + } + + callback.reject( + rewriteError( + error, + `Protocol error (${callback.label}): ${message}`, + originalMessage + ) + ); + } + + resolve(id: number, value: unknown): void { + const callback = this.#callbacks.get(id); + if (!callback) { + return; + } + callback.resolve(value); + } + + clear(): void { + for (const callback of this.#callbacks.values()) { + // TODO: probably we can accept error messages as params. + this._reject(callback, new TargetCloseError('Target closed')); + } + this.#callbacks.clear(); + } +} +/** + * @internal + */ + +export class Callback { + #id: number; + #error = new ProtocolError(); + #deferred = Deferred.create(); + #timer?: ReturnType; + #label: string; + + constructor(id: number, label: string, timeout?: number) { + this.#id = id; + this.#label = label; + if (timeout) { + this.#timer = setTimeout(() => { + this.#deferred.reject( + rewriteError( + this.#error, + `${label} timed out. Increase the 'protocolTimeout' setting in launch/connect calls for a higher timeout if needed.` + ) + ); + }, timeout); + } + } + + resolve(value: unknown): void { + clearTimeout(this.#timer); + this.#deferred.resolve(value); + } + + reject(error: Error): void { + clearTimeout(this.#timer); + this.#deferred.reject(error); + } + + get id(): number { + return this.#id; + } + + get promise(): Deferred { + return this.#deferred; + } + + get error(): ProtocolError { + return this.#error; + } + + get label(): string { + return this.#label; + } +} + +/** + * @internal + */ +export function createIncrementalIdGenerator(): GetIdFn { + let id = 0; + return (): number => { + return ++id; + }; +} + +/** + * @internal + */ +export type GetIdFn = () => number; diff --git a/packages/puppeteer-core/src/util/ErrorLike.ts b/packages/puppeteer-core/src/util/ErrorLike.ts index e5659ce3..eec3218f 100644 --- a/packages/puppeteer-core/src/util/ErrorLike.ts +++ b/packages/puppeteer-core/src/util/ErrorLike.ts @@ -1,27 +1,76 @@ +/** + * 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 type {ProtocolError} from '../common/Errors.js'; + /** * @internal */ - export interface ErrorLike extends Error { name: string; message: string; } + /** * @internal */ - export function isErrorLike(obj: unknown): obj is ErrorLike { return ( typeof obj === 'object' && obj !== null && 'name' in obj && 'message' in obj ); } + /** * @internal */ - export function isErrnoException(obj: unknown): obj is NodeJS.ErrnoException { return ( isErrorLike(obj) && ('errno' in obj || 'code' in obj || 'path' in obj || 'syscall' in obj) ); } + +/** + * @internal + */ +export function rewriteError( + error: ProtocolError, + message: string, + originalMessage?: string +): Error { + error.message = message; + error.originalMessage = originalMessage ?? error.originalMessage; + return error; +} + +/** + * @internal + */ +export function createProtocolErrorMessage(object: { + error: {message: string; data: any; code: number}; +}): string { + let message = `${object.error.message}`; + // TODO: remove the type checks when we stop connecting to BiDi with a CDP + // client. + if ( + object.error && + typeof object.error === 'object' && + 'data' in object.error + ) { + message += ` ${object.error.data}`; + } + return message; +}