From c81b377723240aeb54854c3e45db70e1fc32ca41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5var=20Aamb=C3=B8=20Fosstveit?= Date: Mon, 28 Oct 2019 11:45:37 +0100 Subject: [PATCH] Fixed race condition on socket acknowledge callback. Fixed naming on socket methods. Added error handling on socket messages. --- app/src/RoomClient.js | 652 ++++++++++++++++---------------- app/src/components/Lobby.js | 8 +- server/config/config.example.js | 1 + server/lib/Lobby.js | 48 ++- server/lib/Peer.js | 57 +-- server/lib/Room.js | 85 +++-- server/server.js | 1 + 7 files changed, 438 insertions(+), 414 deletions(-) diff --git a/app/src/RoomClient.js b/app/src/RoomClient.js index 3662bd4..ff9494f 100644 --- a/app/src/RoomClient.js +++ b/app/src/RoomClient.js @@ -440,17 +440,17 @@ export default class RoomClient } } - async changeProfilePicture(picture) + async changePicture(picture) { - logger.debug('changeProfilePicture() [picture: "%s"]', picture); + logger.debug('changePicture() [picture: "%s"]', picture); try { - await this.sendRequest('changeProfilePicture', { picture }); + await this.sendRequest('changePicture', { picture }); } catch (error) { - logger.error('shareProfilePicure() | failed: %o', error); + logger.error('changePicture() | failed: %o', error); } } @@ -1284,344 +1284,358 @@ export default class RoomClient 'socket "notification" event [method:%s, data:%o]', notification.method, notification.data); - switch (notification.method) + try { - case 'enteredLobby': + switch (notification.method) { - store.dispatch(stateActions.setInLobby(true)); - - const { displayName } = store.getState().settings; - - await this.sendRequest('changeDisplayName', { displayName }); - break; - } - - case 'roomReady': - { - store.dispatch(stateActions.toggleJoined()); - store.dispatch(stateActions.setInLobby(false)); - - await this._joinRoom({ joinVideo }); - - break; - } - - case 'lockRoom': - { - store.dispatch( - stateActions.setRoomLocked()); - - store.dispatch(requestActions.notify( - { - text : 'Room is now locked.' - })); - - break; - } - - case 'unlockRoom': - { - store.dispatch( - stateActions.setRoomUnLocked()); - - store.dispatch(requestActions.notify( - { - text : 'Room is now unlocked.' - })); - - break; - } - - case 'parkedPeer': - { - const { peerId } = notification.data; - - store.dispatch( - stateActions.addLobbyPeer(peerId)); - - store.dispatch(requestActions.notify( - { - text : 'New participant entered the lobby.' - })); - - break; - } - - case 'lobbyPeerClosed': - { - const { peerId } = notification.data; - - store.dispatch( - stateActions.removeLobbyPeer(peerId)); - - store.dispatch(requestActions.notify( - { - text : 'Participant in lobby left.' - })); - - break; - } - - case 'promotedPeer': - { - const { peerId } = notification.data; - - store.dispatch( - stateActions.removeLobbyPeer(peerId)); - - break; - } - - case 'lobbyPeerDisplayNameChanged': - { - const { peerId, displayName } = notification.data; - - store.dispatch( - stateActions.setLobbyPeerDisplayName(displayName, peerId)); - - store.dispatch(requestActions.notify( - { - text : `Participant in lobby changed name to ${displayName}.` - })); - - break; - } - - case 'setAccessCode': - { - const { accessCode } = notification.data; - - store.dispatch( - stateActions.setAccessCode(accessCode)); - - store.dispatch(requestActions.notify( - { - text : 'Access code for room updated' - })); - - break; - } - - case 'setJoinByAccessCode': - { - const { joinByAccessCode } = notification.data; - - store.dispatch( - stateActions.setJoinByAccessCode(joinByAccessCode)); - - if (joinByAccessCode) + case 'enteredLobby': { + store.dispatch(stateActions.setInLobby(true)); + + const { displayName } = store.getState().settings; + + await this.sendRequest('changeDisplayName', { displayName }); + break; + } + + case 'roomReady': + { + store.dispatch(stateActions.toggleJoined()); + store.dispatch(stateActions.setInLobby(false)); + + await this._joinRoom({ joinVideo }); + + break; + } + + case 'lockRoom': + { + store.dispatch( + stateActions.setRoomLocked()); + store.dispatch(requestActions.notify( { - text : 'Access code for room is now activated' + text : 'Room is now locked.' })); + + break; } - else + + case 'unlockRoom': { + store.dispatch( + stateActions.setRoomUnLocked()); + store.dispatch(requestActions.notify( { - text : 'Access code for room is now deactivated' + text : 'Room is now unlocked.' })); + + break; } - - break; - } - - case 'activeSpeaker': - { - const { peerId } = notification.data; - - store.dispatch( - stateActions.setRoomActiveSpeaker(peerId)); - - if (peerId && peerId !== this._peerId) - this._spotlights.handleActiveSpeaker(peerId); - - break; - } - - case 'changeDisplayName': - { - const { peerId, displayName, oldDisplayName } = notification.data; - - store.dispatch( - stateActions.setPeerDisplayName(displayName, peerId)); - - store.dispatch(requestActions.notify( - { - text : `${oldDisplayName} is now ${displayName}` - })); - - break; - } - - case 'changeProfilePicture': - { - const { peerId, picture } = notification.data; - - store.dispatch(stateActions.setPeerPicture(peerId, picture)); - - break; - } - - case 'chatMessage': - { - const { peerId, chatMessage } = notification.data; - - store.dispatch( - stateActions.addResponseMessage({ ...chatMessage, peerId })); - - if ( - !store.getState().toolarea.toolAreaOpen || - (store.getState().toolarea.toolAreaOpen && - store.getState().toolarea.currentToolTab !== 'chat') - ) // Make sound + + case 'parkedPeer': { - this._soundNotification(); + const { peerId } = notification.data; + + store.dispatch( + stateActions.addLobbyPeer(peerId)); + + store.dispatch(requestActions.notify( + { + text : 'New participant entered the lobby.' + })); + + break; } - - break; - } - - case 'sendFile': - { - const { peerId, magnetUri } = notification.data; - - store.dispatch(stateActions.addFile(peerId, magnetUri)); - - store.dispatch(requestActions.notify( - { - text : 'New file available.' - })); - - if ( - !store.getState().toolarea.toolAreaOpen || - (store.getState().toolarea.toolAreaOpen && - store.getState().toolarea.currentToolTab !== 'files') - ) // Make sound + + case 'lobby:peerClosed': { - this._soundNotification(); + const { peerId } = notification.data; + + store.dispatch( + stateActions.removeLobbyPeer(peerId)); + + store.dispatch(requestActions.notify( + { + text : 'Participant in lobby left.' + })); + + break; } - - break; - } - - case 'producerScore': - { - const { producerId, score } = notification.data; - - store.dispatch( - stateActions.setProducerScore(producerId, score)); - - break; - } - - case 'newPeer': - { - const { id, displayName, picture, device } = notification.data; - - store.dispatch( - stateActions.addPeer({ id, displayName, picture, device, consumers: [] })); - - store.dispatch(requestActions.notify( + + case 'lobby:promotedPeer': + { + const { peerId } = notification.data; + + store.dispatch( + stateActions.removeLobbyPeer(peerId)); + + break; + } + + case 'lobby:changeDisplayName': + { + const { peerId, displayName } = notification.data; + + store.dispatch( + stateActions.setLobbyPeerDisplayName(displayName, peerId)); + + store.dispatch(requestActions.notify( + { + text : `Participant in lobby changed name to ${displayName}.` + })); + + break; + } + + case 'setAccessCode': + { + const { accessCode } = notification.data; + + store.dispatch( + stateActions.setAccessCode(accessCode)); + + store.dispatch(requestActions.notify( + { + text : 'Access code for room updated' + })); + + break; + } + + case 'setJoinByAccessCode': + { + const { joinByAccessCode } = notification.data; + + store.dispatch( + stateActions.setJoinByAccessCode(joinByAccessCode)); + + if (joinByAccessCode) { - text : `${displayName} joined the room.` - })); - - break; - } - - case 'peerClosed': - { - const { peerId } = notification.data; - - store.dispatch( - stateActions.removePeer(peerId)); - - break; - } - - case 'consumerClosed': - { - const { consumerId } = notification.data; - const consumer = this._consumers.get(consumerId); - - if (!consumer) + store.dispatch(requestActions.notify( + { + text : 'Access code for room is now activated' + })); + } + else + { + store.dispatch(requestActions.notify( + { + text : 'Access code for room is now deactivated' + })); + } + break; - - consumer.close(); - - if (consumer.hark != null) - consumer.hark.stop(); - - this._consumers.delete(consumerId); - - const { peerId } = consumer.appData; - - store.dispatch( - stateActions.removeConsumer(consumerId, peerId)); - - break; - } - - case 'consumerPaused': - { - const { consumerId } = notification.data; - const consumer = this._consumers.get(consumerId); - - if (!consumer) + } + + case 'activeSpeaker': + { + const { peerId } = notification.data; + + store.dispatch( + stateActions.setRoomActiveSpeaker(peerId)); + + if (peerId && peerId !== this._peerId) + this._spotlights.handleActiveSpeaker(peerId); + break; - - store.dispatch( - stateActions.setConsumerPaused(consumerId, 'remote')); - - break; - } - - case 'consumerResumed': - { - const { consumerId } = notification.data; - const consumer = this._consumers.get(consumerId); - - if (!consumer) + } + + case 'changeDisplayName': + { + const { peerId, displayName, oldDisplayName } = notification.data; + + store.dispatch( + stateActions.setPeerDisplayName(displayName, peerId)); + + store.dispatch(requestActions.notify( + { + text : `${oldDisplayName} is now ${displayName}` + })); + break; - - store.dispatch( - stateActions.setConsumerResumed(consumerId, 'remote')); - - break; - } - - case 'consumerLayersChanged': - { - const { consumerId, spatialLayer, temporalLayer } = notification.data; - const consumer = this._consumers.get(consumerId); - - if (!consumer) + } + + case 'changePicture': + { + const { peerId, picture } = notification.data; + + store.dispatch(stateActions.setPeerPicture(peerId, picture)); + break; - - store.dispatch(stateActions.setConsumerCurrentLayers( - consumerId, spatialLayer, temporalLayer)); - - break; - } - - case 'consumerScore': - { - const { consumerId, score } = notification.data; - - store.dispatch( - stateActions.setConsumerScore(consumerId, score)); - - break; - } - - default: - { - logger.error( - 'unknown notification.method "%s"', notification.method); + } + + case 'chatMessage': + { + const { peerId, chatMessage } = notification.data; + + store.dispatch( + stateActions.addResponseMessage({ ...chatMessage, peerId })); + + if ( + !store.getState().toolarea.toolAreaOpen || + (store.getState().toolarea.toolAreaOpen && + store.getState().toolarea.currentToolTab !== 'chat') + ) // Make sound + { + this._soundNotification(); + } + + break; + } + + case 'sendFile': + { + const { peerId, magnetUri } = notification.data; + + store.dispatch(stateActions.addFile(peerId, magnetUri)); + + store.dispatch(requestActions.notify( + { + text : 'New file available.' + })); + + if ( + !store.getState().toolarea.toolAreaOpen || + (store.getState().toolarea.toolAreaOpen && + store.getState().toolarea.currentToolTab !== 'files') + ) // Make sound + { + this._soundNotification(); + } + + break; + } + + case 'producerScore': + { + const { producerId, score } = notification.data; + + store.dispatch( + stateActions.setProducerScore(producerId, score)); + + break; + } + + case 'newPeer': + { + const { id, displayName, picture, device } = notification.data; + + store.dispatch( + stateActions.addPeer({ id, displayName, picture, device, consumers: [] })); + + store.dispatch(requestActions.notify( + { + text : `${displayName} joined the room.` + })); + + break; + } + + case 'peerClosed': + { + const { peerId } = notification.data; + + store.dispatch( + stateActions.removePeer(peerId)); + + break; + } + + case 'consumerClosed': + { + const { consumerId } = notification.data; + const consumer = this._consumers.get(consumerId); + + if (!consumer) + break; + + consumer.close(); + + if (consumer.hark != null) + consumer.hark.stop(); + + this._consumers.delete(consumerId); + + const { peerId } = consumer.appData; + + store.dispatch( + stateActions.removeConsumer(consumerId, peerId)); + + break; + } + + case 'consumerPaused': + { + const { consumerId } = notification.data; + const consumer = this._consumers.get(consumerId); + + if (!consumer) + break; + + store.dispatch( + stateActions.setConsumerPaused(consumerId, 'remote')); + + break; + } + + case 'consumerResumed': + { + const { consumerId } = notification.data; + const consumer = this._consumers.get(consumerId); + + if (!consumer) + break; + + store.dispatch( + stateActions.setConsumerResumed(consumerId, 'remote')); + + break; + } + + case 'consumerLayersChanged': + { + const { consumerId, spatialLayer, temporalLayer } = notification.data; + const consumer = this._consumers.get(consumerId); + + if (!consumer) + break; + + store.dispatch(stateActions.setConsumerCurrentLayers( + consumerId, spatialLayer, temporalLayer)); + + break; + } + + case 'consumerScore': + { + const { consumerId, score } = notification.data; + + store.dispatch( + stateActions.setConsumerScore(consumerId, score)); + + break; + } + + default: + { + logger.error( + 'unknown notification.method "%s"', notification.method); + } } } + catch (error) + { + logger.error('error on socket "notification" event failed:"%o"', error); + + store.dispatch(requestActions.notify( + { + type : 'error', + text : 'Error on server request.' + })); + } + }); } diff --git a/app/src/components/Lobby.js b/app/src/components/Lobby.js index e0fe8e9..7e8bcf0 100644 --- a/app/src/components/Lobby.js +++ b/app/src/components/Lobby.js @@ -70,7 +70,9 @@ const Lobby = ({ case 'Escape': { if (displayName === '') - changeDisplayName('Guest'); + roomClient.changeDisplayName('Guest'); + else + roomClient.changeDisplayName(displayName); break; } default: @@ -111,7 +113,9 @@ const Lobby = ({ onBlur={() => { if (displayName === '') - changeDisplayName('Guest'); + roomClient.changeDisplayName('Guest'); + else + roomClient.changeDisplayName(displayName); }} margin='normal' /> diff --git a/server/config/config.example.js b/server/config/config.example.js index a358a14..71b2bdb 100644 --- a/server/config/config.example.js +++ b/server/config/config.example.js @@ -23,6 +23,7 @@ module.exports = },*/ // session cookie secret cookieSecret : 'T0P-S3cR3t_cook!e', + cookieName : 'multiparty-meeting.sid', tls : { cert : `${__dirname}/../certs/mediasoup-demo.localhost.cert.pem`, diff --git a/server/lib/Lobby.js b/server/lib/Lobby.js index 36644ab..d961ccb 100644 --- a/server/lib/Lobby.js +++ b/server/lib/Lobby.js @@ -98,14 +98,22 @@ class Lobby extends EventEmitter peer.authenticated && this.emit('peerAuthenticated', peer); }); - peer.on('displayNameChanged', () => + peer.socket.on('request', (request, cb) => { - this.emit('displayNameChanged', peer); - }); + logger.debug( + 'Peer "request" event [method:"%s", peer:"%s"]', + request.method, peer.id); + + if (this._closed) + return; - peer.on('pictureChanged', () => - { - this.emit('pictureChanged', peer); + this._handleSocketRequest(peer, request, cb) + .catch((error) => + { + logger.error('request failed [error:"%o"]', error); + + cb(error); + }); }); peer.on('close', () => @@ -124,6 +132,34 @@ class Lobby extends EventEmitter }); } + async _handleSocketRequest(peer, request, cb) + { + logger.debug( + '_handleSocketRequest [peer:"%s"], [request:"%s"]', + peer.id, + request.method + ); + + if (this._closed) + return; + + switch (request.method) + { + case 'changeDisplayName': + { + const { displayName } = request.data; + + peer.displayName = displayName; + + this.emit('changeDisplayName', peer); + + cb(); + + break; + } + } + } + _notification(socket, method, data = {}, broadcast = false) { if (broadcast) diff --git a/server/lib/Peer.js b/server/lib/Peer.js index a527a6a..32b6a81 100644 --- a/server/lib/Peer.js +++ b/server/lib/Peer.js @@ -75,14 +75,6 @@ class Peer extends EventEmitter return next(); }); - this.socket.on('request', (request, cb) => - { - if (this._closed) - return; - - this._handleSocketRequest(request, cb).catch(cb); - }); - this.socket.on('disconnect', () => { if (this.closed) @@ -94,37 +86,6 @@ class Peer extends EventEmitter }); } - async _handleSocketRequest(request, cb) - { - if (this._closed) - return; - - switch (request.method) - { - case 'changeDisplayName': - { - const { displayName } = request.data; - - this.displayName = displayName; - - cb(); - - break; - } - - case 'changeProfilePicture': - { - const { picture } = request.data; - - this.picture = picture; - - cb(); - - break; - } - } - } - _checkAuthentication() { if ( @@ -231,14 +192,7 @@ class Peer extends EventEmitter set displayName(displayName) { - if (displayName !== this._displayName) - { - const oldDisplayName = this._displayName; - - this._displayName = displayName; - - this.emit('displayNameChanged', { oldDisplayName }); - } + this._displayName = displayName; } get picture() @@ -248,14 +202,7 @@ class Peer extends EventEmitter set picture(picture) { - if (picture !== this._picture) - { - const oldPicture = this._picture; - - this._picture = picture; - - this.emit('pictureChanged', { oldPicture }); - } + this._picture = picture; } get email() diff --git a/server/lib/Room.js b/server/lib/Room.js index 564a573..c1080c9 100644 --- a/server/lib/Room.js +++ b/server/lib/Room.js @@ -123,7 +123,7 @@ class Room extends EventEmitter } else if ( this._locked || - (config.requireSignInToAccess && !peer.authenticated) + (Boolean(config.requireSignInToAccess) && !peer.authenticated) ) { this._parkPeer(peer); @@ -146,7 +146,7 @@ class Room extends EventEmitter this._peers.forEach((peer) => { - this._notification(peer.socket, 'promotedPeer', { peerId: id }); + this._notification(peer.socket, 'lobby:promotedPeer', { peerId: id }); }); }); @@ -155,23 +155,23 @@ class Room extends EventEmitter !this._locked && this._lobby.promotePeer(peer.id); }); - this._lobby.on('displayNameChanged', (changedPeer) => + this._lobby.on('changeDisplayName', (changedPeer) => { const { id, displayName } = changedPeer; this._peers.forEach((peer) => { - this._notification(peer.socket, 'lobbyPeerDisplayNameChanged', { peerId: id, displayName }); + this._notification(peer.socket, 'lobby:changeDisplayName', { peerId: id, displayName }); }); }); - this._lobby.on('pictureChanged', (changedPeer) => + this._lobby.on('changePicture', (changedPeer) => { const { id, picture } = changedPeer; this._peers.forEach((peer) => { - this._notification(peer.socket, 'lobbyPeerPictureChanged', { peerId: id, picture }); + this._notification(peer.socket, 'lobby:changePicture', { peerId: id, picture }); }); }); @@ -183,7 +183,7 @@ class Room extends EventEmitter this._peers.forEach((peer) => { - this._notification(peer.socket, 'lobbyPeerClosed', { peerId: id }); + this._notification(peer.socket, 'lobby:peerClosed', { peerId: id }); }); }); @@ -312,31 +312,6 @@ class Room extends EventEmitter }); }); - peer.on('displayNameChanged', ({ oldDisplayName }) => - { - if (!peer.joined) - return; - - // Spread to others - this._notification(peer.socket, 'changeDisplayName', { - peerId : peer.id, - displayName : peer.displayName, - oldDisplayName : oldDisplayName - }, true); - }); - - peer.on('pictureChanged', () => - { - if (!peer.joined) - return; - - // Spread to others - this._notification(peer.socket, 'changeProfilePicture', { - peerId : peer.id, - picture : peer.picture - }, true); - }); - peer.on('close', () => { if (this._closed) @@ -758,6 +733,52 @@ class Room extends EventEmitter break; } + case 'changeDisplayName': + { + // Ensure the Peer is joined. + if (!peer.data.joined) + throw new Error('Peer not yet joined'); + + const { displayName } = request.data; + const oldDisplayName = peer.data.displayName; + + peer.displayName = displayName; + + // Spread to others + this._notification(peer.socket, 'changeDisplayName', { + peerId : peer.id, + displayName : displayName, + oldDisplayName : oldDisplayName + }, true); + + // Return no error + cb(); + + break; + } + + case 'changePicture': + { + // Ensure the Peer is joined. + if (!peer.data.joined) + throw new Error('Peer not yet joined'); + + const { picture } = request.data; + + peer.picture = picture; + + // Spread to others + this._notification(peer.socket, 'changePicture', { + peerId : peer.id, + picture : picture + }, true); + + // Return no error + cb(); + + break; + } + case 'chatMessage': { const { chatMessage } = request.data; diff --git a/server/server.js b/server/server.js index 2511333..a619291 100755 --- a/server/server.js +++ b/server/server.js @@ -68,6 +68,7 @@ app.use(bodyParser.urlencoded({ extended: true })); const session = expressSession({ secret : config.cookieSecret, + name : config.cookieName, resave : true, saveUninitialized : true, store : new RedisStore({ client }),