ServerAdapter: ServerWebSocketBase

Summary: Make it more clear about that this is. `ServerAdapter` was VERY generic.

Reviewed By: passy

Differential Revision: D47185076

fbshipit-source-id: 7d9b30f423398004bedc92ad22bc0217d1c8453f

desktop/flipper-server-core/src/comms/BrowserServerWebSocket.tsx

@@ -16,7 +16,7 @@ import {assertNotNull, parseClientQuery} from './Utilities';
 import SecureServerWebSocket, {
   SecureConnectionCtx,
 } from './SecureServerWebSocket';
-import {SecureClientQuery} from './ServerAdapter';
+import {SecureClientQuery} from './ServerWebSocketBase';
 import {ClientDescription, DeviceOS} from 'flipper-common';
 import {URL} from 'url';
 import {isFBBuild} from '../fb-stubs/constants';

desktop/flipper-server-core/src/comms/SecureServerWebSocket.tsx

@@ -8,7 +8,7 @@
  */
 
 import ServerWebSocket, {ConnectionCtx} from './ServerWebSocket';
-import {SecureClientQuery} from './ServerAdapter';
+import {SecureClientQuery} from './ServerWebSocketBase';
 import {ParsedUrlQuery} from 'querystring';
 import {ClientDescription} from 'flipper-common';
 import {

desktop/flipper-server-core/src/comms/ServerController.tsx

@@ -27,10 +27,10 @@ import {
   assertNotNull,
   cloneClientQuerySafeForLogging,
 } from './Utilities';
-import ServerAdapter, {
+import ServerWebSocketBase, {
   SecureClientQuery,
   ServerEventsListener,
-} from './ServerAdapter';
+} from './ServerWebSocketBase';
 import {
   createBrowserServer,
   createServer,
@@ -79,11 +79,11 @@ export class ServerController
   connections: Map<string, ClientInfo> = new Map();
   timestamps: Map<string, ClientTimestampTracker> = new Map();
 
-  secureServer: ServerAdapter | null = null;
-  insecureServer: ServerAdapter | null = null;
-  altSecureServer: ServerAdapter | null = null;
-  altInsecureServer: ServerAdapter | null = null;
-  browserServer: ServerAdapter | null = null;
+  secureServer: ServerWebSocketBase | null = null;
+  insecureServer: ServerWebSocketBase | null = null;
+  altSecureServer: ServerWebSocketBase | null = null;
+  altInsecureServer: ServerWebSocketBase | null = null;
+  browserServer: ServerWebSocketBase | null = null;
 
   connectionTracker: ConnectionTracker;
 

desktop/flipper-server-core/src/comms/ServerFactory.tsx

@@ -8,7 +8,7 @@
  */
 
 import {SecureServerConfig} from '../utils/certificateUtils';
-import ServerAdapter, {ServerEventsListener} from './ServerAdapter';
+import ServerWebSocketBase, {ServerEventsListener} from './ServerWebSocketBase';
 import ServerRSocket from './ServerRSocket';
 import SecureServerWebSocket from './SecureServerWebSocket';
 import BrowserServerWebSocket from './BrowserServerWebSocket';
@@ -31,8 +31,8 @@ export async function createServer(
   listener: ServerEventsListener,
   sslConfig?: SecureServerConfig,
   transportType: TransportType = TransportType.RSocket,
-): Promise<ServerAdapter> {
-  let server: ServerAdapter;
+): Promise<ServerWebSocketBase> {
+  let server: ServerWebSocketBase;
   if (transportType === TransportType.RSocket) {
     server = new ServerRSocket(listener);
   } else if (sslConfig) {
@@ -56,7 +56,7 @@ export async function createServer(
 export async function createBrowserServer(
   port: number,
   listener: ServerEventsListener,
-): Promise<ServerAdapter> {
+): Promise<ServerWebSocketBase> {
   const server = new BrowserServerWebSocket(listener);
   await server.start(port);
   return server;

desktop/flipper-server-core/src/comms/ServerRSocket.tsx

@@ -8,10 +8,10 @@
  */
 
 import {SecureServerConfig} from '../utils/certificateUtils';
-import ServerAdapter, {
+import ServerWebSocketBase, {
   SecureClientQuery,
   ServerEventsListener,
-} from './ServerAdapter';
+} from './ServerWebSocketBase';
 import tls from 'tls';
 import net, {AddressInfo, Socket} from 'net';
 import {RSocketServer} from 'rsocket-core';
@@ -34,7 +34,7 @@ import {transformCertificateExchangeMediumToType} from './Utilities';
  * RSocket based server. RSocket uses its own protocol for communication between
  * client and server.
  */
-class ServerRSocket extends ServerAdapter {
+class ServerRSocket extends ServerWebSocketBase {
   rawServer_: RSocketServer<any, any> | null | undefined;
   constructor(listener: ServerEventsListener) {
     super(listener);

desktop/flipper-server-core/src/comms/ServerWebSocket.tsx

@@ -8,7 +8,7 @@
  */
 
 import {IncomingMessage} from 'http';
-import ServerAdapter from './ServerAdapter';
+import ServerWebSocketBase from './ServerWebSocketBase';
 import WebSocket, {
   AddressInfo,
   Server as WSServer,
@@ -35,15 +35,15 @@ export interface ConnectionCtx {
   request: IncomingMessage;
 }
 
-// based on https://github.com/websockets/ws/blob/master/lib/websocket-server.js#L40,
-// exposed to share with socket.io defaults
+// Based on https://github.com/websockets/ws/blob/master/lib/websocket-server.js#L40,
+// exposed to share with socket.io defaults.
 export const WEBSOCKET_MAX_MESSAGE_SIZE = 100 * 1024 * 1024;
 
 /**
  * It serves as a base class for WebSocket based servers. It delegates the 'connection'
  * event to subclasses as a customisation point.
  */
-class ServerWebSocket extends ServerAdapter {
+class ServerWebSocket extends ServerWebSocketBase {
   protected wsServer?: WSServer;
   private httpServer?: Server;
 
@@ -77,12 +77,14 @@ class ServerWebSocket extends ServerAdapter {
           'server',
         );
 
-        // Unsubscribe connection error listener. We'll attach a permanent error listener later
+        // Unsubscribe connection error listener.
+        // We'll attach a permanent error listener later.
         wsServer.off('error', onConnectionError);
 
         this.listener.onListening(port);
         this.wsServer = wsServer;
         this.httpServer = server;
+
         resolve((server.address() as AddressInfo).port);
       });
     });
@@ -99,7 +101,7 @@ class ServerWebSocket extends ServerAdapter {
         });
 
         try {
-          this.onConnection(ws, request); // insecure connection, with medium.
+          this.onConnection(ws, request);
         } catch (error) {
           // TODO: Investigate if we need to close the socket in the `error` listener
           // DRI: @aigoncharov
@@ -166,7 +168,8 @@ class ServerWebSocket extends ServerAdapter {
    */
   onConnection(ws: WebSocket, request: IncomingMessage): void {
     const ctx: ConnectionCtx = {ws, request};
-    this.handleClientQuery(ctx);
+
+    this.extractClientQuery(ctx);
     this.handleConnectionAttempt(ctx);
 
     ws.on('message', async (message: WebSocket.RawData) => {
@@ -180,15 +183,24 @@ class ServerWebSocket extends ServerAdapter {
         // all other plugins might still be working correctly. So let's just report it.
         // This avoids ping-ponging connections if an individual plugin sends garbage (e.g. T129428800)
         // or throws an error when handling messages
-        console.error('Failed to handle message', messageString, error);
+        console.error('[conn] Failed to handle message', messageString, error);
       }
     });
   }
 
-  protected handleClientQuery(ctx: ConnectionCtx): void {
+  /**
+   * Extract and create a ClientQuery from the request URL. This method will throw if:
+   * @param ctx The connection context.
+   * @returns It doesn't return anything, if the client query
+   * is extracted, this one is set into the connection context.
+   */
+  protected extractClientQuery(ctx: ConnectionCtx): void {
     const {request} = ctx;
+    if (!request.url) {
+      return;
+    }
 
-    const query = querystring.decode(request.url!.split('?')[1]);
+    const query = querystring.decode(request.url.split('?')[1]);
     const clientQuery = this.parseClientQuery(query);
 
     if (!clientQuery) {
@@ -218,7 +230,6 @@ class ServerWebSocket extends ServerAdapter {
     const parsedMessage = parseMessageToJson(message);
     if (!parsedMessage) {
       console.error('[conn] Failed to parse message', message);
-      // TODO: Create custom DeserializationError
       throw new Error(`[conn] Failed to parse message`);
     }
     return parsedMessage;
@@ -227,7 +238,7 @@ class ServerWebSocket extends ServerAdapter {
   protected async handleMessage(
     ctx: ConnectionCtx,
     parsedMessage: object,
-    // Not used in this method, but left as a reference for overriding classes
+    // Not used in this method, but left as a reference for overriding classes.
     _rawMessage: string,
   ) {
     const {clientQuery, ws} = ctx;

desktop/flipper-server-core/src/comms/ServerAdapter.tsx → desktop/flipper-server-core/src/comms/ServerWebSocketBase.tsx

@@ -111,7 +111,7 @@ export interface ServerEventsListener {
  * Defines the base class to be used by any server implementation e.g.
  * RSocket, WebSocket, etc.
  */
-abstract class ServerAdapter {
+abstract class ServerWebSocketBase {
   constructor(protected listener: ServerEventsListener) {}
 
   /**
@@ -185,4 +185,4 @@ abstract class ServerAdapter {
   }
 }
 
-export default ServerAdapter;
+export default ServerWebSocketBase;

desktop/flipper-server-core/src/comms/Utilities.tsx

@@ -14,7 +14,7 @@ import {
   ResponseMessage,
 } from 'flipper-common';
 import {ParsedUrlQuery} from 'querystring';
-import {SecureClientQuery} from './ServerAdapter';
+import {SecureClientQuery} from './ServerWebSocketBase';
 
 /**
  * Transforms the certificate exchange medium type as number to the

desktop/flipper-server-core/src/comms/__tests__/BrowserServerWebSocket.node.tsx

@@ -18,7 +18,7 @@ import WebSocket from 'ws';
 import {BrowserClientConnection} from '../BrowserClientConnection';
 import {getFlipperServerConfig} from '../../FlipperServerConfig';
 import BrowserServerWebSocket from '../BrowserServerWebSocket';
-import {SecureClientQuery} from '../ServerAdapter';
+import {SecureClientQuery} from '../ServerWebSocketBase';
 import {createMockSEListener, WSMessageAccumulator} from './utils';
 
 jest.mock('../../FlipperServerConfig');

desktop/flipper-server-core/src/comms/__tests__/SecureServerWebSocket.node.tsx

@@ -17,7 +17,7 @@ import {toBase64} from 'js-base64';
 import WebSocket from 'ws';
 
 import SecureServerWebSocket from '../SecureServerWebSocket';
-import {SecureClientQuery} from '../ServerAdapter';
+import {SecureClientQuery} from '../ServerWebSocketBase';
 import {transformCertificateExchangeMediumToType} from '../Utilities';
 import WebSocketClientConnection from '../WebSocketClientConnection';
 import {createMockSEListener, WSMessageAccumulator} from './utils';

desktop/flipper-server-core/src/comms/__tests__/utils.tsx

@@ -8,7 +8,7 @@
  */
 
 import {ClientQuery, ClientDescription} from 'flipper-common';
-import {ServerEventsListener} from '../ServerAdapter';
+import {ServerEventsListener} from '../ServerWebSocketBase';
 
 export class WSMessageAccumulator {
   private messages: unknown[] = [];