Skip to content

Commit

Permalink
feat(rpc): allow sending metadata with rpc calls (#3836)
Browse files Browse the repository at this point in the history
Currently, metadata does only contain the stack trace,
and we send it from the JS client.
  • Loading branch information
dgozman authored Sep 11, 2020
1 parent d2771eb commit 9e41518
Show file tree
Hide file tree
Showing 7 changed files with 189 additions and 163 deletions.
9 changes: 2 additions & 7 deletions src/client/channelOwner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import * as channels from '../protocol/channels';
import type { Connection } from './connection';
import type { Logger } from './types';
import { debugLogger } from '../utils/debugLogger';
import { isUnderTest } from '../utils/utils';
import { rewriteErrorMessage } from '../utils/stackTrace';

export abstract class ChannelOwner<T extends channels.Channel = channels.Channel, Initializer = {}> extends EventEmitter {
private _connection: Connection;
Expand Down Expand Up @@ -89,9 +89,6 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
}

protected async _wrapApiCall<T>(apiName: string, func: () => Promise<T>, logger?: Logger): Promise<T> {
const stackObject: any = {};
Error.captureStackTrace(stackObject);
const stack = stackObject.stack.startsWith('Error') ? stackObject.stack.substring(5) : stackObject.stack;
logger = logger || this._logger;
try {
logApiCall(logger, `=> ${apiName} started`);
Expand All @@ -100,9 +97,7 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
return result;
} catch (e) {
logApiCall(logger, `<= ${apiName} failed`);
const innerStack = (isUnderTest() && e.stack) ? e.stack.substring(e.stack.indexOf(e.message) + e.message.length) : '';
e.message = `${apiName}: ` + e.message;
e.stack = e.message + innerStack + stack;
rewriteErrorMessage(e, `${apiName}: ` + e.message);
throw e;
}
}
Expand Down
15 changes: 13 additions & 2 deletions src/client/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import { FirefoxBrowser } from './firefoxBrowser';
import { debugLogger } from '../utils/debugLogger';
import { SelectorsOwner } from './selectors';
import { Video } from './video';
import { isUnderTest } from '../utils/utils';

class Root extends ChannelOwner<channels.Channel, {}> {
constructor(connection: Connection) {
Expand Down Expand Up @@ -71,12 +72,22 @@ export class Connection {
}

async sendMessageToServer(type: string, guid: string, method: string, params: any): Promise<any> {
const stackObject: any = {};
Error.captureStackTrace(stackObject);
const stack = stackObject.stack.startsWith('Error') ? stackObject.stack.substring(5) : stackObject.stack;
const id = ++this._lastId;
const validated = method === 'debugScopeState' ? params : validateParams(type, method, params);
const converted = { id, guid, method, params: validated };
// Do not include metadata in debug logs to avoid noise.
debugLogger.log('channel:command', converted);
this.onmessage(converted);
return new Promise((resolve, reject) => this._callbacks.set(id, { resolve, reject }));
this.onmessage({ ...converted, metadata: { stack } });
try {
return await new Promise((resolve, reject) => this._callbacks.set(id, { resolve, reject }));
} catch (e) {
const innerStack = (isUnderTest() && e.stack) ? e.stack.substring(e.stack.indexOf(e.message) + e.message.length) : '';
e.stack = e.message + innerStack + stack;
throw e;
}
}

_debugScopeState(): any {
Expand Down
9 changes: 7 additions & 2 deletions src/dispatchers/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import * as channels from '../protocol/channels';
import { serializeError } from '../protocol/serializers';
import { createScheme, Validator, ValidationError } from '../protocol/validator';
import { assert, createGuid, debugAssert, isUnderTest } from '../utils/utils';
import { tOptional } from '../protocol/validatorPrimitives';

export const dispatcherSymbol = Symbol('dispatcher');

Expand Down Expand Up @@ -122,6 +123,7 @@ export class DispatcherConnection {
private _rootDispatcher: Root;
onmessage = (message: object) => {};
private _validateParams: (type: string, method: string, params: any) => any;
private _validateMetadata: (metadata: any) => any;

sendMessageToClient(guid: string, method: string, params: any, disallowDispatchers?: boolean) {
const allowDispatchers = !disallowDispatchers;
Expand Down Expand Up @@ -152,14 +154,17 @@ export class DispatcherConnection {
throw new ValidationError(`Unknown scheme for ${type}.${method}`);
return scheme[name](params, '');
};
this._validateMetadata = (metadata: any): any => {
return tOptional(scheme['Metadata'])(metadata, '');
};
}

rootDispatcher(): Dispatcher<any, any> {
return this._rootDispatcher;
}

async dispatch(message: object) {
const { id, guid, method, params } = message as any;
const { id, guid, method, params, metadata } = message as any;
const dispatcher = this._dispatchers.get(guid);
if (!dispatcher) {
this.onmessage({ id, error: serializeError(new Error('Target browser or context has been closed')) });
Expand All @@ -171,7 +176,7 @@ export class DispatcherConnection {
}
try {
const validated = this._validateParams(dispatcher._type, method, params);
const result = await (dispatcher as any)[method](validated);
const result = await (dispatcher as any)[method](validated, this._validateMetadata(metadata));
this.onmessage({ id, result: this._replaceDispatchersWithGuids(result, true) });
} catch (e) {
this.onmessage({ id, error: serializeError(e) });
Expand Down
Loading

0 comments on commit 9e41518

Please sign in to comment.