Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed issue with websocket connections being closed when deep linking … #2146

Merged
merged 1 commit into from
May 14, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Fixed issue with websocket connections being closed when deep linking…
… into a conversation.
tonyanziano committed May 14, 2020
commit ebb27aa53971d86244d626fdee45af455a1757ee
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
12 changes: 8 additions & 4 deletions packages/app/main/src/server/webSocketServer.spec.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
78 changes: 40 additions & 38 deletions packages/app/main/src/server/webSocketServer.ts
Original file line number Diff line number Diff line change
@@ -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 {