From 20d66e04c27cc550b5c91a460c12106d0e690bae Mon Sep 17 00:00:00 2001
From: prlanzarin <prlanzarin@inf.ufrgs.br>
Date: Thu, 19 Jul 2018 19:21:16 +0000
Subject: [PATCH] Finished error handling on SFU managers, they are now
 propagated to the clients correctly and standardized

---
 labs/bbb-webrtc-sfu/lib/audio/AudioManager.js | 18 +++----
 labs/bbb-webrtc-sfu/lib/audio/audio.js        | 10 ++--
 labs/bbb-webrtc-sfu/lib/base/BaseManager.js   | 47 +++++++++++++++++++
 labs/bbb-webrtc-sfu/lib/base/BaseProvider.js  | 14 +++---
 labs/bbb-webrtc-sfu/lib/base/errors.js        | 14 +++++-
 .../lib/mcs-core/lib/media/MCSApiStub.js      |  2 +-
 .../lib/mcs-core/lib/model/SdpSession.js      | 10 ++--
 .../lib/screenshare/ScreenshareManager.js     | 18 +++----
 .../lib/screenshare/screenshare.js            |  2 +-
 labs/bbb-webrtc-sfu/lib/video/VideoManager.js | 18 +++----
 labs/bbb-webrtc-sfu/lib/video/video.js        |  2 +-
 11 files changed, 103 insertions(+), 52 deletions(-)

diff --git a/labs/bbb-webrtc-sfu/lib/audio/AudioManager.js b/labs/bbb-webrtc-sfu/lib/audio/AudioManager.js
index 06214fee47..c72815e3f3 100755
--- a/labs/bbb-webrtc-sfu/lib/audio/AudioManager.js
+++ b/labs/bbb-webrtc-sfu/lib/audio/AudioManager.js
@@ -12,10 +12,12 @@ const Audio = require('./audio');
 const BaseManager = require('../base/BaseManager');
 const C = require('../bbb/messages/Constants');
 const Logger = require('../utils/Logger');
