chore: move transport to object messages (#1567)

microsoft · Mar 27, 2020 · 00cb4e3 · 00cb4e3
1 parent af7a16c
commit 00cb4e3
Show file tree

Hide file tree

Showing 12 changed files with 133 additions and 121 deletions.
diff --git a/src/chromium/crBrowser.ts b/src/chromium/crBrowser.ts
@@ -243,7 +243,7 @@ export class CRBrowser extends platform.EventEmitter implements Browser {
     return this._clientRootSessionPromise;
   }
 
-  _setDebugFunction(debugFunction: (message: string) => void) {
+  _setDebugFunction(debugFunction: platform.DebuggerType) {
     this._connection._debugProtocol = debugFunction;
   }
 }

diff --git a/src/chromium/crConnection.ts b/src/chromium/crConnection.ts
@@ -17,7 +17,7 @@
 
 import { assert } from '../helper';
 import * as platform from '../platform';
-import { ConnectionTransport } from '../transport';
+import { ConnectionTransport, ProtocolMessage } from '../transport';
 import { Protocol } from './protocol';
 
 export const ConnectionEvents = {
@@ -34,7 +34,7 @@ export class CRConnection extends platform.EventEmitter {
   private readonly _sessions = new Map<string, CRSession>();
   readonly rootSession: CRSession;
   _closed = false;
-  _debugProtocol: (message: string) => void;
+  _debugProtocol: platform.DebuggerType;
 
   constructor(transport: ConnectionTransport) {
     super();
@@ -55,37 +55,37 @@ export class CRConnection extends platform.EventEmitter {
     return this._sessions.get(sessionId) || null;
   }
 
-  _rawSend(sessionId: string, message: any): number {
+  _rawSend(sessionId: string, message: ProtocolMessage): number {
     const id = ++this._lastId;
     message.id = id;
     if (sessionId)
       message.sessionId = sessionId;
-    const data = JSON.stringify(message);
-    this._debugProtocol('SEND ► ' + (rewriteInjectedScriptEvaluationLog(message) || data));
-    this._transport.send(data);
+    if (this._debugProtocol.enabled)
+      this._debugProtocol('SEND ► ' + rewriteInjectedScriptEvaluationLog(message));
+    this._transport.send(message);
     return id;
   }
 
-  async _onMessage(message: string) {
-    this._debugProtocol('◀ RECV ' + message);
-    const object = JSON.parse(message);
-    if (object.id === kBrowserCloseMessageId)
+  async _onMessage(message: ProtocolMessage) {
+    if (this._debugProtocol.enabled)
+      this._debugProtocol('◀ RECV ' + rewriteInjectedScriptEvaluationLog(message));
+    if (message.id === kBrowserCloseMessageId)
       return;
-    if (object.method === 'Target.attachedToTarget') {
-      const sessionId = object.params.sessionId;
-      const rootSessionId = object.sessionId || '';
-      const session = new CRSession(this, rootSessionId, object.params.targetInfo.type, sessionId);
+    if (message.method === 'Target.attachedToTarget') {
+      const sessionId = message.params.sessionId;
+      const rootSessionId = message.sessionId || '';
+      const session = new CRSession(this, rootSessionId, message.params.targetInfo.type, sessionId);
       this._sessions.set(sessionId, session);
-    } else if (object.method === 'Target.detachedFromTarget') {
-      const session = this._sessions.get(object.params.sessionId);
+    } else if (message.method === 'Target.detachedFromTarget') {
+      const session = this._sessions.get(message.params.sessionId);
       if (session) {
         session._onClosed();
-        this._sessions.delete(object.params.sessionId);
+        this._sessions.delete(message.params.sessionId);
       }
     }
-    const session = this._sessions.get(object.sessionId || '');
+    const session = this._sessions.get(message.sessionId || '');
     if (session)
-      session._onMessage(object);
+      session._onMessage(message);
   }
 
   _onClose() {
@@ -156,12 +156,12 @@ export class CRSession extends platform.EventEmitter {
     });
   }
 
-  _onMessage(object: { id?: number; method: string; params: any; error: { message: string; data: any; }; result?: any; }) {
+  _onMessage(object: ProtocolMessage) {
     if (object.id && this._callbacks.has(object.id)) {
       const callback = this._callbacks.get(object.id)!;
       this._callbacks.delete(object.id);
       if (object.error)
-        callback.reject(createProtocolError(callback.error, callback.method, object));
+        callback.reject(createProtocolError(callback.error, callback.method, object.error));
       else
         callback.resolve(object.result);
     } else {
@@ -188,10 +188,10 @@ export class CRSession extends platform.EventEmitter {
   }
 }
 
-function createProtocolError(error: Error, method: string, object: { error: { message: string; data: any; }; }): Error {
-  let message = `Protocol error (${method}): ${object.error.message}`;
-  if ('data' in object.error)
-    message += ` ${object.error.data}`;
+function createProtocolError(error: Error, method: string, protocolError: { message: string; data: any; }): Error {
+  let message = `Protocol error (${method}): ${protocolError.message}`;
+  if ('data' in protocolError)
+    message += ` ${protocolError.data}`;
   return rewriteError(error, message);
 }
 
@@ -200,9 +200,10 @@ function rewriteError(error: Error, message: string): Error {
   return error;
 }
 
-function rewriteInjectedScriptEvaluationLog(message: any): string | undefined {
+function rewriteInjectedScriptEvaluationLog(message: ProtocolMessage): string {
   // Injected script is very long and clutters protocol logs.
   // To increase development velocity, we skip replace it with short description in the log.
   if (message.method === 'Runtime.evaluate' && message.params && message.params.expression && message.params.expression.includes('src/injected/injected.ts'))
     return `{"id":${message.id} [evaluate injected script]}`;
+  return JSON.stringify(message);
 }
diff --git a/src/firefox/ffBrowser.ts b/src/firefox/ffBrowser.ts
@@ -149,7 +149,7 @@ export class FFBrowser extends platform.EventEmitter implements Browser {
     await disconnected;
   }
 
-  _setDebugFunction(debugFunction: (message: string) => void) {
+  _setDebugFunction(debugFunction: platform.DebuggerType) {
     this._connection._debugProtocol = debugFunction;
   }
 }

diff --git a/src/firefox/ffConnection.ts b/src/firefox/ffConnection.ts
@@ -17,7 +17,7 @@
 
 import {assert} from '../helper';
 import * as platform from '../platform';
-import { ConnectionTransport } from '../transport';
+import { ConnectionTransport, ProtocolMessage } from '../transport';
 import { Protocol } from './protocol';
 
 export const ConnectionEvents = {
@@ -33,7 +33,7 @@ export class FFConnection extends platform.EventEmitter {
   private _callbacks: Map<number, {resolve: Function, reject: Function, error: Error, method: string}>;
   private _transport: ConnectionTransport;
   readonly _sessions: Map<string, FFSession>;
-  _debugProtocol: (message: string) => void = platform.debug('pw:protocol');
+  _debugProtocol: platform.DebuggerType = platform.debug('pw:protocol');
   _closed: boolean;
 
   on: <T extends keyof Protocol.Events | symbol>(event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this;
@@ -76,33 +76,33 @@ export class FFConnection extends platform.EventEmitter {
     return ++this._lastId;
   }
 
-  _rawSend(message: any) {
-    const data = JSON.stringify(message);
-    this._debugProtocol('SEND ► ' + (rewriteInjectedScriptEvaluationLog(message) || data));
-    this._transport.send(data);
+  _rawSend(message: ProtocolMessage) {
+    if (this._debugProtocol.enabled)
+      this._debugProtocol('SEND ► ' + rewriteInjectedScriptEvaluationLog(message));
+    this._transport.send(message);
   }
 
-  async _onMessage(message: string) {
-    this._debugProtocol('◀ RECV ' + message);
-    const object = JSON.parse(message);
-    if (object.id === kBrowserCloseMessageId)
+  async _onMessage(message: ProtocolMessage) {
+    if (this._debugProtocol.enabled)
+      this._debugProtocol('◀ RECV ' + message);
+    if (message.id === kBrowserCloseMessageId)
       return;
-    if (object.sessionId) {
-      const session = this._sessions.get(object.sessionId);
+    if (message.sessionId) {
+      const session = this._sessions.get(message.sessionId);
       if (session)
-        session.dispatchMessage(object);
-    } else if (object.id) {
-      const callback = this._callbacks.get(object.id);
+        session.dispatchMessage(message);
+    } else if (message.id) {
+      const callback = this._callbacks.get(message.id);
       // Callbacks could be all rejected if someone has called `.dispose()`.
       if (callback) {
-        this._callbacks.delete(object.id);
-        if (object.error)
-          callback.reject(createProtocolError(callback.error, callback.method, object));
+        this._callbacks.delete(message.id);
+        if (message.error)
+          callback.reject(createProtocolError(callback.error, callback.method, message.error));
         else
-          callback.resolve(object.result);
+          callback.resolve(message.result);
       }
     } else {
-      Promise.resolve().then(() => this.emit(object.method, object.params));
+      Promise.resolve().then(() => this.emit(message.method, message.params));
     }
   }
 
@@ -176,12 +176,12 @@ export class FFSession extends platform.EventEmitter {
     });
   }
 
-  dispatchMessage(object: { id?: number; method: string; params: object; error: { message: string; data: any; }; result?: any; }) {
+  dispatchMessage(object: ProtocolMessage) {
     if (object.id && this._callbacks.has(object.id)) {
       const callback = this._callbacks.get(object.id)!;
       this._callbacks.delete(object.id);
       if (object.error)
-        callback.reject(createProtocolError(callback.error, callback.method, object));
+        callback.reject(createProtocolError(callback.error, callback.method, object.error));
       else
         callback.resolve(object.result);
     } else {
@@ -200,10 +200,10 @@ export class FFSession extends platform.EventEmitter {
   }
 }
 
-function createProtocolError(error: Error, method: string, object: { error: { message: string; data: any; }; }): Error {
-  let message = `Protocol error (${method}): ${object.error.message}`;
-  if ('data' in object.error)
-    message += ` ${object.error.data}`;
+function createProtocolError(error: Error, method: string, protocolError: { message: string; data: any; }): Error {
+  let message = `Protocol error (${method}): ${protocolError.message}`;
+  if ('data' in protocolError)
+    message += ` ${protocolError.data}`;
   return rewriteError(error, message);
 }
 
@@ -212,9 +212,10 @@ function rewriteError(error: Error, message: string): Error {
   return error;
 }
 
-function rewriteInjectedScriptEvaluationLog(message: any): string | undefined {
+function rewriteInjectedScriptEvaluationLog(message: ProtocolMessage): string {
   // Injected script is very long and clutters protocol logs.
   // To increase development velocity, we skip replace it with short description in the log.
   if (message.method === 'Runtime.evaluate' && message.params && message.params.expression && message.params.expression.includes('src/injected/injected.ts'))
     return `{"id":${message.id} [evaluate injected script]}`;
+  return JSON.stringify(message);
 }
diff --git a/src/platform.ts b/src/platform.ts
@@ -29,7 +29,7 @@ import * as NodeWebSocket from 'ws';
 import * as crypto from 'crypto';
 
 import { assert, helper } from './helper';
-import { ConnectionTransport } from './transport';
+import { ConnectionTransport, ProtocolMessage } from './transport';
 
 export const isNode = typeof process === 'object' && !!process && typeof process.versions === 'object' && !!process.versions && !!process.versions.node;
 
@@ -112,7 +112,8 @@ export const EventEmitter: typeof nodeEvents.EventEmitter = isNode ? nodeEvents.
 ) as any as typeof nodeEvents.EventEmitter;
 export type EventEmitterType = nodeEvents.EventEmitter;
 
-type DebugType = typeof nodeDebug;
+export type DebuggerType = nodeDebug.IDebugger;
+export type DebugType = nodeDebug.IDebug;
 export const debug: DebugType = isNode ? nodeDebug : (
   function debug(namespace: string) {
     return () => {};
@@ -322,7 +323,7 @@ export async function connectToWebsocket<T>(url: string, onopen: (transport: Con
 class WebSocketTransport implements ConnectionTransport {
   _ws: WebSocket;
 
-  onmessage?: (message: string) => void;
+  onmessage?: (message: ProtocolMessage) => void;
   onclose?: () => void;
 
   constructor(url: string) {
@@ -339,7 +340,7 @@ class WebSocketTransport implements ConnectionTransport {
     this._ws.addEventListener('message', event => {
       messageWrap(() => {
         if (this.onmessage)
-          this.onmessage.call(null, event.data);
+          this.onmessage.call(null, JSON.parse(event.data));
       });
     });
 
@@ -351,8 +352,8 @@ class WebSocketTransport implements ConnectionTransport {
     this._ws.addEventListener('error', () => {});
   }
 
-  send(message: string) {
-    this._ws.send(message);
+  send(message: ProtocolMessage) {
+    this._ws.send(JSON.stringify(message));
   }
 
   close() {

diff --git a/src/server/chromium.ts b/src/server/chromium.ts
@@ -115,7 +115,7 @@ export class Chromium implements BrowserType<CRBrowser> {
         // our connection ignores kBrowserCloseMessageId.
         const t = transport || await platform.connectToWebsocket(browserWSEndpoint!, async transport => transport);
         const message = { method: 'Browser.close', id: kBrowserCloseMessageId };
-        await t.send(JSON.stringify(message));
+        await t.send(message);
       },
       onkill: (exitCode, signal) => {
         if (browserServer)

diff --git a/src/server/firefox.ts b/src/server/firefox.ts
@@ -130,7 +130,7 @@ export class Firefox implements BrowserType<FFBrowser> {
         // our connection ignores kBrowserCloseMessageId.
         const transport = await platform.connectToWebsocket(browserWSEndpoint, async transport => transport);
         const message = { method: 'Browser.close', params: {}, id: kBrowserCloseMessageId };
-        await transport.send(JSON.stringify(message));
+        await transport.send(message);
       },
       onkill: (exitCode, signal) => {
         if (browserServer)

diff --git a/src/server/pipeTransport.ts b/src/server/pipeTransport.ts
@@ -16,7 +16,7 @@
  */
 
 import { debugError, helper, RegisteredListener } from '../helper';
-import { ConnectionTransport } from '../transport';
+import { ConnectionTransport, ProtocolMessage } from '../transport';
 import { makeWaitForNextTask } from '../platform';
 
 export class PipeTransport implements ConnectionTransport {
@@ -26,7 +26,7 @@ export class PipeTransport implements ConnectionTransport {
   private _waitForNextTask = makeWaitForNextTask();
   private readonly _closeCallback: () => void;
 
-  onmessage?: (message: string) => void;
+  onmessage?: (message: ProtocolMessage) => void;
   onclose?: () => void;
 
   constructor(pipeWrite: NodeJS.WritableStream, pipeRead: NodeJS.ReadableStream, closeCallback: () => void) {
@@ -46,8 +46,8 @@ export class PipeTransport implements ConnectionTransport {
     this.onclose = undefined;
   }
 
-  send(message: string) {
-    this._pipeWrite!.write(message);
+  send(message: ProtocolMessage) {
+    this._pipeWrite!.write(JSON.stringify(message));
     this._pipeWrite!.write('\0');
   }
 
@@ -64,7 +64,7 @@ export class PipeTransport implements ConnectionTransport {
     const message = this._pendingMessage + buffer.toString(undefined, 0, end);
     this._waitForNextTask(() => {
       if (this.onmessage)
-        this.onmessage.call(null, message);
+        this.onmessage.call(null, JSON.parse(message));
     });
 
     let start = end + 1;
@@ -73,7 +73,7 @@ export class PipeTransport implements ConnectionTransport {
       const message = buffer.toString(undefined, start, end);
       this._waitForNextTask(() => {
         if (this.onmessage)
-          this.onmessage.call(null, message);
+          this.onmessage.call(null, JSON.parse(message));
       });
       start = end + 1;
       end = buffer.indexOf('\0', start);