Skip to content
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

chore: remove logger infrastructure from server side #3487

Merged
merged 1 commit into from
Aug 17, 2020
Merged
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
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'))
dgozman marked this conversation as resolved.
Show resolved Hide resolved
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