From 667445849667d1947c98d134f984f06f566c776a Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 10 Jul 2020 18:00:10 -0700 Subject: [PATCH] feat(rpc): log api calls into LoggerSink (#2904) --- src/rpc/client/browser.ts | 14 ++++++----- src/rpc/client/browserContext.ts | 4 ++-- src/rpc/client/browserServer.ts | 4 ++-- src/rpc/client/browserType.ts | 40 ++++++++++++++++++++------------ src/rpc/client/cdpSession.ts | 4 ++-- src/rpc/client/channelOwner.ts | 29 +++++++++++++++++++---- src/rpc/client/connection.ts | 39 +++++++++++++++---------------- src/rpc/client/consoleMessage.ts | 4 ++-- src/rpc/client/dialog.ts | 4 ++-- src/rpc/client/download.ts | 4 ++-- src/rpc/client/elementHandle.ts | 4 ++-- src/rpc/client/frame.ts | 4 ++-- src/rpc/client/jsHandle.ts | 4 ++-- src/rpc/client/network.ts | 12 +++++----- src/rpc/client/page.ts | 8 +++---- src/rpc/client/playwright.ts | 4 ++-- src/rpc/client/worker.ts | 4 ++-- test/launcher.spec.js | 2 +- test/logger.spec.js | 22 ++++++++++++++---- 19 files changed, 127 insertions(+), 83 deletions(-) diff --git a/src/rpc/client/browser.ts b/src/rpc/client/browser.ts index 0da524c252..2a2e3e5afe 100644 --- a/src/rpc/client/browser.ts +++ b/src/rpc/client/browser.ts @@ -21,6 +21,7 @@ import { Page } from './page'; import { ChannelOwner } from './channelOwner'; import { Events } from '../../events'; import { CDPSession } from './cdpSession'; +import { LoggerSink } from '../../loggerSink'; export class Browser extends ChannelOwner { readonly _contexts = new Set(); @@ -36,8 +37,8 @@ export class Browser extends ChannelOwner { return browser ? Browser.from(browser) : null; } - constructor(parent: ChannelOwner, guid: string, initializer: BrowserInitializer) { - super(parent, guid, initializer, true); + constructor(parent: ChannelOwner, type: string, guid: string, initializer: BrowserInitializer) { + super(parent, type, guid, initializer, true); this._channel.on('close', () => { this._isConnected = false; this.emit(Events.Browser.Disconnected); @@ -47,10 +48,12 @@ export class Browser extends ChannelOwner { this._closedPromise = new Promise(f => this.once(Events.Browser.Disconnected, f)); } - async newContext(options: types.BrowserContextOptions = {}): Promise { - delete (options as any).logger; + async newContext(options: types.BrowserContextOptions & { logger?: LoggerSink } = {}): Promise { + const logger = options.logger; + options = { ...options, logger: undefined }; const context = BrowserContext.from(await this._channel.newContext(options)); this._contexts.add(context); + context._logger = logger || this._logger; context._browser = this; return context; } @@ -59,8 +62,7 @@ export class Browser extends ChannelOwner { return [...this._contexts]; } - async newPage(options: types.BrowserContextOptions = {}): Promise { - delete (options as any).logger; + async newPage(options: types.BrowserContextOptions & { logger?: LoggerSink } = {}): Promise { const context = await this.newContext(options); const page = await context.newPage(); page._ownedContext = context; diff --git a/src/rpc/client/browserContext.ts b/src/rpc/client/browserContext.ts index 7b1b28ba61..26ed48932f 100644 --- a/src/rpc/client/browserContext.ts +++ b/src/rpc/client/browserContext.ts @@ -50,8 +50,8 @@ export class BrowserContext extends ChannelOwner { const page = Page.from(p); this._pages.add(page); diff --git a/src/rpc/client/browserServer.ts b/src/rpc/client/browserServer.ts index eacbbe852a..39bbb2bab5 100644 --- a/src/rpc/client/browserServer.ts +++ b/src/rpc/client/browserServer.ts @@ -24,8 +24,8 @@ export class BrowserServer extends ChannelOwner this.emit(Events.BrowserServer.Close)); } diff --git a/src/rpc/client/browserType.ts b/src/rpc/client/browserType.ts index 07c5a16270..d33ee24b95 100644 --- a/src/rpc/client/browserType.ts +++ b/src/rpc/client/browserType.ts @@ -20,15 +20,16 @@ import { Browser } from './browser'; import { BrowserContext } from './browserContext'; import { ChannelOwner } from './channelOwner'; import { BrowserServer } from './browserServer'; +import { LoggerSink } from '../../loggerSink'; export class BrowserType extends ChannelOwner { - static from(browserTyep: BrowserTypeChannel): BrowserType { - return (browserTyep as any)._object; + static from(browserType: BrowserTypeChannel): BrowserType { + return (browserType as any)._object; } - constructor(parent: ChannelOwner, guid: string, initializer: BrowserTypeInitializer) { - super(parent, guid, initializer); + constructor(parent: ChannelOwner, type: string, guid: string, initializer: BrowserTypeInitializer) { + super(parent, type, guid, initializer); } executablePath(): string { @@ -39,23 +40,32 @@ export class BrowserType extends ChannelOwner { - delete (options as any).logger; - return Browser.from(await this._channel.launch(options)); + async launch(options: types.LaunchOptions & { logger?: LoggerSink } = {}): Promise { + const logger = options.logger; + options = { ...options, logger: undefined }; + const browser = Browser.from(await this._channel.launch(options)); + browser._logger = logger; + return browser; } - async launchServer(options: types.LaunchServerOptions = {}): Promise { - delete (options as any).logger; + async launchServer(options: types.LaunchServerOptions & { logger?: LoggerSink } = {}): Promise { + options = { ...options, logger: undefined }; return BrowserServer.from(await this._channel.launchServer(options)); } - async launchPersistentContext(userDataDir: string, options: types.LaunchOptions & types.BrowserContextOptions = {}): Promise { - delete (options as any).logger; - return BrowserContext.from(await this._channel.launchPersistentContext({ userDataDir, ...options })); + async launchPersistentContext(userDataDir: string, options: types.LaunchOptions & types.BrowserContextOptions & { logger?: LoggerSink } = {}): Promise { + const logger = options.logger; + options = { ...options, logger: undefined }; + const context = BrowserContext.from(await this._channel.launchPersistentContext({ userDataDir, ...options })); + context._logger = logger; + return context; } - async connect(options: types.ConnectOptions): Promise { - delete (options as any).logger; - return Browser.from(await this._channel.connect(options)); + async connect(options: types.ConnectOptions & { logger?: LoggerSink }): Promise { + const logger = options.logger; + options = { ...options, logger: undefined }; + const browser = Browser.from(await this._channel.connect(options)); + browser._logger = logger; + return browser; } } diff --git a/src/rpc/client/cdpSession.ts b/src/rpc/client/cdpSession.ts index 953b6e9b26..170f368a24 100644 --- a/src/rpc/client/cdpSession.ts +++ b/src/rpc/client/cdpSession.ts @@ -29,8 +29,8 @@ export class CDPSession extends ChannelOwner(event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; once: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; - constructor(parent: ChannelOwner, guid: string, initializer: CDPSessionInitializer) { - super(parent, guid, initializer, true); + constructor(parent: ChannelOwner, type: string, guid: string, initializer: CDPSessionInitializer) { + super(parent, type, guid, initializer, true); this._channel.on('event', ({ method, params }) => this.emit(method, params)); this._channel.on('disconnected', () => this._dispose()); diff --git a/src/rpc/client/channelOwner.ts b/src/rpc/client/channelOwner.ts index aba5155d55..0e94b4455a 100644 --- a/src/rpc/client/channelOwner.ts +++ b/src/rpc/client/channelOwner.ts @@ -18,6 +18,7 @@ import { EventEmitter } from 'events'; import { Channel } from '../channels'; import { Connection } from './connection'; import { assert } from '../../helper'; +import { LoggerSink } from '../../loggerSink'; export abstract class ChannelOwner extends EventEmitter { private _connection: Connection; @@ -27,21 +28,25 @@ export abstract class ChannelOwner(); + readonly _type: string; readonly _guid: string; readonly _channel: T; readonly _initializer: Initializer; + _logger: LoggerSink | undefined; - constructor(parent: ChannelOwner | Connection, guid: string, initializer: Initializer, isScope?: boolean) { + constructor(parent: ChannelOwner | Connection, type: string, guid: string, initializer: Initializer, isScope?: boolean) { super(); - this._connection = parent instanceof Connection ? parent : parent._connection; + this._type = type; this._guid = guid; this._isScope = !!isScope; this._parent = parent instanceof Connection ? undefined : parent; this._connection._objects.set(guid, this); - if (this._parent) + if (this._parent) { this._parent._objects.set(guid, this); + this._logger = this._parent._logger; + } const base = new EventEmitter(); this._channel = new Proxy(base, { @@ -60,7 +65,23 @@ export abstract class ChannelOwner this._connection.sendMessageToServer({ guid, method: String(prop), params }); + + return async (params: any) => { + const method = String(prop); + const apiName = this._type + '.' + method; + if (this._logger && this._logger.isEnabled('api', 'info')) + this._logger.log('api', 'info', `=> ${apiName} started`, [], { color: 'cyan' }); + try { + const result = await this._connection.sendMessageToServer({ guid, method: String(prop), params }); + if (this._logger && this._logger.isEnabled('api', 'info')) + this._logger.log('api', 'info', `=> ${apiName} succeeded`, [], { color: 'cyan' }); + return result; + } catch (e) { + if (this._logger && this._logger.isEnabled('api', 'info')) + this._logger.log('api', 'info', `=> ${apiName} failed`, [], { color: 'cyan' }); + throw e; + } + }; }, }); (this._channel as any)._object = this; diff --git a/src/rpc/client/connection.ts b/src/rpc/client/connection.ts index 578bc2b661..b503995e85 100644 --- a/src/rpc/client/connection.ts +++ b/src/rpc/client/connection.ts @@ -36,7 +36,7 @@ import { Channel } from '../channels'; class Root extends ChannelOwner { constructor(connection: Connection) { - super(connection, '', {}, true); + super(connection, '', '', {}, true); } } @@ -131,59 +131,58 @@ export class Connection { initializer = this._replaceGuidsWithChannels(initializer); switch (type) { case 'bindingCall': - result = new BindingCall(parent, guid, initializer); + result = new BindingCall(parent, type, guid, initializer); break; case 'browser': - result = new Browser(parent, guid, initializer); + result = new Browser(parent, type, guid, initializer); break; case 'browserServer': - result = new BrowserServer(parent, guid, initializer); + result = new BrowserServer(parent, type, guid, initializer); break; case 'browserType': - result = new BrowserType(parent, guid, initializer); + result = new BrowserType(parent, type, guid, initializer); break; case 'cdpSession': - // Chromium-specific. - result = new CDPSession(parent, guid, initializer); + result = new CDPSession(parent, type, guid, initializer); break; case 'context': - result = new BrowserContext(parent, guid, initializer); + result = new BrowserContext(parent, type, guid, initializer); break; case 'consoleMessage': - result = new ConsoleMessage(parent, guid, initializer); + result = new ConsoleMessage(parent, type, guid, initializer); break; case 'dialog': - result = new Dialog(parent, guid, initializer); + result = new Dialog(parent, type, guid, initializer); break; case 'download': - result = new Download(parent, guid, initializer); + result = new Download(parent, type, guid, initializer); break; case 'elementHandle': - result = new ElementHandle(parent, guid, initializer); + result = new ElementHandle(parent, type, guid, initializer); break; case 'frame': - result = new Frame(parent, guid, initializer); + result = new Frame(parent, type, guid, initializer); break; case 'jsHandle': - result = new JSHandle(parent, guid, initializer); + result = new JSHandle(parent, type, guid, initializer); break; case 'page': - result = new Page(parent, guid, initializer); + result = new Page(parent, type, guid, initializer); break; case 'playwright': - result = new Playwright(parent, guid, initializer); + result = new Playwright(parent, type, guid, initializer); break; case 'request': - result = new Request(parent, guid, initializer); + result = new Request(parent, type, guid, initializer); break; case 'response': - result = new Response(parent, guid, initializer); + result = new Response(parent, type, guid, initializer); break; case 'route': - result = new Route(parent, guid, initializer); + result = new Route(parent, type, guid, initializer); break; case 'worker': - result = new Worker(parent, guid, initializer); + result = new Worker(parent, type, guid, initializer); break; default: throw new Error('Missing type ' + type); diff --git a/src/rpc/client/consoleMessage.ts b/src/rpc/client/consoleMessage.ts index 82fa8433e3..4a8c98dba4 100644 --- a/src/rpc/client/consoleMessage.ts +++ b/src/rpc/client/consoleMessage.ts @@ -25,8 +25,8 @@ export class ConsoleMessage extends ChannelOwner { return (dialog as any)._object; } - constructor(parent: ChannelOwner, guid: string, initializer: DialogInitializer) { - super(parent, guid, initializer); + constructor(parent: ChannelOwner, type: string, guid: string, initializer: DialogInitializer) { + super(parent, type, guid, initializer); } type(): string { diff --git a/src/rpc/client/download.ts b/src/rpc/client/download.ts index 5c4434c597..56179dbbfc 100644 --- a/src/rpc/client/download.ts +++ b/src/rpc/client/download.ts @@ -24,8 +24,8 @@ export class Download extends ChannelOwner return (download as any)._object; } - constructor(parent: ChannelOwner, guid: string, initializer: DownloadInitializer) { - super(parent, guid, initializer); + constructor(parent: ChannelOwner, type: string, guid: string, initializer: DownloadInitializer) { + super(parent, type, guid, initializer); } url(): string { diff --git a/src/rpc/client/elementHandle.ts b/src/rpc/client/elementHandle.ts index 4dabff91ac..3d3a34a7d7 100644 --- a/src/rpc/client/elementHandle.ts +++ b/src/rpc/client/elementHandle.ts @@ -33,8 +33,8 @@ export class ElementHandle extends JSHandle { return handle ? ElementHandle.from(handle) : null; } - constructor(parent: ChannelOwner, guid: string, initializer: JSHandleInitializer) { - super(parent, guid, initializer); + constructor(parent: ChannelOwner, type: string, guid: string, initializer: JSHandleInitializer) { + super(parent, type, guid, initializer); this._elementChannel = this._channel as ElementHandleChannel; } diff --git a/src/rpc/client/frame.ts b/src/rpc/client/frame.ts index 189ce1e269..4aaa513f4b 100644 --- a/src/rpc/client/frame.ts +++ b/src/rpc/client/frame.ts @@ -48,8 +48,8 @@ export class Frame extends ChannelOwner { return frame ? Frame.from(frame) : null; } - constructor(parent: ChannelOwner, guid: string, initializer: FrameInitializer) { - super(parent, guid, initializer); + constructor(parent: ChannelOwner, type: string, guid: string, initializer: FrameInitializer) { + super(parent, type, guid, initializer); this._parentFrame = Frame.fromNullable(initializer.parentFrame); if (this._parentFrame) this._parentFrame._childFrames.add(this); diff --git a/src/rpc/client/jsHandle.ts b/src/rpc/client/jsHandle.ts index 7f75a607a8..29e0c70f08 100644 --- a/src/rpc/client/jsHandle.ts +++ b/src/rpc/client/jsHandle.ts @@ -46,8 +46,8 @@ export class JSHandle extends ChannelOwner this._preview = preview); } diff --git a/src/rpc/client/network.ts b/src/rpc/client/network.ts index e5d92c504d..d1b47fa571 100644 --- a/src/rpc/client/network.ts +++ b/src/rpc/client/network.ts @@ -57,8 +57,8 @@ export class Request extends ChannelOwner { return request ? Request.from(request) : null; } - constructor(parent: ChannelOwner, guid: string, initializer: RequestInitializer) { - super(parent, guid, initializer); + constructor(parent: ChannelOwner, type: string, guid: string, initializer: RequestInitializer) { + super(parent, type, guid, initializer); this._redirectedFrom = Request.fromNullable(initializer.redirectedFrom); if (this._redirectedFrom) this._redirectedFrom._redirectedTo = this; @@ -137,8 +137,8 @@ export class Route extends ChannelOwner { return (route as any)._object; } - constructor(parent: ChannelOwner, guid: string, initializer: RouteInitializer) { - super(parent, guid, initializer); + constructor(parent: ChannelOwner, type: string, guid: string, initializer: RouteInitializer) { + super(parent, type, guid, initializer); } request(): Request { @@ -170,8 +170,8 @@ export class Response extends ChannelOwner return response ? Response.from(response) : null; } - constructor(parent: ChannelOwner, guid: string, initializer: ResponseInitializer) { - super(parent, guid, initializer); + constructor(parent: ChannelOwner, type: string, guid: string, initializer: ResponseInitializer) { + super(parent, type, guid, initializer); } url(): string { diff --git a/src/rpc/client/page.ts b/src/rpc/client/page.ts index 010cbea275..0e3c8beb88 100644 --- a/src/rpc/client/page.ts +++ b/src/rpc/client/page.ts @@ -68,8 +68,8 @@ export class Page extends ChannelOwner { return page ? Page.from(page) : null; } - constructor(parent: ChannelOwner, guid: string, initializer: PageInitializer) { - super(parent, guid, initializer); + constructor(parent: ChannelOwner, type: string, guid: string, initializer: PageInitializer) { + super(parent, type, guid, initializer); this.accessibility = new Accessibility(this._channel); this.keyboard = new Keyboard(this._channel); this.mouse = new Mouse(this._channel); @@ -522,8 +522,8 @@ export class BindingCall extends ChannelOwner { return (worker as any)._object; } - constructor(parent: ChannelOwner, guid: string, initializer: WorkerInitializer) { - super(parent, guid, initializer); + constructor(parent: ChannelOwner, type: string, guid: string, initializer: WorkerInitializer) { + super(parent, type, guid, initializer); this._channel.on('close', () => { if (this._page) this._page._workers.delete(this); diff --git a/test/launcher.spec.js b/test/launcher.spec.js index f09c8da53f..671ade4fa1 100644 --- a/test/launcher.spec.js +++ b/test/launcher.spec.js @@ -18,7 +18,7 @@ const path = require('path'); const fs = require('fs'); const utils = require('./utils'); -const {FFOX, CHROMIUM, WEBKIT, USES_HOOKS} = utils.testOptions(browserType); +const {FFOX, CHROMIUM, WEBKIT, WIN, USES_HOOKS} = utils.testOptions(browserType); describe('Playwright', function() { describe('browserType.launch', function() { diff --git a/test/logger.spec.js b/test/logger.spec.js index 993161c991..b24b1df1b7 100644 --- a/test/logger.spec.js +++ b/test/logger.spec.js @@ -18,33 +18,45 @@ const fs = require('fs'); const path = require('path'); const {FFOX, CHROMIUM, WEBKIT, CHANNEL} = require('./utils').testOptions(browserType); -describe.skip(CHANNEL)('Logger', function() { +describe('Logger', function() { it('should log', async({browserType, defaultBrowserOptions}) => { const log = []; const browser = await browserType.launch({...defaultBrowserOptions, logger: { log: (name, severity, message) => log.push({name, severity, message}), isEnabled: (name, severity) => severity !== 'verbose' }}); + await browser.newContext(); await browser.close(); expect(log.length > 0).toBeTruthy(); - expect(log.filter(item => item.name.includes('browser')).length > 0).toBeTruthy(); expect(log.filter(item => item.severity === 'info').length > 0).toBeTruthy(); - expect(log.filter(item => item.message.includes('')).length > 0).toBeTruthy(); + if (CHANNEL) { + expect(log.filter(item => item.message.includes('browser.newContext started')).length > 0).toBeTruthy(); + expect(log.filter(item => item.message.includes('browser.newContext succeeded')).length > 0).toBeTruthy(); + } else { + expect(log.filter(item => item.message.includes('browserType.launch started')).length > 0).toBeTruthy(); + expect(log.filter(item => item.message.includes('browserType.launch succeeded')).length > 0).toBeTruthy(); + } }); it('should log context-level', async({browserType, defaultBrowserOptions}) => { const log = []; const browser = await browserType.launch(defaultBrowserOptions); - const page = await browser.newPage({ + const context = await browser.newContext({ logger: { log: (name, severity, message) => log.push({name, severity, message}), isEnabled: (name, severity) => severity !== 'verbose' } }); + const page = await context.newPage(); await page.setContent(''); await page.click('button'); await browser.close(); expect(log.length > 0).toBeTruthy(); - expect(log.filter(item => item.message.includes('waiting for element')).length > 0).toBeTruthy(); + if (CHANNEL) { + expect(log.filter(item => item.message.includes('context.newPage')).length > 0).toBeTruthy(); + expect(log.filter(item => item.message.includes('frame.click')).length > 0).toBeTruthy(); + } else { + expect(log.filter(item => item.message.includes('page.click')).length > 0).toBeTruthy(); + } }); });