Skip to content

chore: make sure dispatchers work with SdkObjects #36158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/playwright-core/src/server/android/android.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ export interface DeviceBackend {
}

export interface SocketBackend extends EventEmitter {
guid: string;
write(data: Buffer): Promise<void>;
close(): void;
}
Expand Down
2 changes: 0 additions & 2 deletions packages/playwright-core/src/server/android/backendAdb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { EventEmitter } from 'events';
import net from 'net';

import { assert } from '../../utils/isomorphic/assert';
import { createGuid } from '../utils/crypto';
import { debug } from '../../utilsBundle';

import type { Backend, DeviceBackend, SocketBackend } from './android';
Expand Down Expand Up @@ -117,7 +116,6 @@ function encodeMessage(message: string): Buffer {
}

class BufferedSocketWrapper extends EventEmitter implements SocketBackend {
readonly guid = createGuid();
private _socket: net.Socket;
private _buffer = Buffer.from([]);
private _isSocket = false;
Expand Down
3 changes: 1 addition & 2 deletions packages/playwright-core/src/server/browserType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export abstract class BrowserType extends SdkObject {
super(parent, 'browser-type');
this.attribution.browserType = this;
this._name = browserName;
this.logName = 'browser';
}

executablePath(): string {
Expand All @@ -85,7 +86,6 @@ export abstract class BrowserType extends SdkObject {
async launch(metadata: CallMetadata, options: types.LaunchOptions, protocolLogger?: types.ProtocolLogger): Promise<Browser> {
options = this._validateLaunchOptions(options);
const controller = new ProgressController(metadata, this);
controller.setLogName('browser');
const browser = await controller.run(progress => {
const seleniumHubUrl = (options as any).__testHookSeleniumRemoteURL || process.env.SELENIUM_REMOTE_URL;
if (seleniumHubUrl)
Expand All @@ -98,7 +98,6 @@ export abstract class BrowserType extends SdkObject {
async launchPersistentContext(metadata: CallMetadata, userDataDir: string, options: channels.BrowserTypeLaunchPersistentContextOptions & { timeout: number, cdpPort?: number, internalIgnoreHTTPSErrors?: boolean }): Promise<BrowserContext> {
const launchOptions = this._validateLaunchOptions(options);
const controller = new ProgressController(metadata, this);
controller.setLogName('browser');
const browser = await controller.run(async progress => {
// Note: Any initial TLS requests will fail since we rely on the Page/Frames initialize which sets ignoreHTTPSErrors.
let clientCertificatesProxy: ClientCertificatesProxy | undefined;
Expand Down
1 change: 0 additions & 1 deletion packages/playwright-core/src/server/chromium/chromium.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ export class Chromium extends BrowserType {

override async connectOverCDP(metadata: CallMetadata, endpointURL: string, options: { slowMo?: number, headers?: types.HeadersArray, timeout: number }) {
const controller = new ProgressController(metadata, this);
controller.setLogName('browser');
return controller.run(async progress => {
return await this._connectOverCDPInternal(progress, endpointURL, options);
}, options.timeout);
Expand Down
2 changes: 1 addition & 1 deletion packages/playwright-core/src/server/chromium/crBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export class CRBrowser extends Browser {
static async connect(parent: SdkObject, transport: ConnectionTransport, options: BrowserOptions, devtools?: CRDevTools): Promise<CRBrowser> {
// Make a copy in case we need to update `headful` property below.
options = { ...options };
const connection = new CRConnection(transport, options.protocolLogger, options.browserLogsCollector);
const connection = new CRConnection(parent, transport, options.protocolLogger, options.browserLogsCollector);
const browser = new CRBrowser(parent, connection, options);
browser._devtools = devtools;
if (browser.isClank())
Expand Down
19 changes: 8 additions & 11 deletions packages/playwright-core/src/server/chromium/crConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,11 @@
* limitations under the License.
*/

import { EventEmitter } from 'events';

import { assert, eventsHelper } from '../../utils';
import { debugLogger } from '../utils/debugLogger';
import { helper } from '../helper';
import { ProtocolError } from '../protocolError';
import { SdkObject } from '../instrumentation';

import type { RegisteredListener } from '../../utils';
import type { ConnectionTransport, ProtocolRequest, ProtocolResponse } from '../transport';
Expand All @@ -37,7 +36,7 @@ export const ConnectionEvents = {
// should ignore.
export const kBrowserCloseMessageId = -9999;

export class CRConnection extends EventEmitter {
export class CRConnection extends SdkObject {
private _lastId = 0;
private readonly _transport: ConnectionTransport;
readonly _sessions = new Map<string, CRSession>();
Expand All @@ -47,8 +46,8 @@ export class CRConnection extends EventEmitter {
readonly rootSession: CRSession;
_closed = false;

constructor(transport: ConnectionTransport, protocolLogger: ProtocolLogger, browserLogsCollector: RecentLogsCollector) {
super();
constructor(parent: SdkObject, transport: ConnectionTransport, protocolLogger: ProtocolLogger, browserLogsCollector: RecentLogsCollector) {
super(parent, 'cr-connection');
this.setMaxListeners(0);
this._transport = transport;
this._protocolLogger = protocolLogger;
Expand Down Expand Up @@ -101,7 +100,7 @@ export class CRConnection extends EventEmitter {

type SessionEventListener = (method: string, params?: Object) => void;

export class CRSession extends EventEmitter {
export class CRSession extends SdkObject {
private readonly _connection: CRConnection;
private _eventListener?: SessionEventListener;
private readonly _callbacks = new Map<number, { resolve: (o: any) => void, reject: (e: ProtocolError) => void, error: ProtocolError }>();
Expand All @@ -116,7 +115,7 @@ export class CRSession extends EventEmitter {
override once: <T extends keyof Protocol.Events | symbol>(event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this;

constructor(connection: CRConnection, parentSession: CRSession | null, sessionId: string, eventListener?: SessionEventListener) {
super();
super(connection, 'cr-session');
this.setMaxListeners(0);
this._connection = connection;
this._parentSession = parentSession;
Expand Down Expand Up @@ -203,19 +202,17 @@ export class CRSession extends EventEmitter {
}
}

export class CDPSession extends EventEmitter {
export class CDPSession extends SdkObject {
static Events = {
Event: 'event',
Closed: 'close',
};

readonly guid: string;
private _session: CRSession;
private _listeners: RegisteredListener[] = [];

constructor(parentSession: CRSession, sessionId: string) {
super();
this.guid = `cdp-session@${sessionId}`;
super(parentSession, 'cdp-session');
this._session = parentSession.createChildSession(sessionId, (method, params) => this.emit(CDPSession.Events.Event, { method, params }));
this._listeners = [eventsHelper.addEventListener(parentSession, 'Target.detachedFromTarget', (event: Protocol.Target.detachedFromTargetPayload) => {
if (event.sessionId === sessionId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import { BrowserContextDispatcher } from './browserContextDispatcher';
import { Dispatcher } from './dispatcher';
import { AndroidDevice } from '../android/android';
import { eventsHelper } from '../utils/eventsHelper';
import { SdkObject } from '../instrumentation';

import type { RootDispatcher } from './dispatcher';
import type { Android, SocketBackend } from '../android/android';
Expand Down Expand Up @@ -145,7 +147,7 @@ export class AndroidDeviceDispatcher extends Dispatcher<AndroidDevice, channels.

async open(params: channels.AndroidDeviceOpenParams, metadata: CallMetadata): Promise<channels.AndroidDeviceOpenResult> {
const socket = await this._object.open(params.command);
return { socket: new AndroidSocketDispatcher(this, socket) };
return { socket: new AndroidSocketDispatcher(this, new SocketSdkObject(this._object, socket)) };
}

async installApk(params: channels.AndroidDeviceInstallApkParams) {
Expand All @@ -170,10 +172,33 @@ export class AndroidDeviceDispatcher extends Dispatcher<AndroidDevice, channels.
}
}

export class AndroidSocketDispatcher extends Dispatcher<SocketBackend, channels.AndroidSocketChannel, AndroidDeviceDispatcher> implements channels.AndroidSocketChannel {
class SocketSdkObject extends SdkObject implements SocketBackend {
private _socket: SocketBackend;
private _eventListeners;

constructor(parent: SdkObject, socket: SocketBackend) {
super(parent, 'socket');
this._socket = socket;
this._eventListeners = [
eventsHelper.addEventListener(socket, 'data', data => this.emit('data', data)),
eventsHelper.addEventListener(socket, 'close', () => this.emit('close')),
];
}

async write(data: Buffer) {
await this._socket.write(data);
}

close() {
this._socket.close();
eventsHelper.removeEventListeners(this._eventListeners);
}
}

export class AndroidSocketDispatcher extends Dispatcher<SocketSdkObject, channels.AndroidSocketChannel, AndroidDeviceDispatcher> implements channels.AndroidSocketChannel {
_type_AndroidSocket = true;

constructor(scope: AndroidDeviceDispatcher, socket: SocketBackend) {
constructor(scope: AndroidDeviceDispatcher, socket: SocketSdkObject) {
super(scope, socket, 'AndroidSocket', {});
this.addObjectListener('data', (data: Buffer) => this._dispatchEvent('data', { data }));
this.addObjectListener('close', () => {
Expand Down
34 changes: 16 additions & 18 deletions packages/playwright-core/src/server/dispatchers/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { ValidationError, createMetadataValidator, findValidator } from '../../
import { LongStandingScope, assert, monotonicTime, rewriteErrorMessage } from '../../utils';
import { isUnderTest } from '../utils/debug';
import { TargetClosedError, isTargetClosedError, serializeError } from '../errors';
import { SdkObject } from '../instrumentation';
import { createRootSdkObject, SdkObject } from '../instrumentation';
import { isProtocolError } from '../protocolError';
import { compressCallLog } from '../callLog';

Expand All @@ -44,7 +44,7 @@ function maxDispatchersForBucket(gcBucket: string) {
}[gcBucket] ?? 10000;
}

export class Dispatcher<Type extends { guid: string }, ChannelType, ParentScopeType extends DispatcherScope> extends EventEmitter implements channels.Channel {
export class Dispatcher<Type extends SdkObject, ChannelType, ParentScopeType extends DispatcherScope> extends EventEmitter implements channels.Channel {
readonly connection: DispatcherConnection;
// Parent is always "isScope".
private _parent: ParentScopeType | undefined;
Expand Down Expand Up @@ -161,13 +161,13 @@ export class Dispatcher<Type extends { guid: string }, ChannelType, ParentScopeT
}
}

export type DispatcherScope = Dispatcher<any, any, any>;
export type DispatcherScope = Dispatcher<SdkObject, any, any>;

export class RootDispatcher extends Dispatcher<{ guid: '' }, any, any> {
export class RootDispatcher extends Dispatcher<SdkObject, any, any> {
private _initialized = false;

constructor(connection: DispatcherConnection, private readonly createPlaywright?: (scope: RootDispatcher, options: channels.RootInitializeParams) => Promise<PlaywrightDispatcher>) {
super(connection, { guid: '' }, 'Root', {});
super(connection, createRootSdkObject(), 'Root', {});
}

async initialize(params: channels.RootInitializeParams): Promise<channels.RootInitializeResult> {
Expand Down Expand Up @@ -304,16 +304,16 @@ export class DispatcherConnection {
return;
}

const sdkObject = dispatcher._object instanceof SdkObject ? dispatcher._object : undefined;
const sdkObject = dispatcher._object;
const callMetadata: CallMetadata = {
id: `call@${id}`,
location: validMetadata.location,
title: validMetadata.title,
internal: validMetadata.internal,
stepId: validMetadata.stepId,
objectId: sdkObject?.guid,
pageId: sdkObject?.attribution?.page?.guid,
frameId: sdkObject?.attribution?.frame?.guid,
objectId: sdkObject.guid,
pageId: sdkObject.attribution?.page?.guid,
frameId: sdkObject.attribution?.frame?.guid,
startTime: monotonicTime(),
endTime: 0,
type: dispatcher._type,
Expand All @@ -322,7 +322,7 @@ export class DispatcherConnection {
log: [],
};

if (sdkObject && params?.info?.waitId) {
if (params?.info?.waitId) {
// Process logs for waitForNavigation/waitForLoadState/etc.
const info = params.info;
switch (info.phase) {
Expand All @@ -349,32 +349,30 @@ export class DispatcherConnection {
}
}

await sdkObject?.instrumentation.onBeforeCall(sdkObject, callMetadata);
await sdkObject.instrumentation.onBeforeCall(sdkObject, callMetadata);
const response: any = { id };
try {
const result = await dispatcher._handleCommand(callMetadata, method, validParams);
const validator = findValidator(dispatcher._type, method, 'Result');
response.result = validator(result, '', this._validatorToWireContext());
callMetadata.result = result;
} catch (e) {
if (isTargetClosedError(e) && sdkObject) {
if (isTargetClosedError(e)) {
const reason = closeReason(sdkObject);
if (reason)
rewriteErrorMessage(e, reason);
} else if (isProtocolError(e)) {
if (e.type === 'closed') {
const reason = sdkObject ? closeReason(sdkObject) : undefined;
e = new TargetClosedError(reason, e.browserLogMessage());
} else if (e.type === 'crashed') {
if (e.type === 'closed')
e = new TargetClosedError(closeReason(sdkObject), e.browserLogMessage());
else if (e.type === 'crashed')
rewriteErrorMessage(e, 'Target crashed ' + e.browserLogMessage());
}
}
response.error = serializeError(e);
// The command handler could have set error in the metadata, do not reset it if there was no exception.
callMetadata.error = response.error;
} finally {
callMetadata.endTime = monotonicTime();
await sdkObject?.instrumentation.onAfterCall(sdkObject, callMetadata);
await sdkObject.instrumentation.onAfterCall(sdkObject, callMetadata);
}

if (response.error)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@
*/

import { Dispatcher } from './dispatcher';
import { createGuid } from '../utils/crypto';
import { SdkObject } from '../instrumentation';

import type { LocalUtilsDispatcher } from './localUtilsDispatcher';
import type * as channels from '@protocol/channels';

export class JsonPipeDispatcher extends Dispatcher<{ guid: string }, channels.JsonPipeChannel, LocalUtilsDispatcher> implements channels.JsonPipeChannel {
export class JsonPipeDispatcher extends Dispatcher<SdkObject, channels.JsonPipeChannel, LocalUtilsDispatcher> implements channels.JsonPipeChannel {
_type_JsonPipe = true;
constructor(scope: LocalUtilsDispatcher) {
super(scope, { guid: 'jsonPipe@' + createGuid() }, 'JsonPipe', {});
super(scope, new SdkObject(scope._object, 'jsonPipe'), 'JsonPipe', {});
}

async send(params: channels.JsonPipeSendParams): Promise<channels.JsonPipeSendResult> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,14 @@ import type * as channels from '@protocol/channels';
import type * as http from 'http';
import type { HTTPRequestParams } from '../utils/network';

export class LocalUtilsDispatcher extends Dispatcher<{ guid: string }, channels.LocalUtilsChannel, RootDispatcher> implements channels.LocalUtilsChannel {
export class LocalUtilsDispatcher extends Dispatcher<SdkObject, channels.LocalUtilsChannel, RootDispatcher> implements channels.LocalUtilsChannel {
_type_LocalUtils: boolean;
private _harBackends = new Map<string, HarBackend>();
private _stackSessions = new Map<string, localUtils.StackSession>();

constructor(scope: RootDispatcher, playwright: Playwright) {
const localUtils = new SdkObject(playwright, 'localUtils', 'localUtils');
localUtils.logName = 'browser';
const deviceDescriptors = Object.entries(descriptors)
.map(([name, descriptor]) => ({ name, descriptor }));
super(scope, localUtils, 'LocalUtils', {
Expand Down Expand Up @@ -82,8 +83,7 @@ export class LocalUtilsDispatcher extends Dispatcher<{ guid: string }, channels.
}

async connect(params: channels.LocalUtilsConnectParams, metadata: CallMetadata): Promise<channels.LocalUtilsConnectResult> {
const controller = new ProgressController(metadata, this._object as SdkObject);
controller.setLogName('browser');
const controller = new ProgressController(metadata, this._object);
return await controller.run(async progress => {
const wsHeaders = {
'User-Agent': getUserAgent(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { RequestDispatcher } from './networkDispatchers';
import { ResponseDispatcher } from './networkDispatchers';
import { RouteDispatcher, WebSocketDispatcher } from './networkDispatchers';
import { WebSocketRouteDispatcher } from './webSocketRouteDispatcher';
import { createGuid } from '../utils/crypto';
import { SdkObject } from '../instrumentation';
import { urlMatches } from '../../utils/isomorphic/urlMatch';

import type { Artifact } from '../artifact';
Expand Down Expand Up @@ -403,15 +403,15 @@ export class WorkerDispatcher extends Dispatcher<Worker, channels.WorkerChannel,
}
}

export class BindingCallDispatcher extends Dispatcher<{ guid: string }, channels.BindingCallChannel, PageDispatcher | BrowserContextDispatcher> implements channels.BindingCallChannel {
export class BindingCallDispatcher extends Dispatcher<SdkObject, channels.BindingCallChannel, PageDispatcher | BrowserContextDispatcher> implements channels.BindingCallChannel {
_type_BindingCall = true;
private _resolve: ((arg: any) => void) | undefined;
private _reject: ((error: any) => void) | undefined;
private _promise: Promise<any>;

constructor(scope: PageDispatcher, name: string, needsHandle: boolean, source: { context: BrowserContext, page: Page, frame: Frame }, args: any[]) {
const frameDispatcher = FrameDispatcher.from(scope.parentScope(), source.frame);
super(scope, { guid: 'bindingCall@' + createGuid() }, 'BindingCall', {
super(scope, new SdkObject(scope._object, 'bindingCall'), 'BindingCall', {
frame: frameDispatcher,
name,
args: needsHandle ? undefined : args.map(serializeResult),
Expand Down
Loading
Loading