Skip to content

Commit

Permalink
feat: support timeouts per CDP command (#11595)
Browse files Browse the repository at this point in the history
  • Loading branch information
@OrKoN
OrKoN authored Jan 2, 2024
1 parent acf1518 commit c660d40
Show file tree
Hide file tree
Showing 11 changed files with 86 additions and 27 deletions.
1 change: 1 addition & 0 deletions docs/api/index.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) | <p>Defines options to configure Puppeteer's behavior during installation and runtime.</p><p>See individual properties for more information.</p> |
| [ConnectionTransport](./puppeteer.connectiontransport.md) | |
Expand Down
12 changes: 6 additions & 6 deletions docs/api/puppeteer.cdpsession.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) | | |
12 changes: 7 additions & 5 deletions docs/api/puppeteer.cdpsession.send.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,19 @@ sidebar_label: CDPSession.send
class CDPSession {
abstract send<T extends keyof ProtocolMapping.Commands>(
method: T,
...paramArgs: ProtocolMapping.Commands[T]['paramsType']
params?: ProtocolMapping.Commands[T]['paramsType'][0],
options?: CommandOptions
): Promise<ProtocolMapping.Commands[T]['returnType']>;
}
```

## 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:**

Expand Down
17 changes: 17 additions & 0 deletions docs/api/puppeteer.commandoptions.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
sidebar_label: CommandOptions
---

# CommandOptions interface

#### Signature:

```typescript
export interface CommandOptions
```

## Properties

| Property | Modifiers | Type | Description | Default |
| -------- | --------- | ------ | ----------- | ------- |
| timeout | | number | | |
2 changes: 1 addition & 1 deletion docs/api/puppeteer.connection.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ export declare class Connection extends EventEmitter<CDPSessionEvents>
| [createSession(targetInfo)](./puppeteer.connection.createsession.md) | | |
| [dispose()](./puppeteer.connection.dispose.md) | | |
| [fromSession(session)](./puppeteer.connection.fromsession.md) | <code>static</code> | |
| [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) | | |
12 changes: 7 additions & 5 deletions docs/api/puppeteer.connection.send.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,19 @@ sidebar_label: Connection.send
class Connection {
send<T extends keyof ProtocolMapping.Commands>(
method: T,
...paramArgs: ProtocolMapping.Commands[T]['paramsType']
params?: ProtocolMapping.Commands[T]['paramsType'][0],
options?: CommandOptions
): Promise<ProtocolMapping.Commands[T]['returnType']>;
}
```

## 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:**

Expand Down
10 changes: 9 additions & 1 deletion packages/puppeteer-core/src/api/CDPSession.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -97,7 +104,8 @@ export abstract class CDPSession extends EventEmitter<CDPSessionEvents> {

abstract send<T extends keyof ProtocolMapping.Commands>(
method: T,
...paramArgs: ProtocolMapping.Commands[T]['paramsType']
params?: ProtocolMapping.Commands[T]['paramsType'][0],
options?: CommandOptions
): Promise<ProtocolMapping.Commands[T]['returnType']>;

/**
Expand Down
9 changes: 5 additions & 4 deletions packages/puppeteer-core/src/cdp/CDPSession.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -91,7 +92,8 @@ export class CdpCDPSession extends CDPSession {

override send<T extends keyof ProtocolMapping.Commands>(
method: T,
...paramArgs: ProtocolMapping.Commands[T]['paramsType']
params?: ProtocolMapping.Commands[T]['paramsType'][0],
options?: CommandOptions
): Promise<ProtocolMapping.Commands[T]['returnType']> {
if (!this.#connection) {
return Promise.reject(
Expand All @@ -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
);
}

Expand Down
12 changes: 7 additions & 5 deletions packages/puppeteer-core/src/cdp/Connection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -104,16 +105,16 @@ export class Connection extends EventEmitter<CDPSessionEvents> {

send<T extends keyof ProtocolMapping.Commands>(
method: T,
...paramArgs: ProtocolMapping.Commands[T]['paramsType']
params?: ProtocolMapping.Commands[T]['paramsType'][0],
options?: CommandOptions
): Promise<ProtocolMapping.Commands[T]['returnType']> {
// There is only ever 1 param arg passed, but the Protocol defines it as an
// array of 0 or 1 items See this comment:
// https://github.com/ChromeDevTools/devtools-protocol/pull/113#issuecomment-412603285
// 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);
}

/**
Expand All @@ -123,9 +124,10 @@ export class Connection extends EventEmitter<CDPSessionEvents> {
callbacks: CallbackRegistry,
method: T,
params: ProtocolMapping.Commands[T]['paramsType'][0],
sessionId?: string
sessionId?: string,
options?: CommandOptions
): Promise<ProtocolMapping.Commands[T]['returnType']> {
return callbacks.create(method, this.#timeout, id => {
return callbacks.create(method, options?.timeout ?? this.#timeout, id => {
const stringifiedMessage = JSON.stringify({
method,
params,
Expand Down
6 changes: 6 additions & 0 deletions test/TestExpectations.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down
20 changes: 20 additions & 0 deletions test/src/cdp/CDPSession.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down

0 comments on commit c660d40

Please sign in to comment.