From c8a88148e1634a7e1860c1d794fe6fc321cb95a7 Mon Sep 17 00:00:00 2001 From: prlanzarin Date: Fri, 6 Sep 2019 18:58:22 +0000 Subject: [PATCH 1/4] Improve log message for screenshare/listen only/autoplay and harden media play with retries --- .../api/audio/client/bridge/kurento.js | 105 +++++++++++----- .../api/screenshare/client/bridge/kurento.js | 117 +++++++++++++++--- .../ui/components/screenshare/component.jsx | 24 +++- .../video-provider/video-list/component.jsx | 22 +++- .../ui/services/audio-manager/index.js | 22 +++- .../imports/utils/mediaElementPlayRetry.js | 27 ++++ 6 files changed, 262 insertions(+), 55 deletions(-) create mode 100644 bigbluebutton-html5/imports/utils/mediaElementPlayRetry.js diff --git a/bigbluebutton-html5/imports/api/audio/client/bridge/kurento.js b/bigbluebutton-html5/imports/api/audio/client/bridge/kurento.js index c5b4ac9e82..95c559adcf 100755 --- a/bigbluebutton-html5/imports/api/audio/client/bridge/kurento.js +++ b/bigbluebutton-html5/imports/api/audio/client/bridge/kurento.js @@ -1,6 +1,7 @@ import BaseAudioBridge from './base'; import Auth from '/imports/ui/services/auth'; import { fetchWebRTCMappedStunTurnServers } from '/imports/utils/fetchStunTurnServers'; +import { playAndRetry } from '/imports/utils/mediaElementPlayRetry'; import logger from '/imports/startup/client/logger'; const SFU_URL = Meteor.settings.public.kurento.wsUrl; @@ -37,6 +38,14 @@ export default class KurentoAudioBridge extends BaseAudioBridge { this.hasSuccessfullyStarted = false; } + static normalizeError(error = {}) { + const errorMessage = error.message || error.name || error.reason || 'Unknown error'; + const errorCode = error.code || 'Undefined code'; + const errorReason = error.reason || error.id || 'Undefined reason'; + + return { errorMessage, errorCode, errorReason }; + } + joinAudio({ isListenOnly, inputStream }, callback) { return new Promise(async (resolve, reject) => { this.callback = callback; @@ -59,34 +68,65 @@ export default class KurentoAudioBridge extends BaseAudioBridge { inputStream, }; + const audioTag = document.getElementById(MEDIA_TAG); + + const playElement = () => { + const mediaTagPlayed = () => { + logger.info({ + logCode: 'listenonly_media_play_success', + }, 'Listen only media played successfully'); + resolve(this.callback({ status: this.baseCallStates.started })); + }; + if (audioTag.paused) { + // Tag isn't playing yet. Play it. + audioTag.play() + .then(mediaTagPlayed) + .catch((error) => { + // NotAllowedError equals autoplay issues, fire autoplay handling event. + // This will be handled in audio-manager. + if (error.name === 'NotAllowedError') { + logger.error({ + logCode: 'listenonly_error_autoplay', + extraInfo: { errorName: error.name }, + }, 'Listen only media play failed due to autoplay error'); + const tagFailedEvent = new CustomEvent('audioPlayFailed', { detail: { mediaElement: audioTag } }); + window.dispatchEvent(tagFailedEvent); + resolve(this.callback({ + status: this.baseCallStates.autoplayBlocked, + })); + } else { + // Tag failed for reasons other than autoplay. Log the error and + // try playing again a few times until it works or fails for good + const played = playAndRetry(audioTag); + if (!played) { + logger.error({ + logCode: 'listenonly_error_media_play_failed', + extraInfo: { errorName: error.name }, + }, `Listen only media play failed due to ${error.name}`); + } else { + mediaTagPlayed(); + } + } + }); + } else { + // Media tag is already playing, so log a success. This is really a + // logging fallback for a case that shouldn't happen. But if it does + // (ie someone re-enables the autoPlay prop in the element), then it + // means the stream is playing properly and it'll be logged. + mediaTagPlayed(); + } + }; + const onSuccess = () => { const { webRtcPeer } = window.kurentoManager.kurentoAudio; this.hasSuccessfullyStarted = true; if (webRtcPeer) { - const audioTag = document.getElementById(MEDIA_TAG); const stream = webRtcPeer.getRemoteStream(); audioTag.pause(); audioTag.srcObject = stream; audioTag.muted = false; - audioTag.play() - .then(() => { - resolve(this.callback({ status: this.baseCallStates.started })); - }) - .catch((error) => { - // NotAllowedError equals autoplay issues, fire autoplay handling event - if (error.name === 'NotAllowedError') { - const tagFailedEvent = new CustomEvent('audioPlayFailed', { detail: { mediaElement: audioTag } }); - window.dispatchEvent(tagFailedEvent); - } - logger.warn({ - logCode: 'sfuaudiobridge_play_maybe_error', - extraInfo: { error }, - }, `Listen only media play failed due to ${error.name}`); - resolve(this.callback({ - status: this.baseCallStates.autoplayBlocked, - })); - }); + playElement(); } else { this.callback({ status: this.baseCallStates.failed, @@ -101,12 +141,13 @@ export default class KurentoAudioBridge extends BaseAudioBridge { }; const onFail = (error) => { + const { errorMessage, errorCode, errorReason } = KurentoAudioBridge.normalizeError(error); // Listen only connected successfully already and dropped mid-call. // Try to reconnect ONCE (binded to reconnectOngoing flag) if (this.hasSuccessfullyStarted && !this.reconnectOngoing) { logger.error({ - logCode: 'sfuaudiobridge_listen_only_error_reconnect', - extraInfo: { error }, + logCode: 'listenonly_error_try_to_reconnect', + extraInfo: { errorMessage, errorCode, errorReason }, }, 'Listen only failed for an ongoing session, try to reconnect'); window.kurentoExitAudio(); this.callback({ status: this.baseCallStates.reconnecting }); @@ -135,26 +176,34 @@ export default class KurentoAudioBridge extends BaseAudioBridge { } else { // Already tried reconnecting once OR the user handn't succesfully // connected firsthand. Just finish the session and reject with error + if (!this.reconnectOngoing) { + logger.error({ + logCode: 'listenonly_error_failed_to_connect', + extraInfo: { errorMessage, errorCode, errorReason }, + }, `Listen only failed when trying to start due to ${errorMessage}`); + } else { + logger.error({ + logCode: 'listenonly_error_reconnect_failed', + extraInfo: { errorMessage, errorCode, errorReason }, + }, `Listen only failed when trying to reconnect due to ${errorMessage}`); + } + this.reconnectOngoing = false; this.hasSuccessfullyStarted = false; window.kurentoExitAudio(); - let reason = 'Undefined'; - if (error) { - reason = error.reason || error.id || error; - } this.callback({ status: this.baseCallStates.failed, error: this.baseErrorCodes.CONNECTION_ERROR, - bridgeError: reason, + bridgeError: errorReason, }); - reject(reason); + reject(errorReason); } }; if (!isListenOnly) { - return reject('Invalid bridge option'); + return reject(new Error('Invalid bridge option')); } window.kurentoJoinAudio( diff --git a/bigbluebutton-html5/imports/api/screenshare/client/bridge/kurento.js b/bigbluebutton-html5/imports/api/screenshare/client/bridge/kurento.js index af155e19b6..49ce6e610f 100755 --- a/bigbluebutton-html5/imports/api/screenshare/client/bridge/kurento.js +++ b/bigbluebutton-html5/imports/api/screenshare/client/bridge/kurento.js @@ -1,6 +1,7 @@ import Auth from '/imports/ui/services/auth'; import BridgeService from './service'; import { fetchWebRTCMappedStunTurnServers } from '/imports/utils/fetchStunTurnServers'; +import { playAndRetry } from '/imports/utils/mediaElementPlayRetry'; import logger from '/imports/startup/client/logger'; const SFU_CONFIG = Meteor.settings.public.kurento; @@ -20,13 +21,40 @@ const getMeetingId = () => Auth.meetingID; const getSessionToken = () => Auth.sessionToken; export default class KurentoScreenshareBridge { + static normalizeError(error = {}) { + const errorMessage = error.message || error.name || error.reason || 'Unknown error'; + const errorCode = error.code || 'Undefined code'; + const errorReason = error.reason || error.id || 'Undefined reason'; + + return { errorMessage, errorCode, errorReason }; + } + + static handlePresenterFailure(error) { + const normalizedError = KurentoScreenshareBridge.normalizeError(error); + logger.error({ + logCode: 'screenshare_presenter_error_failed_to_connect', + extraInfo: { ...normalizedError }, + }, `Screenshare presenter failed when trying to start due to ${normalizedError.errorMessage}`); + return normalizedError; + } + + static handleViewerFailure(error) { + const normalizedError = KurentoScreenshareBridge.normalizeError(error); + logger.error({ + logCode: 'screenshare_viewer_error_failed_to_connect', + extraInfo: { ...normalizedError }, + }, `Screenshare viewer failed when trying to start due to ${normalizedError.errorMessage}`); + + return normalizedError; + } + async kurentoWatchVideo() { let iceServers = []; try { iceServers = await fetchWebRTCMappedStunTurnServers(getSessionToken()); } catch (error) { - logger.error({ logCode: 'sfuviewscreen_fetchstunturninfo_error', extraInfo: { error } }, + logger.error({ logCode: 'screenshare_viwer_fetchstunturninfo_error', extraInfo: { error } }, 'Screenshare bridge failed to fetch STUN/TURN info, using default'); } finally { const options = { @@ -35,26 +63,67 @@ export default class KurentoScreenshareBridge { logger, }; + const screenshareTag = document.getElementById(SCREENSHARE_VIDEO_TAG); + + const playElement = () => { + const mediaTagPlayed = () => { + logger.info({ + logCode: 'screenshare_viewer_media_play_success', + }, 'Screenshare viewer media played successfully'); + }; + if (screenshareTag.paused) { + // Tag isn't playing yet. Play it. + screenshareTag.play() + .then(mediaTagPlayed) + .catch((error) => { + // NotAllowedError equals autoplay issues, fire autoplay handling event. + // This will be handled in the screenshare react component. + if (error.name === 'NotAllowedError') { + logger.error({ + logCode: 'screenshare_viewer_error_autoplay', + extraInfo: { errorName: error.name }, + }, 'Screenshare viewer play failed due to autoplay error'); + const tagFailedEvent = new CustomEvent('screensharePlayFailed', + { detail: { mediaElement: screenshareTag } }); + window.dispatchEvent(tagFailedEvent); + } else { + // Tag failed for reasons other than autoplay. Log the error and + // try playing again a few times until it works or fails for good + const played = playAndRetry(screenshareTag); + if (!played) { + logger.error({ + logCode: 'screenshare_viewer_error_media_play_failed', + extraInfo: { errorName: error.name }, + }, `Screenshare viewer media play failed due to ${error.name}`); + } else { + mediaTagPlayed(); + } + } + }); + } else { + // Media tag is already playing, so log a success. This is really a + // logging fallback for a case that shouldn't happen. But if it does + // (ie someone re-enables the autoPlay prop in the element), then it + // means the stream is playing properly and it'll be logged. + mediaTagPlayed(); + } + }; + + const onFail = (error) => { + KurentoScreenshareBridge.handleViewerFailure(error); + }; + + // Callback for the kurento-extension.js script. It's called when the whole + // negotiation with SFU is successful. This will load the stream into the + // screenshare media element and play it manually. const onSuccess = () => { const { webRtcPeer } = window.kurentoManager.kurentoVideo; if (webRtcPeer) { - const screenshareTag = document.getElementById(SCREENSHARE_VIDEO_TAG); const stream = webRtcPeer.getRemoteStream(); screenshareTag.muted = true; screenshareTag.pause(); screenshareTag.srcObject = stream; - screenshareTag.play().catch((error) => { - // NotAllowedError equals autoplay issues, fire autoplay handling event - if (error.name === 'NotAllowedError') { - const tagFailedEvent = new CustomEvent('screensharePlayFailed', - { detail: { mediaElement: screenshareTag } }); - window.dispatchEvent(tagFailedEvent); - } - logger.warn({ - logCode: 'sfuscreenshareview_play_maybe_error', - extraInfo: { error }, - }, `Screenshare viewer media play failed due to ${error.name}`); - }); + playElement(); } }; @@ -63,7 +132,7 @@ export default class KurentoScreenshareBridge { BridgeService.getConferenceBridge(), getUserId(), getMeetingId(), - null, + onFail, onSuccess, options, ); @@ -79,7 +148,7 @@ export default class KurentoScreenshareBridge { try { iceServers = await fetchWebRTCMappedStunTurnServers(getSessionToken()); } catch (error) { - logger.error({ logCode: 'sfusharescreen_fetchstunturninfo_error' }, + logger.error({ logCode: 'screenshare_presenter_fetchstunturninfo_error' }, 'Screenshare bridge failed to fetch STUN/TURN info, using default'); } finally { const options = { @@ -90,13 +159,25 @@ export default class KurentoScreenshareBridge { iceServers, logger, }; + + const failureCallback = (error) => { + const normalizedError = KurentoScreenshareBridge.handlePresenterFailure(error); + onFail(normalizedError); + }; + + const successCallback = () => { + logger.info({ + logCode: 'screenshare_presenter_start_success', + }, 'Screenshare presenter started succesfully'); + }; + window.kurentoShareScreen( SCREENSHARE_VIDEO_TAG, BridgeService.getConferenceBridge(), getUserId(), getMeetingId(), - onFail, - null, + failureCallback, + successCallback, options, ); } diff --git a/bigbluebutton-html5/imports/ui/components/screenshare/component.jsx b/bigbluebutton-html5/imports/ui/components/screenshare/component.jsx index aae6e278ce..3781cbab9c 100755 --- a/bigbluebutton-html5/imports/ui/components/screenshare/component.jsx +++ b/bigbluebutton-html5/imports/ui/components/screenshare/component.jsx @@ -6,6 +6,8 @@ import FullscreenService from '../fullscreen-button/service'; import FullscreenButtonContainer from '../fullscreen-button/container'; import { styles } from './styles'; import AutoplayOverlay from '../media/autoplay-overlay/component'; +import logger from '/imports/startup/client/logger'; +import playAndRetry from '/imports/utils/mediaElementPlayRetry'; const intlMessages = defineMessages({ screenShareLabel: { @@ -80,13 +82,24 @@ class ScreenshareComponent extends React.Component { handleAllowAutoplay() { const { autoplayBlocked } = this.state; + logger.info({ + logCode: 'screenshare_autoplay_allowed', + }, 'Screenshare media autoplay allowed by the user'); + window.removeEventListener('screensharePlayFailed', this.handlePlayElementFailed); while (this.failedMediaElements.length) { const mediaElement = this.failedMediaElements.shift(); if (mediaElement) { - mediaElement.play().catch(() => { - // Ignore the error for now. - }); + const played = playAndRetry(mediaElement); + if (!played) { + logger.error({ + logCode: 'screenshare_autoplay_handling_failed', + }, 'Screenshare autoplay handling failed to play media'); + } else { + logger.info({ + logCode: 'screenshare_viewer_media_play_success', + }, 'Screenshare viewer media played successfully'); + } } } if (autoplayBlocked) { this.setState({ autoplayBlocked: false }); } @@ -99,6 +112,10 @@ class ScreenshareComponent extends React.Component { e.stopPropagation(); this.failedMediaElements.push(mediaElement); if (!autoplayBlocked) { + logger.info({ + logCode: 'screenshare_autoplay_prompt', + }, 'Prompting user for action to play screenshare media'); + this.setState({ autoplayBlocked: true }); } } @@ -154,7 +171,6 @@ class ScreenshareComponent extends React.Component { id="screenshareVideo" key="screenshareVideo" style={{ maxHeight: '100%', width: '100%' }} - autoPlay playsInline onLoadedData={this.onVideoLoad} ref={(ref) => { this.videoTag = ref; }} diff --git a/bigbluebutton-html5/imports/ui/components/video-provider/video-list/component.jsx b/bigbluebutton-html5/imports/ui/components/video-provider/video-list/component.jsx index d42afebf6a..58d2968f88 100755 --- a/bigbluebutton-html5/imports/ui/components/video-provider/video-list/component.jsx +++ b/bigbluebutton-html5/imports/ui/components/video-provider/video-list/component.jsx @@ -7,6 +7,8 @@ import { styles } from './styles'; import VideoListItemContainer from './video-list-item/container'; import { withDraggableConsumer } from '../../media/webcam-draggable-overlay/context'; import AutoplayOverlay from '../../media/autoplay-overlay/component'; +import logger from '/imports/startup/client/logger'; +import playAndRetry from '/imports/utils/mediaElementPlayRetry'; const propTypes = { users: PropTypes.arrayOf(PropTypes.object).isRequired, @@ -145,14 +147,25 @@ class VideoList extends Component { handleAllowAutoplay() { const { autoplayBlocked } = this.state; + logger.info({ + logCode: 'video_provider_autoplay_allowed', + }, 'Video media autoplay allowed by the user'); + this.autoplayWasHandled = true; window.removeEventListener('videoPlayFailed', this.handlePlayElementFailed); while (this.failedMediaElements.length) { const mediaElement = this.failedMediaElements.shift(); if (mediaElement) { - mediaElement.play().catch(() => { - // Ignore the error for now. - }); + const played = playAndRetry(mediaElement); + if (!played) { + logger.error({ + logCode: 'video_provider_autoplay_handling_failed', + }, 'Video autoplay handling failed to play media'); + } else { + logger.info({ + logCode: 'video_provider_media_play_success', + }, 'Video media played successfully'); + } } } if (autoplayBlocked) { this.setState({ autoplayBlocked: false }); } @@ -165,6 +178,9 @@ class VideoList extends Component { e.stopPropagation(); this.failedMediaElements.push(mediaElement); if (!autoplayBlocked && !this.autoplayWasHandled) { + logger.info({ + logCode: 'video_provider_autoplay_prompt', + }, 'Prompting user for action to play video media'); this.setState({ autoplayBlocked: true }); } } diff --git a/bigbluebutton-html5/imports/ui/services/audio-manager/index.js b/bigbluebutton-html5/imports/ui/services/audio-manager/index.js index e2458d4ffb..55a82ee7fc 100755 --- a/bigbluebutton-html5/imports/ui/services/audio-manager/index.js +++ b/bigbluebutton-html5/imports/ui/services/audio-manager/index.js @@ -8,6 +8,7 @@ import { notify } from '/imports/ui/services/notification'; import browser from 'browser-detect'; import iosWebviewAudioPolyfills from '../../../utils/ios-webview-audio-polyfills'; import { tryGenerateIceCandidates } from '../../../utils/safari-webrtc'; +import playAndRetry from '/imports/utils/mediaElementPlayRetry'; const MEDIA = Meteor.settings.public.media; const MEDIA_TAG = MEDIA.mediaTag; @@ -485,11 +486,25 @@ class AudioManager { handleAllowAutoplay() { window.removeEventListener('audioPlayFailed', this.handlePlayElementFailed); + + logger.info({ + logCode: 'audiomanager_autoplay_allowed', + }, 'Listen only autoplay allowed by the user'); + while (this.failedMediaElements.length) { const mediaElement = this.failedMediaElements.shift(); if (mediaElement) { - mediaElement.play().catch(() => { - // Ignore the error for now. + playAndRetry(mediaElement).then((played) => { + if (!played) { + logger.error({ + logCode: 'audiomanager_autoplay_handling_failed', + }, 'Listen only autoplay handling failed to play media'); + } else { + // logCode is listenonly_* to make it consistent with the other tag play log + logger.info({ + logCode: 'listenonly_media_play_success', + }, 'Listen only media played successfully'); + } }); } } @@ -502,6 +517,9 @@ class AudioManager { e.stopPropagation(); this.failedMediaElements.push(mediaElement); if (!this.autoplayBlocked) { + logger.info({ + logCode: 'audiomanager_autoplay_prompt', + }, 'Prompting user for action to play listen only media'); this.autoplayBlocked = true; } } diff --git a/bigbluebutton-html5/imports/utils/mediaElementPlayRetry.js b/bigbluebutton-html5/imports/utils/mediaElementPlayRetry.js new file mode 100644 index 0000000000..fc8e1ec73e --- /dev/null +++ b/bigbluebutton-html5/imports/utils/mediaElementPlayRetry.js @@ -0,0 +1,27 @@ +const DEFAULT_MAX_RETRIES = 10; +const DEFAULT_RETRY_TIMEOUT = 500; + +const playAndRetry = async (mediaElement, maxRetries = DEFAULT_MAX_RETRIES) => { + let attempt = 0; + let played = false; + + const playElement = () => new Promise((resolve, reject) => { + setTimeout(() => { + mediaElement.play().then(resolve).catch(reject); + }, DEFAULT_RETRY_TIMEOUT); + }); + + while (!played && attempt < maxRetries && mediaElement.paused) { + try { + await playElement(); + played = true; + return played; + } catch (error) { + attempt += 1; + } + } + + return played || mediaElement.paused; +}; + +export default playAndRetry; From 397286b7c20fa7ff9fd157e8219103dbec107956 Mon Sep 17 00:00:00 2001 From: prlanzarin Date: Fri, 6 Sep 2019 19:00:34 +0000 Subject: [PATCH 2/4] Add an ICE queue to kurento-extension to only process candidates after the remote answer is set This fixes a webkit quirk and should get rid of some of the screensharing/listen only problems with Safari Also improved the extraInfo error logging in the script to log only the name instead of the whole object on browser errors --- .../public/compatibility/kurento-extension.js | 85 ++++++++++++++++--- 1 file changed, 71 insertions(+), 14 deletions(-) diff --git a/bigbluebutton-html5/public/compatibility/kurento-extension.js b/bigbluebutton-html5/public/compatibility/kurento-extension.js index 0394f67e5c..2c9298c441 100755 --- a/bigbluebutton-html5/public/compatibility/kurento-extension.js +++ b/bigbluebutton-html5/public/compatibility/kurento-extension.js @@ -245,7 +245,7 @@ Kurento.prototype.init = function () { kurentoManager.exitScreenShare(); this.logger.error({ logCode: 'kurentoextension_websocket_error', - extraInfo: { error } + extraInfo: { errorMessage: error.name || error.message || 'Unknown error' } }, 'Error in the WebSocket connection to SFU, screenshare/listen only will drop'); self.onFail('Websocket connection error'); }; @@ -269,7 +269,7 @@ Kurento.prototype.onWSMessage = function (message) { kurentoManager.exitScreenShare(); break; case 'iceCandidate': - this.webRtcPeer.addIceCandidate(parsedMessage.candidate); + this.handleIceCandidate(parsedMessage.candidate); break; case 'webRTCAudioSuccess': this.onSuccess(parsedMessage.success); @@ -292,6 +292,47 @@ Kurento.prototype.setRenderTag = function (tag) { this.renderTag = tag; }; +Kurento.prototype.processIceQueue = function () { + const peer = this.webRtcPeer; + while (peer.iceQueue.length) { + const candidate = peer.iceQueue.shift(); + peer.addIceCandidate(candidate, (error) => { + if (error) { + // Just log the error. We can't be sure if a candidate failure on add is + // fatal or not, so that's why we have a timeout set up for negotiations and + // listeners for ICE state transitioning to failures, so we won't act on it here + this.logger.error({ + logCode: 'kurentoextension_addicecandidate_error', + extraInfo: { errorMessage: error.name || error.message || 'Unknown error' }, + }, `Adding ICE candidate failed due to ${error.message}`); + } + }); + } +} + +Kurento.prototype.handleIceCandidate = function (candidate) { + const peer = this.webRtcPeer; + if (peer.negotiated) { + peer.addIceCandidate(candidate, (error) => { + if (error) { + // Just log the error. We can't be sure if a candidate failure on add is + // fatal or not, so that's why we have a timeout set up for negotiations and + // listeners for ICE state transitioning to failures, so we won't act on it here + this.logger.error({ + logCode: 'kurentoextension_addicecandidate_error', + extraInfo: { errorMessage: error.name || error.message || 'Unknown error' }, + }, `Adding ICE candidate failed due to ${error.message}`); + } + }); + } else { + // ICE candidates are queued until a SDP answer has been processed. + // This was done due to a long term iOS/Safari quirk where it'd + // fail if candidates were added before the offer/answer cycle was completed. + // IT STILL HAPPENS - prlanzarin sept 2019 + peer.iceQueue.push(candidate); + } +} + Kurento.prototype.startResponse = function (message) { if (message.response !== 'accepted') { this.handleSFUError(message); @@ -300,12 +341,27 @@ Kurento.prototype.startResponse = function (message) { logCode: 'kurentoextension_start_success', extraInfo: { sfuResponse: message } }, `Start request accepted for ${message.type}`); - this.webRtcPeer.processAnswer(message.sdpAnswer); - // audio calls gets their success callback in a subsequent step (@webRTCAudioSuccess) - // due to legacy messaging which I don't intend to break now - prlanzarin - if (message.type === 'screenshare') { - this.onSuccess() - } + + this.webRtcPeer.processAnswer(message.sdpAnswer, (error) => { + if (error) { + this.logger.error({ + logCode: 'kurentoextension_peerconnection_processanswer_error', + extraInfo: { + errorMessage: error.name || error.message || 'Unknown error', + }, + }, `Processing SDP answer from SFU for failed due to ${error.message}`); + + return this.onFail(error); + } + // Mark the peer as negotiated and flush the ICE queue + this.webRtcPeer.negotiated = true; + this.processIceQueue(); + // audio calls gets their success callback in a subsequent step (@webRTCAudioSuccess) + // due to legacy messaging which I don't intend to break now - prlanzarin + if (message.type === 'screenshare') { + this.onSuccess() + } + }); } }; @@ -343,7 +399,7 @@ Kurento.prototype.onOfferPresenter = function (error, offerSdp) { if (error) { this.logger.error({ logCode: 'kurentoextension_screenshare_presenter_offer_failure', - extraInfo: { error } + extraInfo: { errorMessage: error.name || error.message || 'Unknown error' }, }, `Failed to generate peer connection offer for screenshare presenter with error ${error.message}`); this.onFail(error); return; @@ -407,12 +463,13 @@ Kurento.prototype.startScreensharing = function () { if (error) { this.logger.error({ logCode: 'kurentoextension_screenshare_peerconnection_create_error', - extraInfo: { error } + extraInfo: { errorMessage: error.name || error.message || 'Unknown error' }, }, `WebRTC peer constructor for screenshare (presenter) failed due to ${error.message}`); this.onFail(error); return kurentoManager.exitScreenShare(); } + this.webRtcPeer.iceQueue = []; this.webRtcPeer.generateOffer(this.onOfferPresenter.bind(this)); const localStream = this.webRtcPeer.peerConnection.getLocalStreams()[0]; @@ -486,7 +543,7 @@ Kurento.prototype.viewer = function () { if (error) { return self.onFail(error); } - + self.webRtcPeer.iceQueue = []; this.generateOffer(self.onOfferViewer.bind(self)); }); self.webRtcPeer.peerConnection.oniceconnectionstatechange = () => { @@ -510,7 +567,7 @@ Kurento.prototype.onOfferViewer = function (error, offerSdp) { if (error) { this.logger.error({ logCode: 'kurentoextension_screenshare_viewer_offer_failure', - extraInfo: { error } + extraInfo: { errorMessage: error.name || error.message || 'Unknown error' }, }, `Failed to generate peer connection offer for screenshare viewer with error ${error.message}`); return this.onFail(error); @@ -564,7 +621,7 @@ Kurento.prototype.listenOnly = function () { if (error) { return self.onFail(error); } - + self.webRtcPeer.iceQueue = []; this.generateOffer(self.onOfferListenOnly.bind(self)); }); } @@ -592,7 +649,7 @@ Kurento.prototype.onOfferListenOnly = function (error, offerSdp) { if (error) { this.logger.error({ logCode: 'kurentoextension_listenonly_offer_failure', - extraInfo: { error } + extraInfo: { errorMessage: error.name || error.message || 'Unknown error' }, }, `Failed to generate peer connection offer for listen only with error ${error.message}`); return this.onFail(error); From 73698b1064d7588c7eadb20bc64e0c06590f2eb0 Mon Sep 17 00:00:00 2001 From: prlanzarin Date: Fri, 6 Sep 2019 20:54:48 +0000 Subject: [PATCH 3/4] Fix playAndRetry import --- bigbluebutton-html5/imports/api/audio/client/bridge/kurento.js | 2 +- .../imports/api/screenshare/client/bridge/kurento.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bigbluebutton-html5/imports/api/audio/client/bridge/kurento.js b/bigbluebutton-html5/imports/api/audio/client/bridge/kurento.js index 95c559adcf..9e32b75222 100755 --- a/bigbluebutton-html5/imports/api/audio/client/bridge/kurento.js +++ b/bigbluebutton-html5/imports/api/audio/client/bridge/kurento.js @@ -1,7 +1,7 @@ import BaseAudioBridge from './base'; import Auth from '/imports/ui/services/auth'; import { fetchWebRTCMappedStunTurnServers } from '/imports/utils/fetchStunTurnServers'; -import { playAndRetry } from '/imports/utils/mediaElementPlayRetry'; +import playAndRetry from '/imports/utils/mediaElementPlayRetry'; import logger from '/imports/startup/client/logger'; const SFU_URL = Meteor.settings.public.kurento.wsUrl; diff --git a/bigbluebutton-html5/imports/api/screenshare/client/bridge/kurento.js b/bigbluebutton-html5/imports/api/screenshare/client/bridge/kurento.js index 49ce6e610f..ee0fc23d55 100755 --- a/bigbluebutton-html5/imports/api/screenshare/client/bridge/kurento.js +++ b/bigbluebutton-html5/imports/api/screenshare/client/bridge/kurento.js @@ -1,7 +1,7 @@ import Auth from '/imports/ui/services/auth'; import BridgeService from './service'; import { fetchWebRTCMappedStunTurnServers } from '/imports/utils/fetchStunTurnServers'; -import { playAndRetry } from '/imports/utils/mediaElementPlayRetry'; +import playAndRetry from '/imports/utils/mediaElementPlayRetry'; import logger from '/imports/startup/client/logger'; const SFU_CONFIG = Meteor.settings.public.kurento; From 5caac33e33479d76a8920d29d14946effeb4365a Mon Sep 17 00:00:00 2001 From: prlanzarin Date: Fri, 6 Sep 2019 20:56:20 +0000 Subject: [PATCH 4/4] Further normalize kurento-extension script errors --- .../public/compatibility/kurento-extension.js | 54 ++++++++++++------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/bigbluebutton-html5/public/compatibility/kurento-extension.js b/bigbluebutton-html5/public/compatibility/kurento-extension.js index 2c9298c441..336a56144d 100755 --- a/bigbluebutton-html5/public/compatibility/kurento-extension.js +++ b/bigbluebutton-html5/public/compatibility/kurento-extension.js @@ -243,11 +243,12 @@ Kurento.prototype.init = function () { }; this.ws.onerror = (error) => { kurentoManager.exitScreenShare(); + const { errorMessage, errorCode, errorReason } = this.normalizeError(error); this.logger.error({ logCode: 'kurentoextension_websocket_error', - extraInfo: { errorMessage: error.name || error.message || 'Unknown error' } + extraInfo: { errorMessage, errorCode, errorReason }, }, 'Error in the WebSocket connection to SFU, screenshare/listen only will drop'); - self.onFail('Websocket connection error'); + self.onFail({ errorMessage, errorCode, errorReason }); }; this.ws.onopen = function () { self.pingInterval = setInterval(self.ping.bind(self), self.PING_INTERVAL); @@ -301,10 +302,11 @@ Kurento.prototype.processIceQueue = function () { // Just log the error. We can't be sure if a candidate failure on add is // fatal or not, so that's why we have a timeout set up for negotiations and // listeners for ICE state transitioning to failures, so we won't act on it here + const { errorMessage, errorCode, errorReason } = this.normalizeError(error); this.logger.error({ logCode: 'kurentoextension_addicecandidate_error', - extraInfo: { errorMessage: error.name || error.message || 'Unknown error' }, - }, `Adding ICE candidate failed due to ${error.message}`); + extraInfo: { errorMessage, errorCode, errorReason }, + }, `Adding ICE candidate failed due to ${errorMessage}`); } }); } @@ -318,10 +320,11 @@ Kurento.prototype.handleIceCandidate = function (candidate) { // Just log the error. We can't be sure if a candidate failure on add is // fatal or not, so that's why we have a timeout set up for negotiations and // listeners for ICE state transitioning to failures, so we won't act on it here + const { errorMessage, errorCode, errorReason } = this.normalizeError(error); this.logger.error({ logCode: 'kurentoextension_addicecandidate_error', - extraInfo: { errorMessage: error.name || error.message || 'Unknown error' }, - }, `Adding ICE candidate failed due to ${error.message}`); + extraInfo: { errorMessage, errorCode, errorReason }, + }, `Adding ICE candidate failed due to ${errorMessage}`); } }); } else { @@ -333,6 +336,14 @@ Kurento.prototype.handleIceCandidate = function (candidate) { } } +Kurento.prototype.normalizeError = function (error = {}) { + const errorMessage = error.message || error.name || error.reason || 'Unknown error'; + const errorCode = error.code || 'Undefined code'; + const errorReason = error.reason || error.id || 'Undefined reason'; + + return { errorMessage, errorCode, errorReason }; +} + Kurento.prototype.startResponse = function (message) { if (message.response !== 'accepted') { this.handleSFUError(message); @@ -344,14 +355,13 @@ Kurento.prototype.startResponse = function (message) { this.webRtcPeer.processAnswer(message.sdpAnswer, (error) => { if (error) { + const { errorMessage, errorCode, errorReason } = this.normalizeError(error); this.logger.error({ logCode: 'kurentoextension_peerconnection_processanswer_error', - extraInfo: { - errorMessage: error.name || error.message || 'Unknown error', - }, - }, `Processing SDP answer from SFU for failed due to ${error.message}`); + extraInfo: { errorMessage, errorCode, errorReason }, + }, `Processing SDP answer from SFU for failed due to ${errorMessage}`); - return this.onFail(error); + return this.onFail({ errorMessage, errorCode, errorReason }); } // Mark the peer as negotiated and flush the ICE queue this.webRtcPeer.negotiated = true; @@ -390,18 +400,19 @@ Kurento.prototype.handleSFUError = function (sfuResponse) { break; } - this.onFail( { code, reason } ); + this.onFail({ errorMessage: reason, errorCode: code, errorReason: reason }); }; Kurento.prototype.onOfferPresenter = function (error, offerSdp) { const self = this; if (error) { + const { errorMessage, errorCode, errorReason } = this.normalizeError(error); this.logger.error({ logCode: 'kurentoextension_screenshare_presenter_offer_failure', - extraInfo: { errorMessage: error.name || error.message || 'Unknown error' }, + extraInfo: { errorMessage, errorCode, errorReason }, }, `Failed to generate peer connection offer for screenshare presenter with error ${error.message}`); - this.onFail(error); + this.onFail({ errorMessage, errorCode, errorReason }); return; } @@ -461,11 +472,12 @@ Kurento.prototype.startScreensharing = function () { this.webRtcPeer = kurentoUtils.WebRtcPeer.WebRtcPeerSendonly(options, (error) => { if (error) { + const { errorMessage, errorCode, errorReason } = this.normalizeError(error); this.logger.error({ logCode: 'kurentoextension_screenshare_peerconnection_create_error', - extraInfo: { errorMessage: error.name || error.message || 'Unknown error' }, + extraInfo: { errorMessage, errorCode, errorReason }, }, `WebRTC peer constructor for screenshare (presenter) failed due to ${error.message}`); - this.onFail(error); + this.onFail({ errorMessage, errorCode, errorReason }); return kurentoManager.exitScreenShare(); } @@ -565,12 +577,13 @@ Kurento.prototype.viewer = function () { Kurento.prototype.onOfferViewer = function (error, offerSdp) { const self = this; if (error) { + const { errorMessage, errorCode, errorReason } = this.normalizeError(error); this.logger.error({ logCode: 'kurentoextension_screenshare_viewer_offer_failure', - extraInfo: { errorMessage: error.name || error.message || 'Unknown error' }, + extraInfo: { errorMessage, errorCode, errorReason }, }, `Failed to generate peer connection offer for screenshare viewer with error ${error.message}`); - return this.onFail(error); + return this.onFail({ errorMessage, errorCode, errorReason }); } const message = { @@ -647,12 +660,13 @@ Kurento.prototype.onListenOnlyIceCandidate = function (candidate) { Kurento.prototype.onOfferListenOnly = function (error, offerSdp) { const self = this; if (error) { + const { errorMessage, errorCode, errorReason } = this.normalizeError(error); this.logger.error({ logCode: 'kurentoextension_listenonly_offer_failure', - extraInfo: { errorMessage: error.name || error.message || 'Unknown error' }, + extraInfo: { errorMessage, errorCode, errorReason }, }, `Failed to generate peer connection offer for listen only with error ${error.message}`); - return this.onFail(error); + return this.onFail({ errorMessage, errorCode, errorReason }); } const message = {