Skip to content

Commit

Permalink
chore: remove logger infrastructure from server side (#3487)
Browse files Browse the repository at this point in the history
We do not implement LoggerSink on the server, so we can
use a simple debugLogger.
  • Loading branch information
dgozman authored Aug 17, 2020
1 parent f3c2584 commit 1e9c0eb
Show file tree
Hide file tree
Showing 28 changed files with 188 additions and 408 deletions.
5 changes: 1 addition & 4 deletions src/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ import { Page } from './page';
import { EventEmitter } from 'events';
import { Download } from './download';
import { Events } from './events';
import { Loggers } from './logger';
import { ProxySettings } from './types';
import { LoggerSink } from './loggerSink';
import { ChildProcess } from 'child_process';

export interface BrowserProcess {
Expand All @@ -34,7 +32,6 @@ export interface BrowserProcess {

export type BrowserOptions = {
name: string,
loggers: Loggers,
downloadsPath?: string,
headful?: boolean,
persistent?: types.BrowserContextOptions, // Undefined means no persistent context.
Expand All @@ -43,7 +40,7 @@ export type BrowserOptions = {
proxy?: ProxySettings,
};

export type BrowserContextOptions = types.BrowserContextOptions & { logger?: LoggerSink };
export type BrowserContextOptions = types.BrowserContextOptions;

export interface Browser extends EventEmitter {
newContext(options?: BrowserContextOptions): Promise<BrowserContext>;
Expand Down
18 changes: 5 additions & 13 deletions src/browserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,9 @@ import * as types from './types';
import { Events } from './events';
import { Download } from './download';
import { BrowserBase } from './browser';
import { Loggers, Logger } from './logger';
import { EventEmitter } from 'events';
import { ProgressController } from './progress';
import { DebugController } from './debug/debugController';
import { LoggerSink } from './loggerSink';

export interface BrowserContext extends EventEmitter {
setDefaultNavigationTimeout(timeout: number): void;
Expand All @@ -53,12 +51,10 @@ export interface BrowserContext extends EventEmitter {
close(): Promise<void>;
}

type BrowserContextOptions = types.BrowserContextOptions & { logger?: LoggerSink };

export abstract class BrowserContextBase extends EventEmitter implements BrowserContext {
readonly _timeoutSettings = new TimeoutSettings();
readonly _pageBindings = new Map<string, PageBinding>();
readonly _options: BrowserContextOptions;
readonly _options: types.BrowserContextOptions;
_routes: { url: types.URLMatch, handler: network.RouteHandler }[] = [];
private _isPersistentContext: boolean;
private _closedStatus: 'open' | 'closing' | 'closed' = 'open';
Expand All @@ -67,14 +63,11 @@ export abstract class BrowserContextBase extends EventEmitter implements Browser
readonly _permissions = new Map<string, string[]>();
readonly _downloads = new Set<Download>();
readonly _browserBase: BrowserBase;
readonly _apiLogger: Logger;

constructor(browserBase: BrowserBase, options: BrowserContextOptions, isPersistentContext: boolean) {
constructor(browserBase: BrowserBase, options: types.BrowserContextOptions, isPersistentContext: boolean) {
super();
this._browserBase = browserBase;
this._options = options;
const loggers = options.logger ? new Loggers(options.logger) : browserBase._options.loggers;
this._apiLogger = loggers.api;
this._isPersistentContext = isPersistentContext;
this._closePromise = new Promise(fulfill => this._closePromiseFulfill = fulfill);
}
Expand All @@ -86,7 +79,7 @@ export abstract class BrowserContextBase extends EventEmitter implements Browser

async waitForEvent(event: string, optionsOrPredicate: types.WaitForEventOptions = {}): Promise<any> {
const options = typeof optionsOrPredicate === 'function' ? { predicate: optionsOrPredicate } : optionsOrPredicate;
const progressController = new ProgressController(this._apiLogger, this._timeoutSettings.timeout(options));
const progressController = new ProgressController(this._timeoutSettings.timeout(options));
if (event !== Events.BrowserContext.Close)
this._closePromise.then(error => progressController.abort(error));
return progressController.run(progress => helper.waitForEvent(progress, this, event, options.predicate).promise);
Expand Down Expand Up @@ -248,10 +241,10 @@ export function assertBrowserContextIsNotOwned(context: BrowserContextBase) {
}
}

export function validateBrowserContextOptions(options: BrowserContextOptions): BrowserContextOptions {
export function validateBrowserContextOptions(options: types.BrowserContextOptions): types.BrowserContextOptions {
// Copy all fields manually to strip any extra junk.
// Especially useful when we share context and launch options for launchPersistent.
const result: BrowserContextOptions = {
const result: types.BrowserContextOptions = {
ignoreHTTPSErrors: options.ignoreHTTPSErrors,
bypassCSP: options.bypassCSP,
locale: options.locale,
Expand All @@ -269,7 +262,6 @@ export function validateBrowserContextOptions(options: BrowserContextOptions): B
deviceScaleFactor: options.deviceScaleFactor,
isMobile: options.isMobile,
hasTouch: options.hasTouch,
logger: options.logger,
};
if (result.viewport === null && result.deviceScaleFactor !== undefined)
throw new Error(`"deviceScaleFactor" option is not supported with null "viewport"`);
Expand Down
2 changes: 1 addition & 1 deletion src/chromium/crBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export class CRBrowser extends BrowserBase {
private _tracingClient: CRSession | undefined;

static async connect(transport: ConnectionTransport, options: BrowserOptions, devtools?: CRDevTools): Promise<CRBrowser> {
const connection = new CRConnection(SlowMoTransport.wrap(transport, options.slowMo), options.loggers);
const connection = new CRConnection(SlowMoTransport.wrap(transport, options.slowMo));
const browser = new CRBrowser(connection, options);
browser._devtools = devtools;
const session = connection.rootSession;
Expand Down
21 changes: 7 additions & 14 deletions src/chromium/crConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@
* limitations under the License.
*/

import { assert } from '../helper';
import { assert, debugLogger } from '../helper';
import { ConnectionTransport, ProtocolRequest, ProtocolResponse } from '../transport';
import { Protocol } from './protocol';
import { EventEmitter } from 'events';
import { Loggers, Logger } from '../logger';
import { rewriteErrorMessage } from '../utils/stackTrace';

export const ConnectionEvents = {
Expand All @@ -36,19 +35,16 @@ export class CRConnection extends EventEmitter {
private readonly _sessions = new Map<string, CRSession>();
readonly rootSession: CRSession;
_closed = false;
readonly _logger: Logger;

constructor(transport: ConnectionTransport, loggers: Loggers) {
constructor(transport: ConnectionTransport) {
super();
this._transport = transport;
this._logger = loggers.protocol;
this._transport.onmessage = this._onMessage.bind(this);
this._transport.onclose = this._onClose.bind(this);
this.rootSession = new CRSession(this, '', 'browser', '');
this._sessions.set('', this.rootSession);
}


static fromSession(session: CRSession): CRConnection {
return session._connection!;
}
Expand All @@ -62,15 +58,15 @@ export class CRConnection extends EventEmitter {
const message: ProtocolRequest = { id, method, params };
if (sessionId)
message.sessionId = sessionId;
if (this._logger.isEnabled())
this._logger.info('SEND ► ' + rewriteInjectedScriptEvaluationLog(message));
if (debugLogger.isEnabled('protocol'))
debugLogger.log('protocol', 'SEND ► ' + rewriteInjectedScriptEvaluationLog(message));
this._transport.send(message);
return id;
}

async _onMessage(message: ProtocolResponse) {
if (this._logger.isEnabled())
this._logger.info('◀ RECV ' + JSON.stringify(message));
if (debugLogger.isEnabled('protocol'))
debugLogger.log('protocol', '◀ RECV ' + JSON.stringify(message));
if (message.id === kBrowserCloseMessageId)
return;
if (message.method === 'Target.attachedToTarget') {
Expand Down Expand Up @@ -167,10 +163,7 @@ export class CRSession extends EventEmitter {
}

_sendMayFail<T extends keyof Protocol.CommandParameters>(method: T, params?: Protocol.CommandParameters[T]): Promise<Protocol.CommandReturnValues[T] | void> {
return this.send(method, params).catch((error: Error) => {
if (this._connection)
this._connection._logger.error(error);
});
return this.send(method, params).catch((error: Error) => debugLogger.log('error', error));
}

_onMessage(object: ProtocolResponse) {
Expand Down
1 change: 0 additions & 1 deletion src/chromium/crPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,6 @@ class FrameSession {

_onDialog(event: Protocol.Page.javascriptDialogOpeningPayload) {
this._page.emit(Events.Page.Dialog, new dialog.Dialog(
this._page._logger,
event.type,
event.message,
async (accept: boolean, promptText?: string) => {
Expand Down
13 changes: 5 additions & 8 deletions src/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,28 +15,25 @@
* limitations under the License.
*/

import { assert } from './helper';
import { Logger } from './logger';
import { assert, debugLogger } from './helper';

type OnHandle = (accept: boolean, promptText?: string) => Promise<void>;

export type DialogType = 'alert' | 'beforeunload' | 'confirm' | 'prompt';

export class Dialog {
private _logger: Logger;
private _type: string;
private _message: string;
private _onHandle: OnHandle;
private _handled = false;
private _defaultValue: string;

constructor(logger: Logger, type: string, message: string, onHandle: OnHandle, defaultValue?: string) {
this._logger = logger;
constructor(type: string, message: string, onHandle: OnHandle, defaultValue?: string) {
this._type = type;
this._message = message;
this._onHandle = onHandle;
this._defaultValue = defaultValue || '';
this._logger.info(` ${this._preview()} was shown`);
debugLogger.log('api', ` ${this._preview()} was shown`);
}

type(): string {
Expand All @@ -54,14 +51,14 @@ export class Dialog {
async accept(promptText: string | undefined) {
assert(!this._handled, 'Cannot accept dialog which is already handled!');
this._handled = true;
this._logger.info(` ${this._preview()} was accepted`);
debugLogger.log('api', ` ${this._preview()} was accepted`);
await this._onHandle(true, promptText);
}

async dismiss() {
assert(!this._handled, 'Cannot dismiss dialog which is already handled!');
this._handled = true;
this._logger.info(` ${this._preview()} was dismissed`);
debugLogger.log('api', ` ${this._preview()} was dismissed`);
await this._onHandle(false);
}

Expand Down
Loading

0 comments on commit 1e9c0eb

Please sign in to comment.