+const errors = require('../base/errors');
 
 module.exports = class AudioManager extends BaseManager {
   constructor (connectionChannel, additionalChannels, logPrefix) {
     super(connectionChannel, additionalChannels, logPrefix);
+    this.sfuApp = C.AUDIO_APP;
     this._meetings = {};
     this._trackMeetingTermination();
     this.messageFactory(this._onMessage);
@@ -73,14 +75,10 @@ module.exports = class AudioManager extends BaseManager {
           Logger.info(this._logPrefix, "Started presenter ", sessionId, " for connection", connectionId);
           Logger.debug(this._logPrefix, "SDP answer was", sdpAnswer);
           if (error) {
-            this._bbbGW.publish(JSON.stringify({
-              connectionId: connectionId,
-              type: 'audio',
-              id : 'startResponse',
-              response : 'rejected',
-              message : error
+            const errorMessage = this._handleError(this._logPrefix, connectionId, callerName, C.RECV_ROLE, error);
+            return this._bbbGW.publish(JSON.stringify({
+              ...errorMessage
             }), C.FROM_AUDIO);
-            return error;
           }
 
           this._bbbGW.publish(JSON.stringify({
@@ -123,11 +121,9 @@ module.exports = class AudioManager extends BaseManager {
         break;
 
       default:
+        const errorMessage = this._handleError(this._logPrefix, connectionId, null, null, errors.SFU_INVALID_REQUEST);
         this._bbbGW.publish(JSON.stringify({
-          connectionId: connectionId,
-          id : 'error',
-          type: 'audio',
-          message: 'Invalid message ' + message
+          ...errorMessage,
         }), C.FROM_AUDIO);
         break;
     }
diff --git a/labs/bbb-webrtc-sfu/lib/audio/audio.js b/labs/bbb-webrtc-sfu/lib/audio/audio.js
index fa0bf1530b..8e6667e65a 100644
--- a/labs/bbb-webrtc-sfu/lib/audio/audio.js
+++ b/labs/bbb-webrtc-sfu/lib/audio/audio.js
@@ -13,7 +13,7 @@ const LOG_PREFIX = "[audio]";
 module.exports = class Audio extends BaseProvider {
   constructor(_bbbGW, _id, voiceBridge) {
     super();
-    this.sfuApp = "audio";
+    this.sfuApp = C.AUDIO_APP;
     this.mcs = new MCSApi();
     this.bbbGW = _bbbGW;
     this.id = _id;
@@ -51,9 +51,11 @@ module.exports = class Audio extends BaseProvider {
   flushCandidatesQueue (connectionId) {
     if (this.audioEndpoints[connectionId]) {
       try {
-        while(this.candidatesQueue[connectionId].length) {
-          const candidate = this.candidatesQueue[connectionId].shift();
-          this.mcs.addIceCandidate(this.audioEndpoints[connectionId], candidate);
+        if (this.candidatesQueue[connectionId]) {
+          while(this.candidatesQueue[connectionId].length) {
+            const candidate = this.candidatesQueue[connectionId].shift();
+            this.mcs.addIceCandidate(this.audioEndpoints[connectionId], candidate);
+          }
         }
       }
       catch (err) {
diff --git a/labs/bbb-webrtc-sfu/lib/base/BaseManager.js b/labs/bbb-webrtc-sfu/lib/base/BaseManager.js
index 6e3668e1f9..9e7320b2a0 100644
--- a/labs/bbb-webrtc-sfu/lib/base/BaseManager.js
+++ b/labs/bbb-webrtc-sfu/lib/base/BaseManager.js
@@ -10,6 +10,7 @@
 const BigBlueButtonGW = require('../bbb/pubsub/bbb-gw');
 const C = require('../bbb/messages/Constants');
 const Logger = require('../utils/Logger');
+const errors = require('../base/errors');
 
 module.exports = class BaseManager {
   constructor (connectionChannel, additionalChannels = [], logPrefix = C.BASE_MANAGER_PREFIX) {
@@ -134,4 +135,50 @@ module.exports = class BaseManager {
       Logger.debug(logInfo);
     }
   }
+
+  _handleError (logPrefix, connectionId, streamId, role, error) {
+    if (this._validateErrorMessage(error)) {
+      return error;
+    }
+
+    const { code } = error;
+    const reason = errors[code];
+
+    if (reason == null) {
+      return;
+    }
+
+    error.message = reason;
+
+    Logger.debug(logPrefix, "Handling error", error.code, error.message);
+    Logger.trace(logPrefix, error.stack);
+
+    return this._assembleErrorMessage(error, role, streamId, connectionId);
+  }
+
+  _assembleErrorMessage (error, role, streamId, connectionId) {
+    return {
+      connectionId,
+      type: this.sfuApp,
+      id: 'error',
+      role,
+      streamId,
+      code: error.code,
+      reason: error.message,
+    };
+  }
+
+  _validateErrorMessage (error) {
+    const {
+      connectionId = null,
+      type = null,
+      id = null,
+      role = null,
+      streamId = null,
+      code = null,
+      reason = null,
+    } = error;
+    return connectionId && type && id && role && streamId && code && reason;
+  }
+
 };
diff --git a/labs/bbb-webrtc-sfu/lib/base/BaseProvider.js b/labs/bbb-webrtc-sfu/lib/base/BaseProvider.js
index b623e3bf58..45dcfe1c33 100644
--- a/labs/bbb-webrtc-sfu/lib/base/BaseProvider.js
+++ b/labs/bbb-webrtc-sfu/lib/base/BaseProvider.js
@@ -3,7 +3,7 @@
 const C = require('../bbb/messages/Constants');
 const Logger = require('../utils/Logger');
 const EventEmitter = require('events').EventEmitter;
-const { errors } = require('../base/errors');
+const errors = require('../base/errors');
 
 module.exports = class BaseProvider extends EventEmitter {
   constructor () {
@@ -11,7 +11,7 @@ module.exports = class BaseProvider extends EventEmitter {
     this.sfuApp = "base";
   }
 
-  _handleError (logPrefix, error, role, endpointId) {
+  _handleError (logPrefix, error, role, streamId) {
     if (this._validateErrorMessage(error)) {
       return error;
     }
@@ -28,15 +28,15 @@ module.exports = class BaseProvider extends EventEmitter {
     Logger.debug(logPrefix, "Handling error", error.code, error.message);
     Logger.trace(logPrefix, error.stack);
 
-    return this._assembleErrorMessage(error, role, endpointId);
+    return this._assembleErrorMessage(error, role, streamId);
   }
 
-  _assembleErrorMessage (error, role, endpointId) {
+  _assembleErrorMessage (error, role, streamId) {
     return {
       type: this.sfuApp,
       id: 'error',
       role,
-      endpointId,
+      streamId,
       code: error.code,
       reason: error.message,
     };
@@ -47,10 +47,10 @@ module.exports = class BaseProvider extends EventEmitter {
       type = null,
       id = null,
       role = null,
-      endpointId = null,
+      streamId = null,
       code = null,
       reason = null,
     } = error;
-    return type && id && role && endpointId && code && reason;
+    return type && id && role && streamId && code && reason;
   }
 };
diff --git a/labs/bbb-webrtc-sfu/lib/base/errors.js b/labs/bbb-webrtc-sfu/lib/base/errors.js
index 39caba3c1c..e1f48d277c 100644
--- a/labs/bbb-webrtc-sfu/lib/base/errors.js
+++ b/labs/bbb-webrtc-sfu/lib/base/errors.js
@@ -1,4 +1,4 @@
-exports.errors =  {
+const errorCodes =  {
   MIN_CODE: 2000,
   MAX_CODE: 2999,
   2000: "MEDIA_SERVER_CONNECTION_ERROR",
@@ -15,4 +15,16 @@ exports.errors =  {
   2209: "MEDIA_ADAPTER_OBJECT_NOT_FOUND",
   2210: "MEDIA_CONNECT_ERROR",
   2211: "MEDIA_NOT_FLOWING",
+  2300: "SFU_INVALID_REQUEST",
 }
+
+const expandErrors = () => {
+  let expandedErrors = Object.keys(errorCodes).reduce((map, key) => {
+    map[errorCodes[key]] = { code: key, reason: errorCodes[key] };
+    return map;
+  }, {});
+
+  return { ...errorCodes, ...expandedErrors };
+}
+
+module.exports = expandErrors();
diff --git a/labs/bbb-webrtc-sfu/lib/mcs-core/lib/media/MCSApiStub.js b/labs/bbb-webrtc-sfu/lib/mcs-core/lib/media/MCSApiStub.js
index 89d4c629e0..c6b48ebdd6 100644
--- a/labs/bbb-webrtc-sfu/lib/mcs-core/lib/media/MCSApiStub.js
+++ b/labs/bbb-webrtc-sfu/lib/mcs-core/lib/media/MCSApiStub.js
@@ -162,7 +162,7 @@ module.exports = class MCSApiStub extends EventEmitter {
 
   _handleError (error, operation, params) {
     const { code, message, details } = error;
-    const response = { type: 'error', code, message, details, operation, params};
+    const response = { type: 'error', code, message, details, operation, params };
     Logger.error("[mcs-api] Reject operation", response.operation, "with", { error: response });
 
     return response;
diff --git a/labs/bbb-webrtc-sfu/lib/mcs-core/lib/model/SdpSession.js b/labs/bbb-webrtc-sfu/lib/mcs-core/lib/model/SdpSession.js
index 55c351677b..a1eac49832 100644
--- a/labs/bbb-webrtc-sfu/lib/mcs-core/lib/model/SdpSession.js
+++ b/labs/bbb-webrtc-sfu/lib/mcs-core/lib/model/SdpSession.js
@@ -34,11 +34,15 @@ module.exports = class SdpSession extends MediaSession {
   }
 
   setOffer (offer) {
-    this._offer = new SdpWrapper(offer, this._type);
+    if (offer) {
+      this._offer = new SdpWrapper(offer, this._type);
+    }
   }
 
   setAnswer (answer) {
-    this._answer = new SdpWrapper(answer, this._type);
+    if (answer) {
+      this._answer = new SdpWrapper(answer, this._type);
+    }
   }
 
   process () {
@@ -52,7 +56,7 @@ module.exports = class SdpSession extends MediaSession {
         this.setAnswer(answer);
 
         // Checks if the media server was able to find a compatible media line
-        if (!this._hasAvailableCodec()) {
+        if (answer && !this._hasAvailableCodec()) {
           return reject(this._handleError(C.ERROR.MEDIA_NO_AVAILABLE_CODEC));
         }
 
diff --git a/labs/bbb-webrtc-sfu/lib/screenshare/ScreenshareManager.js b/labs/bbb-webrtc-sfu/lib/screenshare/ScreenshareManager.js
index 9a5a47db14..f98c989838 100644
--- a/labs/bbb-webrtc-sfu/lib/screenshare/ScreenshareManager.js
+++ b/labs/bbb-webrtc-sfu/lib/screenshare/ScreenshareManager.js
@@ -12,10 +12,12 @@ const Screenshare = require('./screenshare');
 const BaseManager = require('../base/BaseManager');
 const C = require('../bbb/messages/Constants');
 const Logger = require('../utils/Logger');
+const errors = require('../base/errors');
 
 module.exports = class ScreenshareManager extends BaseManager {
   constructor (connectionChannel, additionalChannels, logPrefix) {
     super(connectionChannel, additionalChannels, logPrefix);
+    this.sfuApp = C.SCREENSHARE_APP;
     this.messageFactory(this._onMessage);
     this._iceQueues = {};
   }
@@ -71,14 +73,9 @@ module.exports = class ScreenshareManager extends BaseManager {
           Logger.info(this._logPrefix, "Sending startResponse to peer", sessionId, "for connection", session._id);
         }
         catch (error) {
-          Logger.error(this._logPrefix, error);
+          let errorMessage = this._handleError(this._logPrefix, connectionId, callerName, role, error);
           this._bbbGW.publish(JSON.stringify({
-            connectionId: connectionId,
-            type: C.SCREENSHARE_APP,
-            role: role,
-            id : 'startResponse',
-            response : 'rejected',
-            message : error
+            ...errorMessage
           }), C.FROM_SCREENSHARE);
         }
         break;
@@ -93,7 +90,7 @@ module.exports = class ScreenshareManager extends BaseManager {
         }
         break;
 
-      case 'onIceCandidate':
+      case 'iceCandidate':
         if (session && session.constructor === Screenshare) {
           session.onIceCandidate(message.candidate, role, callerName);
         } else {
@@ -118,10 +115,9 @@ module.exports = class ScreenshareManager extends BaseManager {
         break;
 
       default:
+        const errorMessage = this._handleError(this._logPrefix, connectionId, null, null, errors.SFU_INVALID_REQUEST);
         this._bbbGW.publish(JSON.stringify({
-          connectionId: (session && session._id) ? session._id : 'none',
-          id : 'error',
-          message: 'Invalid message ' + message
+          ...errorMessage,
         }), C.FROM_SCREENSHARE);
         break;
     }
diff --git a/labs/bbb-webrtc-sfu/lib/screenshare/screenshare.js b/labs/bbb-webrtc-sfu/lib/screenshare/screenshare.js
index 7eeaebbb57..f9e71e626d 100644
--- a/labs/bbb-webrtc-sfu/lib/screenshare/screenshare.js
+++ b/labs/bbb-webrtc-sfu/lib/screenshare/screenshare.js
@@ -33,7 +33,7 @@ var rtpEndpoints = {};
 module.exports = class Screenshare extends BaseProvider {
   constructor(id, bbbgw, voiceBridge, userId, vh, vw, meetingId) {
     super();
-    this.sfuApp = "screenshare";
+    this.sfuApp = C.SCREENSHARE_APP;
     this.mcs = new MCSApi();
     this.mcsUserId;
     this.userId = userId;
diff --git a/labs/bbb-webrtc-sfu/lib/video/VideoManager.js b/labs/bbb-webrtc-sfu/lib/video/VideoManager.js
index 81bee34d12..cd7bad9624 100755
--- a/labs/bbb-webrtc-sfu/lib/video/VideoManager.js
+++ b/labs/bbb-webrtc-sfu/lib/video/VideoManager.js
@@ -11,10 +11,12 @@ const Video = require('./video');
 const BaseManager = require('../base/BaseManager');
 const C = require('../bbb/messages/Constants');
 const Logger = require('../utils/Logger');
+const errors = require('../base/errors');
 
 module.exports = class VideoManager extends BaseManager {
   constructor (connectionChannel, additionalChannels, logPrefix) {
     super(connectionChannel, additionalChannels, logPrefix);
+    this.sfuApp = C.VIDEO_APP;
     this.messageFactory(this._onMessage);
   }
 
@@ -97,14 +99,9 @@ module.exports = class VideoManager extends BaseManager {
           }), C.FROM_VIDEO);
         }
         catch (err) {
+          const errorMessage = this._handleError(this._logPrefix, connectionId, cameraId, role, error);
           return this._bbbGW.publish(JSON.stringify({
-            connectionId: connectionId,
-            type: 'video',
-            role: role,
-            id : 'error',
-            response : 'rejected',
-            cameraId : message.cameraId,
-            message :err 
+            ...errorMessage
           }), C.FROM_VIDEO);
         }
         break;
@@ -134,12 +131,9 @@ module.exports = class VideoManager extends BaseManager {
         break;
 
       default:
+        const errorMessage = this._handleError(this._logPrefix, connectionId, null, null, errors.SFU_INVALID_REQUEST);
         this._bbbGW.publish(JSON.stringify({
-          connectionId: connectionId,
-          type: 'video',
-          id : 'error',
-          response : 'rejected',
-          message : 'Invalid message ' + JSON.stringify(message)
+          ...errorMessage,
         }), C.FROM_VIDEO);
         break;
     }
diff --git a/labs/bbb-webrtc-sfu/lib/video/video.js b/labs/bbb-webrtc-sfu/lib/video/video.js
index fb0c36b679..d3154dd7ed 100644
--- a/labs/bbb-webrtc-sfu/lib/video/video.js
+++ b/labs/bbb-webrtc-sfu/lib/video/video.js
@@ -17,7 +17,7 @@ let sharedWebcams = {};
 module.exports = class Video extends BaseProvider {
   constructor(_bbbGW, _meetingId, _id, _shared, _connectionId) {
     super();
-    this.sfuApp = "video";
+    this.sfuApp = C.VIDEO_APP;
     this.mcs = new MCSApi();
     this.bbbGW = _bbbGW;
     this.id = _id;
-- 
GitLab