Fixed issue with websocket connections being closed when deep linking…

… into a conversation. (#2146)
microsoft · May 14, 2020 · 4681f24 · 4681f24
1 parent 0f53dfa
commit 4681f24
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 42 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Fixed
 - [build] Fixed system dialog on Mac OS warning about being unable to check for malicious code in PR [2135](https://github.com/microsoft/BotFramework-Emulator/pull/2135)
+- [client / main] Fixed an issue where starting a conversation via deeplink was initializing the WebSocket server twice and closing previously established connections in PR [2146](https://github.com/microsoft/BotFramework-Emulator/pull/2146)
 
 ## v4.8.1 - 2019 - 03 - 18
 ## Fixed

diff --git a/packages/app/main/src/server/webSocketServer.spec.ts b/packages/app/main/src/server/webSocketServer.spec.ts
@@ -51,6 +51,7 @@ describe('WebSocketServer', () => {
     mockCreateServer.mockClear();
     mockWSServer.handleUpgrade.mockClear();
     mockWSServer.on.mockClear();
+    (WebSocketServer as any)._restServer = undefined;
   });
 
   it('should return the corresponding socket for a conversation id', () => {
@@ -65,8 +66,6 @@ describe('WebSocketServer', () => {
   });
 
   it('should initialize the server', async () => {
-    const cleanupSpy = jest.spyOn(WebSocketServer, 'cleanup').mockImplementationOnce(() => null);
-    (WebSocketServer as any)._restServer = {};
     (WebSocketServer as any)._servers = {};
     (WebSocketServer as any)._sockets = {};
     const mockRestServer = {
@@ -83,8 +82,13 @@ describe('WebSocketServer', () => {
     expect(port).toBe(56273);
     expect(mockRestServer.get).toHaveBeenCalledWith('/ws/:conversationId', jasmine.any(Function));
     expect((WebSocketServer as any)._restServer).toEqual(mockRestServer);
-    expect(cleanupSpy).toHaveBeenCalled();
-    cleanupSpy.mockRestore();
+  });
+
+  it('should not re-initialize if already initialized', async () => {
+    (WebSocketServer as any)._restServer = {}; // server initialized
+    await WebSocketServer.init();
+
+    expect(mockCreateServer).not.toHaveBeenCalled();
   });
 
   it('should clean up the rest server, web sockets, and web socket servers', () => {

diff --git a/packages/app/main/src/server/webSocketServer.ts b/packages/app/main/src/server/webSocketServer.ts
@@ -50,48 +50,50 @@ export class WebSocketServer {
     return this._sockets[conversationId];
   }
 
-  public static async init(): Promise<number> {
-    if (this._restServer) {
-      this.cleanup();
-    }
-    this._restServer = createServer({ handleUpgrades: true, name: 'Emulator-WebSocket-Host' });
-    this._restServer.get('/ws/:conversationId', (req: Request, res: Response, next: Next) => {
-      const conversationId = req.params.conversationId;
+  /** Initializes the server and returns the port it is listening on, or if already initialized,
+   *  is a no-op.
+   */
+  public static async init(): Promise<number | void> {
+    if (!this._restServer) {
+      this._restServer = createServer({ handleUpgrades: true, name: 'Emulator-WebSocket-Host' });
+      this._restServer.get('/ws/:conversationId', (req: Request, res: Response, next: Next) => {
+        const conversationId = req.params.conversationId;
 
-      // initialize a new web socket server for each new conversation
-      if (conversationId && !this._servers[conversationId]) {
-        if (!(res as any).claimUpgrade) {
-          return next(new Error('Connection must upgrade for web sockets.'));
-        }
-        const { head, socket } = (res as any).claimUpgrade();
-        const wsServer = new WSServer({
-          noServer: true,
-        });
-        wsServer.on('connection', (socket, req) => {
-          this._sockets[conversationId] = socket;
-          socket.on('close', (code, reason) => {
-            delete this._servers[conversationId];
-            delete this._sockets[conversationId];
+        // initialize a new web socket server for each new conversation
+        if (conversationId && !this._servers[conversationId]) {
+          if (!(res as any).claimUpgrade) {
+            return next(new Error('Connection must upgrade for web sockets.'));
+          }
+          const { head, socket } = (res as any).claimUpgrade();
+          const wsServer = new WSServer({
+            noServer: true,
           });
+          wsServer.on('connection', (socket, req) => {
+            this._sockets[conversationId] = socket;
+            socket.on('close', (code, reason) => {
+              delete this._servers[conversationId];
+              delete this._sockets[conversationId];
+            });
+          });
+          // upgrade the connection to a ws connection
+          wsServer.handleUpgrade(req, socket, head, socket => {
+            wsServer.emit('connection', socket, req);
+          });
+          this._servers[conversationId] = wsServer;
+        }
+      });
+      // dynamically generate the web socket server port
+      const port = await new Promise<number>((resolve, reject) => {
+        this._restServer.once('error', err => reject(err));
+        this._restServer.listen(null, () => {
+          resolve(this._restServer.address().port);
         });
-        // upgrade the connection to a ws connection
-        wsServer.handleUpgrade(req, socket, head, socket => {
-          wsServer.emit('connection', socket, req);
-        });
-        this._servers[conversationId] = wsServer;
-      }
-    });
-    // dynamically generate the web socket server port
-    const port = await new Promise<number>((resolve, reject) => {
-      this._restServer.once('error', err => reject(err));
-      this._restServer.listen(null, () => {
-        resolve(this._restServer.address().port);
       });
-    });
-    this.port = port;
-    // eslint-disable-next-line no-console
-    console.log(`Web Socket host server listening on ${port}...`);
-    return port;
+      this.port = port;
+      // eslint-disable-next-line no-console
+      console.log(`Web Socket host server listening on ${port}...`);
+      return port;
+    }
   }
 
   public static cleanup(): void {