From c660d4001d610854399d7ecb551c4eb56a7f840a Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Tue, 2 Jan 2024 11:00:07 +0100 Subject: [PATCH] feat: support timeouts per CDP command (#11595) --- docs/api/index.md | 1 + docs/api/puppeteer.cdpsession.md | 12 +++++------ docs/api/puppeteer.cdpsession.send.md | 12 ++++++----- docs/api/puppeteer.commandoptions.md | 17 ++++++++++++++++ docs/api/puppeteer.connection.md | 2 +- docs/api/puppeteer.connection.send.md | 12 ++++++----- packages/puppeteer-core/src/api/CDPSession.ts | 10 +++++++++- packages/puppeteer-core/src/cdp/CDPSession.ts | 9 +++++---- packages/puppeteer-core/src/cdp/Connection.ts | 12 ++++++----- test/TestExpectations.json | 6 ++++++ test/src/cdp/CDPSession.spec.ts | 20 +++++++++++++++++++ 11 files changed, 86 insertions(+), 27 deletions(-) create mode 100644 docs/api/puppeteer.commandoptions.md diff --git a/docs/api/index.md b/docs/api/index.md index a306b990cae..4c0f6f80e13 100644 --- a/docs/api/index.md +++ b/docs/api/index.md @@ -80,6 +80,7 @@ sidebar_label: API | [BrowserLaunchArgumentOptions](./puppeteer.browserlaunchargumentoptions.md) | Launcher options that only apply to Chrome. | | [CDPSessionEvents](./puppeteer.cdpsessionevents.md) | | | [ClickOptions](./puppeteer.clickoptions.md) | | +| [CommandOptions](./puppeteer.commandoptions.md) | | | [CommonEventEmitter](./puppeteer.commoneventemitter.md) | | | [Configuration](./puppeteer.configuration.md) |

Defines options to configure Puppeteer's behavior during installation and runtime.

See individual properties for more information.

| | [ConnectionTransport](./puppeteer.connectiontransport.md) | | diff --git a/docs/api/puppeteer.cdpsession.md b/docs/api/puppeteer.cdpsession.md index 7a8a4ade595..203f3a544f8 100644 --- a/docs/api/puppeteer.cdpsession.md +++ b/docs/api/puppeteer.cdpsession.md @@ -39,9 +39,9 @@ await client.send('Animation.setPlaybackRate', { ## Methods -| Method | Modifiers | Description | -| --------------------------------------------------------- | --------- | --------------------------------------------------------------------------------------------------------------------------------------- | -| [connection()](./puppeteer.cdpsession.connection.md) | | | -| [detach()](./puppeteer.cdpsession.detach.md) | | Detaches the cdpSession from the target. Once detached, the cdpSession object won't emit any events and can't be used to send messages. | -| [id()](./puppeteer.cdpsession.id.md) | | Returns the session's id. | -| [send(method, paramArgs)](./puppeteer.cdpsession.send.md) | | | +| Method | Modifiers | Description | +| --------------------------------------------------------------- | --------- | --------------------------------------------------------------------------------------------------------------------------------------- | +| [connection()](./puppeteer.cdpsession.connection.md) | | | +| [detach()](./puppeteer.cdpsession.detach.md) | | Detaches the cdpSession from the target. Once detached, the cdpSession object won't emit any events and can't be used to send messages. | +| [id()](./puppeteer.cdpsession.id.md) | | Returns the session's id. | +| [send(method, params, options)](./puppeteer.cdpsession.send.md) | | | diff --git a/docs/api/puppeteer.cdpsession.send.md b/docs/api/puppeteer.cdpsession.send.md index 9c532ef151d..31a2ceddc86 100644 --- a/docs/api/puppeteer.cdpsession.send.md +++ b/docs/api/puppeteer.cdpsession.send.md @@ -10,17 +10,19 @@ sidebar_label: CDPSession.send class CDPSession { abstract send( method: T, - ...paramArgs: ProtocolMapping.Commands[T]['paramsType'] + params?: ProtocolMapping.Commands[T]['paramsType'][0], + options?: CommandOptions ): Promise; } ``` ## Parameters -| Parameter | Type | Description | -| --------- | --------------------------------------------- | ----------- | -| method | T | | -| paramArgs | ProtocolMapping.Commands\[T\]\['paramsType'\] | | +| Parameter | Type | Description | +| --------- | -------------------------------------------------- | ------------ | +| method | T | | +| params | ProtocolMapping.Commands\[T\]\['paramsType'\]\[0\] | _(Optional)_ | +| options | [CommandOptions](./puppeteer.commandoptions.md) | _(Optional)_ | **Returns:** diff --git a/docs/api/puppeteer.commandoptions.md b/docs/api/puppeteer.commandoptions.md new file mode 100644 index 00000000000..2149b4ba779 --- /dev/null +++ b/docs/api/puppeteer.commandoptions.md @@ -0,0 +1,17 @@ +--- +sidebar_label: CommandOptions +--- + +# CommandOptions interface + +#### Signature: + +```typescript +export interface CommandOptions +``` + +## Properties + +| Property | Modifiers | Type | Description | Default | +| -------- | --------- | ------ | ----------- | ------- | +| timeout | | number | | | diff --git a/docs/api/puppeteer.connection.md b/docs/api/puppeteer.connection.md index e6075c76a7f..3b7d0560412 100644 --- a/docs/api/puppeteer.connection.md +++ b/docs/api/puppeteer.connection.md @@ -31,6 +31,6 @@ export declare class Connection extends EventEmitter | [createSession(targetInfo)](./puppeteer.connection.createsession.md) | | | | [dispose()](./puppeteer.connection.dispose.md) | | | | [fromSession(session)](./puppeteer.connection.fromsession.md) | static | | -| [send(method, paramArgs)](./puppeteer.connection.send.md) | | | +| [send(method, params, options)](./puppeteer.connection.send.md) | | | | [session(sessionId)](./puppeteer.connection.session.md) | | | | [url()](./puppeteer.connection.url.md) | | | diff --git a/docs/api/puppeteer.connection.send.md b/docs/api/puppeteer.connection.send.md index a34deaf4387..d8334a46be3 100644 --- a/docs/api/puppeteer.connection.send.md +++ b/docs/api/puppeteer.connection.send.md @@ -10,17 +10,19 @@ sidebar_label: Connection.send class Connection { send( method: T, - ...paramArgs: ProtocolMapping.Commands[T]['paramsType'] + params?: ProtocolMapping.Commands[T]['paramsType'][0], + options?: CommandOptions ): Promise; } ``` ## Parameters -| Parameter | Type | Description | -| --------- | --------------------------------------------- | ----------- | -| method | T | | -| paramArgs | ProtocolMapping.Commands\[T\]\['paramsType'\] | | +| Parameter | Type | Description | +| --------- | -------------------------------------------------- | ------------ | +| method | T | | +| params | ProtocolMapping.Commands\[T\]\['paramsType'\]\[0\] | _(Optional)_ | +| options | [CommandOptions](./puppeteer.commandoptions.md) | _(Optional)_ | **Returns:** diff --git a/packages/puppeteer-core/src/api/CDPSession.ts b/packages/puppeteer-core/src/api/CDPSession.ts index 93f8291c2b3..8bdf96f9544 100644 --- a/packages/puppeteer-core/src/api/CDPSession.ts +++ b/packages/puppeteer-core/src/api/CDPSession.ts @@ -48,6 +48,13 @@ export interface CDPSessionEvents [CDPSessionEvent.SessionDetached]: CDPSession; } +/** + * @public + */ +export interface CommandOptions { + timeout: number; +} + /** * The `CDPSession` instances are used to talk raw Chrome Devtools Protocol. * @@ -97,7 +104,8 @@ export abstract class CDPSession extends EventEmitter { abstract send( method: T, - ...paramArgs: ProtocolMapping.Commands[T]['paramsType'] + params?: ProtocolMapping.Commands[T]['paramsType'][0], + options?: CommandOptions ): Promise; /** diff --git a/packages/puppeteer-core/src/cdp/CDPSession.ts b/packages/puppeteer-core/src/cdp/CDPSession.ts index a29e4e50c33..aed38a606cc 100644 --- a/packages/puppeteer-core/src/cdp/CDPSession.ts +++ b/packages/puppeteer-core/src/cdp/CDPSession.ts @@ -20,6 +20,7 @@ import { type CDPEvents, CDPSession, CDPSessionEvent, + type CommandOptions, } from '../api/CDPSession.js'; import {CallbackRegistry} from '../common/CallbackRegistry.js'; import {TargetCloseError} from '../common/Errors.js'; @@ -91,7 +92,8 @@ export class CdpCDPSession extends CDPSession { override send( method: T, - ...paramArgs: ProtocolMapping.Commands[T]['paramsType'] + params?: ProtocolMapping.Commands[T]['paramsType'][0], + options?: CommandOptions ): Promise { if (!this.#connection) { return Promise.reject( @@ -100,13 +102,12 @@ export class CdpCDPSession extends CDPSession { ) ); } - // See the comment in Connection#send explaining why we do this. - const params = paramArgs.length ? paramArgs[0] : undefined; return this.#connection._rawSend( this.#callbacks, method, params, - this.#sessionId + this.#sessionId, + options ); } diff --git a/packages/puppeteer-core/src/cdp/Connection.ts b/packages/puppeteer-core/src/cdp/Connection.ts index e3ec333d720..c725edfb6eb 100644 --- a/packages/puppeteer-core/src/cdp/Connection.ts +++ b/packages/puppeteer-core/src/cdp/Connection.ts @@ -17,6 +17,7 @@ import type {Protocol} from 'devtools-protocol'; import type {ProtocolMapping} from 'devtools-protocol/types/protocol-mapping.js'; +import type {CommandOptions} from '../api/CDPSession.js'; import { CDPSessionEvent, type CDPSession, @@ -104,7 +105,8 @@ export class Connection extends EventEmitter { send( method: T, - ...paramArgs: ProtocolMapping.Commands[T]['paramsType'] + params?: ProtocolMapping.Commands[T]['paramsType'][0], + options?: CommandOptions ): Promise { // There is only ever 1 param arg passed, but the Protocol defines it as an // array of 0 or 1 items See this comment: @@ -112,8 +114,7 @@ export class Connection extends EventEmitter { // which explains why the protocol defines the params this way for better // type-inference. // So now we check if there are any params or not and deal with them accordingly. - const params = paramArgs.length ? paramArgs[0] : undefined; - return this._rawSend(this.#callbacks, method, params); + return this._rawSend(this.#callbacks, method, params, undefined, options); } /** @@ -123,9 +124,10 @@ export class Connection extends EventEmitter { callbacks: CallbackRegistry, method: T, params: ProtocolMapping.Commands[T]['paramsType'][0], - sessionId?: string + sessionId?: string, + options?: CommandOptions ): Promise { - return callbacks.create(method, this.#timeout, id => { + return callbacks.create(method, options?.timeout ?? this.#timeout, id => { const stringifiedMessage = JSON.stringify({ method, params, diff --git a/test/TestExpectations.json b/test/TestExpectations.json index 0847236871e..0c150a4cb71 100644 --- a/test/TestExpectations.json +++ b/test/TestExpectations.json @@ -1445,6 +1445,12 @@ "parameters": ["chrome", "webDriverBiDi"], "expectations": ["PASS"] }, + { + "testIdPattern": "[CDPSession.spec] Target.createCDPSession should respect custom timeout", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["cdp", "firefox"], + "expectations": ["SKIP"] + }, { "testIdPattern": "[CDPSession.spec] Target.createCDPSession should send events", "platforms": ["darwin", "linux", "win32"], diff --git a/test/src/cdp/CDPSession.spec.ts b/test/src/cdp/CDPSession.spec.ts index 65254f9a133..ad0aa1f0231 100644 --- a/test/src/cdp/CDPSession.spec.ts +++ b/test/src/cdp/CDPSession.spec.ts @@ -127,6 +127,26 @@ describe('Target.createCDPSession', function () { } }); + it('should respect custom timeout', async () => { + const {page} = await getTestState(); + + const client = await page.createCDPSession(); + await expect( + client.send( + 'Runtime.evaluate', + { + expression: 'new Promise(resolve => {})', + awaitPromise: true, + }, + { + timeout: 50, + } + ) + ).rejects.toThrowError( + `Runtime.evaluate timed out. Increase the 'protocolTimeout' setting in launch/connect calls for a higher timeout if needed.` + ); + }); + it('should expose the underlying connection', async () => { const {page} = await getTestState();