Skip to content

Commit

Permalink
chore: introduce session.sendMayFail to ease error logging (#2480)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dgozman
dgozman authored Jun 5, 2020
1 parent fc2432a commit c08da50
Show file tree
Hide file tree
Showing 12 changed files with 98 additions and 111 deletions.
5 changes: 2 additions & 3 deletions src/chromium/crBrowser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import { readProtocolStream } from './crProtocolHelper';
import { Events } from './events';
import { Protocol } from './protocol';
import { CRExecutionContext } from './crExecutionContext';
import { logError } from '../logger';
import { CRDevTools } from '../debug/crDevTools';

export class CRBrowser extends BrowserBase {
Expand Down Expand Up @@ -133,8 +132,8 @@ export class CRBrowser extends BrowserBase {
if (targetInfo.type === 'other' || !context) {
if (waitingForDebugger) {
// Ideally, detaching should resume any target, but there is a bug in the backend.
session.send('Runtime.runIfWaitingForDebugger').catch(logError(this)).then(() => {
this._session.send('Target.detachFromTarget', { sessionId }).catch(logError(this));
session._sendMayFail('Runtime.runIfWaitingForDebugger').then(() => {
this._session._sendMayFail('Target.detachFromTarget', { sessionId });
});
}
return;
Expand Down
11 changes: 9 additions & 2 deletions src/chromium/crConnection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { assert } from '../helper';
import { ConnectionTransport, ProtocolRequest, ProtocolResponse, protocolLog } from '../transport';
import { Protocol } from './protocol';
import { EventEmitter } from 'events';
import { InnerLogger } from '../logger';
import { InnerLogger, errorLog } from '../logger';
import { rewriteErrorMessage } from '../debug/stackTrace';

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

constructor(transport: ConnectionTransport, logger: InnerLogger) {
super();
Expand Down Expand Up @@ -165,6 +165,13 @@ 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 => {
if (this._connection)
this._connection._logger._log(errorLog, error, []);
});
}

_onMessage(object: ProtocolResponse) {
if (object.id && this._callbacks.has(object.id)) {
const callback = this._callbacks.get(object.id)!;
Expand Down
32 changes: 11 additions & 21 deletions src/chromium/crCoverage.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import { assert, helper, RegisteredListener } from '../helper';
import { Protocol } from './protocol';
import * as types from '../types';
import * as debugSupport from '../debug/debugSupport';
import { logError, InnerLogger } from '../logger';

type JSRange = {
startOffset: number,
Expand Down Expand Up @@ -50,9 +49,9 @@ export class CRCoverage {
private _jsCoverage: JSCoverage;
private _cssCoverage: CSSCoverage;

constructor(client: CRSession, logger: InnerLogger) {
this._jsCoverage = new JSCoverage(client, logger);
this._cssCoverage = new CSSCoverage(client, logger);
constructor(client: CRSession) {
this._jsCoverage = new JSCoverage(client);
this._cssCoverage = new CSSCoverage(client);
}

async startJSCoverage(options?: types.JSCoverageOptions) {
Expand Down Expand Up @@ -80,11 +79,9 @@ class JSCoverage {
_eventListeners: RegisteredListener[];
_resetOnNavigation: boolean;
_reportAnonymousScripts = false;
private _logger: InnerLogger;

constructor(client: CRSession, logger: InnerLogger) {
constructor(client: CRSession) {
this._client = client;
this._logger = logger;
this._enabled = false;
this._scriptIds = new Set();
this._scriptSources = new Map();
Expand Down Expand Up @@ -131,13 +128,10 @@ class JSCoverage {
// Ignore other anonymous scripts unless the reportAnonymousScripts option is true.
if (!event.url && !this._reportAnonymousScripts)
return;
try {
const response = await this._client.send('Debugger.getScriptSource', {scriptId: event.scriptId});
// This might fail if the page has already navigated away.
const response = await this._client._sendMayFail('Debugger.getScriptSource', {scriptId: event.scriptId});
if (response)
this._scriptSources.set(event.scriptId, response.scriptSource);
} catch (e) {
// This might happen if the page has already navigated away.
logError(this._logger)(e);
}
}

async stop(): Promise<JSCoverageEntry[]> {
Expand Down Expand Up @@ -174,11 +168,9 @@ class CSSCoverage {
_stylesheetSources: Map<string, string>;
_eventListeners: RegisteredListener[];
_resetOnNavigation: boolean;
private _logger: InnerLogger;

constructor(client: CRSession, logger: InnerLogger) {
constructor(client: CRSession) {
this._client = client;
this._logger = logger;
this._enabled = false;
this._stylesheetURLs = new Map();
this._stylesheetSources = new Map();
Expand Down Expand Up @@ -216,13 +208,11 @@ class CSSCoverage {
// Ignore anonymous scripts
if (!header.sourceURL)
return;
try {
const response = await this._client.send('CSS.getStyleSheetText', {styleSheetId: header.styleSheetId});
// This might fail if the page has already navigated away.
const response = await this._client._sendMayFail('CSS.getStyleSheetText', {styleSheetId: header.styleSheetId});
if (response) {
this._stylesheetURLs.set(header.styleSheetId, header.sourceURL);
this._stylesheetSources.set(header.styleSheetId, response.text);
} catch (e) {
// This might happen if the page has already navigated away.
logError(this._logger)(e);
}
}

Expand Down
39 changes: 16 additions & 23 deletions src/chromium/crNetworkManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import * as network from '../network';
import * as frames from '../frames';
import { Credentials } from '../types';
import { CRPage } from './crPage';
import { logError } from '../logger';

export class CRNetworkManager {
private _client: CRSession;
Expand Down Expand Up @@ -130,26 +129,26 @@ export class CRNetworkManager {
this._attemptedAuthentications.add(event.requestId);
}
const {username, password} = this._credentials || {username: undefined, password: undefined};
this._client.send('Fetch.continueWithAuth', {
this._client._sendMayFail('Fetch.continueWithAuth', {
requestId: event.requestId,
authChallengeResponse: { response, username, password },
}).catch(logError(this._page));
});
}

_onRequestPaused(workerFrame: frames.Frame | undefined, event: Protocol.Fetch.requestPausedPayload) {
if (!this._userRequestInterceptionEnabled && this._protocolRequestInterceptionEnabled) {
this._client.send('Fetch.continueRequest', {
this._client._sendMayFail('Fetch.continueRequest', {
requestId: event.requestId
}).catch(logError(this._page));
});
}
if (!event.networkId) {
// Fetch without networkId means that request was not recongnized by inspector, and
// it will never receive Network.requestWillBeSent. Most likely, this is an internal request
// that we can safely fail.
this._client.send('Fetch.failRequest', {
this._client._sendMayFail('Fetch.failRequest', {
requestId: event.requestId,
errorReason: 'Aborted',
}).catch(logError(this._page));
});
return;
}
if (event.request.url.startsWith('data:'))
Expand Down Expand Up @@ -189,7 +188,7 @@ export class CRNetworkManager {

if (!frame) {
if (requestPausedEvent)
this._client.send('Fetch.continueRequest', { requestId: requestPausedEvent.requestId }).catch(logError(this._page));
this._client._sendMayFail('Fetch.continueRequest', { requestId: requestPausedEvent.requestId });
return;
}
const isNavigationRequest = requestWillBeSentEvent.requestId === requestWillBeSentEvent.loaderId && requestWillBeSentEvent.type === 'Document';
Expand Down Expand Up @@ -325,15 +324,13 @@ class InterceptableRequest implements network.RouteDelegate {
}

async continue(overrides: { method?: string; headers?: network.Headers; postData?: string } = {}) {
await this._client.send('Fetch.continueRequest', {
// In certain cases, protocol will return error if the request was already canceled
// or the page was closed. We should tolerate these errors.
await this._client._sendMayFail('Fetch.continueRequest', {
requestId: this._interceptionId!,
headers: overrides.headers ? headersArray(overrides.headers) : undefined,
method: overrides.method,
postData: overrides.postData
}).catch(error => {
// In certain cases, protocol will return error if the request was already canceled
// or the page was closed. We should tolerate these errors.
logError(this.request._page)(error);
});
}

Expand All @@ -350,29 +347,25 @@ class InterceptableRequest implements network.RouteDelegate {
if (responseBody && !('content-length' in responseHeaders))
responseHeaders['content-length'] = String(Buffer.byteLength(responseBody));

await this._client.send('Fetch.fulfillRequest', {
// In certain cases, protocol will return error if the request was already canceled
// or the page was closed. We should tolerate these errors.
await this._client._sendMayFail('Fetch.fulfillRequest', {
requestId: this._interceptionId!,
responseCode: response.status || 200,
responsePhrase: network.STATUS_TEXTS[String(response.status || 200)],
responseHeaders: headersArray(responseHeaders),
body: responseBody ? responseBody.toString('base64') : undefined,
}).catch(error => {
// In certain cases, protocol will return error if the request was already canceled
// or the page was closed. We should tolerate these errors.
logError(this.request._page)(error);
});
}

async abort(errorCode: string = 'failed') {
const errorReason = errorReasons[errorCode];
assert(errorReason, 'Unknown error code: ' + errorCode);
await this._client.send('Fetch.failRequest', {
// In certain cases, protocol will return error if the request was already canceled
// or the page was closed. We should tolerate these errors.
await this._client._sendMayFail('Fetch.failRequest', {
requestId: this._interceptionId!,
errorReason
}).catch(error => {
// In certain cases, protocol will return error if the request was already canceled
// or the page was closed. We should tolerate these errors.
logError(this.request._page)(error);
});
}
}
Expand Down
35 changes: 17 additions & 18 deletions src/chromium/crPage.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import { CRBrowserContext } from './crBrowser';
import * as types from '../types';
import { ConsoleMessage } from '../console';
import { NotConnectedError } from '../errors';
import { logError } from '../logger';
import * as debugSupport from '../debug/debugSupport';
import { rewriteErrorMessage } from '../debug/stackTrace';

Expand Down Expand Up @@ -70,7 +69,7 @@ export class CRPage implements PageDelegate {
this.rawKeyboard = new RawKeyboardImpl(client);
this.rawMouse = new RawMouseImpl(client);
this._pdf = new CRPDF(client);
this._coverage = new CRCoverage(client, browserContext);
this._coverage = new CRCoverage(client);
this._browserContext = browserContext;
this._page = new Page(this, browserContext);
this._mainFrameSession = new FrameSession(this, client, targetId, null);
Expand Down Expand Up @@ -121,7 +120,7 @@ export class CRPage implements PageDelegate {

async exposeBinding(binding: PageBinding) {
await this._forAllFrameSessions(frame => frame._initBinding(binding));
await Promise.all(this._page.frames().map(frame => frame.evaluate(binding.source).catch(logError(this._page))));
await Promise.all(this._page.frames().map(frame => frame.evaluate(binding.source).catch(e => {})));
}

async updateExtraHTTPHeaders(): Promise<void> {
Expand Down Expand Up @@ -384,13 +383,13 @@ class FrameSession {
const localFrames = this._isMainFrame() ? this._page.frames() : [ this._page._frameManager.frame(this._targetId)! ];
for (const frame of localFrames) {
// Note: frames might be removed before we send these.
this._client.send('Page.createIsolatedWorld', {
this._client._sendMayFail('Page.createIsolatedWorld', {
frameId: frame._id,
grantUniveralAccess: true,
worldName: UTILITY_WORLD_NAME,
}).catch(logError(this._page));
});
for (const binding of this._crPage._browserContext._pageBindings.values())
frame.evaluate(binding.source).catch(logError(this._page));
frame.evaluate(binding.source).catch(e => {});
}
const isInitialEmptyPage = this._isMainFrame() && this._page.mainFrame().url() === ':';
if (isInitialEmptyPage) {
Expand Down Expand Up @@ -560,8 +559,8 @@ class FrameSession {

if (event.targetInfo.type !== 'worker') {
// Ideally, detaching should resume any target, but there is a bug in the backend.
session.send('Runtime.runIfWaitingForDebugger').catch(logError(this._page)).then(() => {
this._client.send('Target.detachFromTarget', { sessionId: event.sessionId }).catch(logError(this._page));
session._sendMayFail('Runtime.runIfWaitingForDebugger').then(() => {
this._client._sendMayFail('Target.detachFromTarget', { sessionId: event.sessionId });
});
return;
}
Expand All @@ -573,10 +572,10 @@ class FrameSession {
worker._createExecutionContext(new CRExecutionContext(session, event.context));
});
Promise.all([
session.send('Runtime.enable'),
session.send('Network.enable'),
session.send('Runtime.runIfWaitingForDebugger'),
]).catch(logError(this._page)); // This might fail if the target is closed before we initialize.
session._sendMayFail('Runtime.enable'),
session._sendMayFail('Network.enable'),
session._sendMayFail('Runtime.runIfWaitingForDebugger'),
]); // This might fail if the target is closed before we initialize.
session.on('Runtime.consoleAPICalled', event => {
const args = event.args.map(o => worker._existingExecutionContext!.createHandle(o));
this._page._addConsoleMessage(event.type, args, toConsoleMessageLocation(event.stackTrace));
Expand Down Expand Up @@ -816,9 +815,9 @@ class FrameSession {
}

async _getBoundingBox(handle: dom.ElementHandle): Promise<types.Rect | null> {
const result = await this._client.send('DOM.getBoxModel', {
const result = await this._client._sendMayFail('DOM.getBoxModel', {
objectId: handle._objectId
}).catch(logError(this._page));
});
if (!result)
return null;
const quad = result.model.border;
Expand All @@ -843,9 +842,9 @@ class FrameSession {
}

async _getContentQuads(handle: dom.ElementHandle): Promise<types.Quad[] | null> {
const result = await this._client.send('DOM.getContentQuads', {
const result = await this._client._sendMayFail('DOM.getContentQuads', {
objectId: handle._objectId
}).catch(logError(this._page));
});
if (!result)
return null;
return result.quads.map(quad => [
Expand All @@ -864,10 +863,10 @@ class FrameSession {
}

async _adoptBackendNodeId(backendNodeId: Protocol.DOM.BackendNodeId, to: dom.FrameExecutionContext): Promise<dom.ElementHandle> {
const result = await this._client.send('DOM.resolveNode', {
const result = await this._client._sendMayFail('DOM.resolveNode', {
backendNodeId,
executionContextId: (to._delegate as CRExecutionContext)._contextId,
}).catch(logError(this._page));
});
if (!result || result.object.subtype === 'null')
throw new Error('Unable to adopt element handle from a different document');
return to.createHandle(result.object).asElement()!;
Expand Down
10 changes: 8 additions & 2 deletions src/firefox/ffConnection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { EventEmitter } from 'events';
import { assert } from '../helper';
import { ConnectionTransport, ProtocolRequest, ProtocolResponse, protocolLog } from '../transport';
import { Protocol } from './protocol';
import { InnerLogger } from '../logger';
import { InnerLogger, errorLog } from '../logger';
import { rewriteErrorMessage } from '../debug/stackTrace';

export const ConnectionEvents = {
Expand All @@ -34,7 +34,7 @@ export class FFConnection extends EventEmitter {
private _lastId: number;
private _callbacks: Map<number, {resolve: Function, reject: Function, error: Error, method: string}>;
private _transport: ConnectionTransport;
private _logger: InnerLogger;
readonly _logger: InnerLogger;
readonly _sessions: Map<string, FFSession>;
_closed: boolean;

Expand Down Expand Up @@ -185,6 +185,12 @@ export class FFSession 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 => {
this._connection._logger._log(errorLog, error, []);
});
}

dispatchMessage(object: ProtocolResponse) {
if (object.id && this._callbacks.has(object.id)) {
const callback = this._callbacks.get(object.id)!;
Expand Down
Loading

0 comments on commit c08da50

Please sign in to comment.