From 96c5d92c467214dd11ceaee22843a43a0e7daa7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Fri, 5 Jun 2020 13:07:18 +0200 Subject: [PATCH] Fixing disconnection taking ~15 seconds Most of the time, sending a disconnect event to one of the players is enough (the player will close the connection which will be shut for the other player). However! In the rare case where the WebRTC connection is not yet established, if we close the connection on one of the player, the other player will try connecting until a timeout happens (during this time, the circle with the name is displayed for nothing). So now, we send disconnection event to every body (not only the people in the group, but also to the person leaving the group) --- back/src/Controller/IoSocketController.ts | 11 ++++++ front/src/Connection.ts | 18 +++++++-- front/src/WebRtc/SimplePeer.ts | 45 ++++++++++++++++------- 3 files changed, 57 insertions(+), 17 deletions(-) diff --git a/back/src/Controller/IoSocketController.ts b/back/src/Controller/IoSocketController.ts index 1baf49ca..f30663f8 100644 --- a/back/src/Controller/IoSocketController.ts +++ b/back/src/Controller/IoSocketController.ts @@ -388,6 +388,17 @@ export class IoSocketController { userId: userId }); + // Most of the time, sending a disconnect event to one of the players is enough (the player will close the connection + // which will be shut for the other player). + // However! In the rare case where the WebRTC connection is not yet established, if we close the connection on one of the player, + // the other player will try connecting until a timeout happens (during this time, the connection icon will be displayed for nothing). + // So we also send the disconnect event to the other player. + for (let user of group.getUsers()) { + Client.emit(SockerIoEvent.WEBRTC_DISCONNECT, { + userId: user.id + }); + } + //disconnect webrtc room if(!Client.webRtcRoomId){ return; diff --git a/front/src/Connection.ts b/front/src/Connection.ts index cf7a3856..3c1bc2d4 100644 --- a/front/src/Connection.ts +++ b/front/src/Connection.ts @@ -7,6 +7,7 @@ import {SetPlayerDetailsMessage} from "./Messages/SetPlayerDetailsMessage"; const SocketIo = require('socket.io-client'); import Socket = SocketIOClient.Socket; import {PlayerAnimationNames} from "./Phaser/Player/Animation"; +import {UserSimplePeer} from "./WebRtc/SimplePeer"; enum EventMessage{ @@ -97,6 +98,15 @@ export interface GroupCreatedUpdatedMessageInterface { groupId: string } +export interface WebRtcStartMessageInterface { + roomId: string, + clients: UserSimplePeer[] +} + +export interface WebRtcDisconnectMessageInterface { + userId: string +} + export interface ConnectionInterface { socket: Socket|null; token: string|null; @@ -116,9 +126,9 @@ export interface ConnectionInterface { receiveWebrtcSignal(callBack: Function): void; - receiveWebrtcStart(callBack: Function): void; + receiveWebrtcStart(callBack: (message: WebRtcStartMessageInterface) => void): void; - disconnectMessage(callBack: Function): void; + disconnectMessage(callBack: (message: WebRtcDisconnectMessageInterface) => void): void; } export class Connection implements ConnectionInterface { @@ -277,7 +287,7 @@ export class Connection implements ConnectionInterface { }); } - receiveWebrtcStart(callback: Function) { + receiveWebrtcStart(callback: (message: WebRtcStartMessageInterface) => void) { this.getSocket().on(EventMessage.WEBRTC_START, callback); } @@ -305,7 +315,7 @@ export class Connection implements ConnectionInterface { }); } - disconnectMessage(callback: Function): void { + disconnectMessage(callback: (message: WebRtcDisconnectMessageInterface) => void): void { this.getSocket().on(EventMessage.WEBRTC_DISCONNECT, callback); } } diff --git a/front/src/WebRtc/SimplePeer.ts b/front/src/WebRtc/SimplePeer.ts index 2af0be27..902d316c 100644 --- a/front/src/WebRtc/SimplePeer.ts +++ b/front/src/WebRtc/SimplePeer.ts @@ -1,13 +1,17 @@ -import {ConnectionInterface} from "../Connection"; +import {ConnectionInterface, WebRtcDisconnectMessageInterface, WebRtcStartMessageInterface} from "../Connection"; import {MediaManager} from "./MediaManager"; import * as SimplePeerNamespace from "simple-peer"; let Peer: SimplePeerNamespace.SimplePeer = require('simple-peer'); -class UserSimplePeer{ +export interface UserSimplePeer{ userId: string; name?: string; initiator?: boolean; } + +/** + * This class manages connections to all the peers in the same group as me. + */ export class SimplePeer { private Connection: ConnectionInterface; private WebRtcRoomId: string; @@ -40,7 +44,7 @@ export class SimplePeer { this.MediaManager.getCamera().then(() => { //receive message start - this.Connection.receiveWebrtcStart((message: any) => { + this.Connection.receiveWebrtcStart((message: WebRtcStartMessageInterface) => { this.receiveWebrtcStart(message); }); @@ -48,22 +52,26 @@ export class SimplePeer { console.error("err", err); }); - //receive signal by gemer - this.Connection.disconnectMessage((data: any) => { + this.Connection.disconnectMessage((data: WebRtcDisconnectMessageInterface): void => { this.closeConnection(data.userId); }); } - private receiveWebrtcStart(data: any) { + private receiveWebrtcStart(data: WebRtcStartMessageInterface) { this.WebRtcRoomId = data.roomId; this.Users = data.clients; + // Note: the clients array contain the list of all clients (event the ones we are already connected to in case a user joints a group) + // So we can receive a request we already had before. (which will abort at the first line of createPeerConnection) + // TODO: refactor this to only send a message to connect to one user (rather than several users). + // This would be symmetrical to the way we handle disconnection. + //console.log('Start message', data); //start connection this.startWebRtc(); } /** - * server has two person connected, start the meet + * server has two people connected, start the meet */ private startWebRtc() { this.Users.forEach((user: UserSimplePeer) => { @@ -83,6 +91,8 @@ export class SimplePeer { return; } + //console.log("Creating connection with peer "+user.userId); + let name = user.name; if(!name){ let userSearch = this.Users.find((userSearch: UserSimplePeer) => userSearch.userId === user.userId); @@ -93,7 +103,7 @@ export class SimplePeer { this.MediaManager.removeActiveVideo(user.userId); this.MediaManager.addActiveVideo(user.userId, name); - let peer : any = new Peer({ + let peer : SimplePeerNamespace.Instance = new Peer({ initiator: user.initiator ? user.initiator : false, reconnectTimer: 10000, config: { @@ -167,15 +177,25 @@ export class SimplePeer { this.addMedia(user.userId); } + /** + * This is triggered twice. Once by the server, and once by a remote client disconnecting + * + * @param userId + */ private closeConnection(userId : string) { try { this.MediaManager.removeActiveVideo(userId); - if (!this.PeerConnectionArray.get(userId)) { + let peer = this.PeerConnectionArray.get(userId); + if (peer === undefined) { + console.warn("Tried to close connection for user "+userId+" but could not find user") return; } - // @ts-ignore - this.PeerConnectionArray.get(userId)?.destroy(); + // FIXME: I don't understand why "Closing connection with" message is displayed TWICE before "Nb users in peerConnectionArray" + // I do understand the method closeConnection is called twice, but I don't understand how they manage to run in parallel. + //console.log('Closing connection with '+userId); + peer.destroy(); this.PeerConnectionArray.delete(userId) + //console.log('Nb users in peerConnectionArray '+this.PeerConnectionArray.size); } catch (err) { console.error("closeConnection", err) } @@ -216,7 +236,7 @@ export class SimplePeer { * @param userId * @param stream */ - private stream(userId : any, stream: MediaStream) { + private stream(userId : string, stream: MediaStream) { if(!stream){ this.MediaManager.disabledVideoByUserId(userId); this.MediaManager.disabledMicrophoneByUserId(userId); @@ -231,7 +251,6 @@ export class SimplePeer { */ private addMedia (userId : any = null) { try { - let transceiver : any = null; let localStream: MediaStream|null = this.MediaManager.localStream; let peer = this.PeerConnectionArray.get(userId); if(localStream === null) {