From 5090a36a62b1eaeb84897cca6ce4ae7ebdc21767 Mon Sep 17 00:00:00 2001 From: amolghode1981 <86001342+amolghode1981@users.noreply.github.com> Date: Mon, 23 May 2022 18:58:26 +0530 Subject: [PATCH] [FIX] Fixing Network connectivity issues with SIP client. (#25391) * Clickup Task: https://app.clickup.com/t/245c0d8 Description: The previous PR did not handle the issues completely. This PR is expected to handle 1. Clearing call related UI when the network is disconnected or switched. 2. Do clean connectivity. There were few issues discovered in earlier implementation. e.g endpoint would randomly get disconnected after a while. This was due to the fact that the earlier socket disconnection caused the removal of contact on asterisk. This should be fixed in this PR. 3. This PR contains a lot of logs. This will be removed before the final merge. * Clickup Task: https://app.clickup.com/t/245c0d8 Description: Fixing error. * Clickup Task: https://app.clickup.com/t/245c0d8 Description: Removed log statents. Changed the call disconnection logic. The call now gets disconnected when the network disconnects. Earlier it would get disconnected when the network was restored back. * Clickup Task: https://app.clickup.com/t/245c0d8 Description: Cleaning VoIPUser.ts. * Clickup Task: https://app.clickup.com/t/245c0d8 Description: Handling review comments * Clickup Task: https://app.clickup.com/t/245c0d8 Description: Correctly handling the registration flag when network is restored. * Clickup Task: https://app.clickup.com/t/245c0d8 Description: Fixing review comments. --- .../meteor/app/lib/server/startup/settings.ts | 9 ++ apps/meteor/client/lib/voip/SimpleVoipUser.ts | 2 + apps/meteor/client/lib/voip/VoIPUser.ts | 134 +++++++++++++++--- .../providers/CallProvider/CallProvider.tsx | 55 ++++++- .../CallProvider/hooks/useVoipClient.ts | 14 +- .../rocketchat-i18n/i18n/en.i18n.json | 2 + .../src/voip/VoIPUserConfiguration.ts | 7 +- 7 files changed, 203 insertions(+), 20 deletions(-) diff --git a/apps/meteor/app/lib/server/startup/settings.ts b/apps/meteor/app/lib/server/startup/settings.ts index 8683597ef8334..112241a71f692 100644 --- a/apps/meteor/app/lib/server/startup/settings.ts +++ b/apps/meteor/app/lib/server/startup/settings.ts @@ -3242,6 +3242,15 @@ settingsRegistry.addGroup('Call_Center', function () { value: true, }, }); + this.add('VoIP_Enable_Keep_Alive_For_Unstable_Networks', true, { + type: 'boolean', + public: true, + i18nDescription: 'VoIP_Enable_Keep_Alive_For_Unstable_Networks_Description', + enableQuery: { + _id: 'Livechat_enabled', + value: true, + }, + }); }); this.section('Management_Server', function () { diff --git a/apps/meteor/client/lib/voip/SimpleVoipUser.ts b/apps/meteor/client/lib/voip/SimpleVoipUser.ts index 818255b14dab2..4e4bc374b2ce5 100644 --- a/apps/meteor/client/lib/voip/SimpleVoipUser.ts +++ b/apps/meteor/client/lib/voip/SimpleVoipUser.ts @@ -10,6 +10,7 @@ export class SimpleVoipUser { webSocketPath: string, iceServers: Array, voipRetryCount: number, + enableKeepAliveForUnstableNetworks: boolean, callType?: 'audio' | 'video', mediaStreamRendered?: IMediaStreamRenderer, ): Promise { @@ -21,6 +22,7 @@ export class SimpleVoipUser { enableVideo: callType === 'video', iceServers, connectionRetryCount: voipRetryCount, + enableKeepAliveUsingOptionsForUnstableNetworks: enableKeepAliveForUnstableNetworks, }; return VoIPUser.create(config, mediaStreamRendered); diff --git a/apps/meteor/client/lib/voip/VoIPUser.ts b/apps/meteor/client/lib/voip/VoIPUser.ts index e72293dca16ca..e8628b4efbd87 100644 --- a/apps/meteor/client/lib/voip/VoIPUser.ts +++ b/apps/meteor/client/lib/voip/VoIPUser.ts @@ -34,7 +34,7 @@ import { SessionInviteOptions, RequestPendingError, } from 'sip.js'; -import { OutgoingByeRequest, URI } from 'sip.js/lib/core'; +import { OutgoingByeRequest, OutgoingRequestDelegate, URI } from 'sip.js/lib/core'; import { SessionDescriptionHandler, SessionDescriptionHandlerOptions } from 'sip.js/lib/platform/web'; import { toggleMediaStreamTracks } from './Helper'; @@ -83,6 +83,14 @@ export class VoIPUser extends Emitter { private onlineNetworkHandler: () => void; + private optionsKeepaliveInterval = 5; + + private optionsKeepAliveDebounceTimeInSec = 5; + + private optionsKeepAliveDebounceCount = 3; + + private attemptRegistration = false; + constructor(private readonly config: VoIPUserConfiguration, mediaRenderer?: IMediaStreamRenderer) { super(); this.mediaStreamRendered = mediaRenderer; @@ -136,12 +144,16 @@ export class VoIPUser extends Emitter { this.userAgent.transport.isConnected(); this._opInProgress = Operation.OP_CONNECT; try { - this.registerer = new Registerer(this.userAgent); + this.registerer = new Registerer(this.userAgent /* , { expires: 60 }*/); + this.userAgent.transport.onConnect = this.onConnected.bind(this); this.userAgent.transport.onDisconnect = this.onDisconnected.bind(this); window.addEventListener('online', this.onlineNetworkHandler); window.addEventListener('offline', this.offlineNetworkHandler); await this.userAgent.start(); + if (this.config.enableKeepAliveUsingOptionsForUnstableNetworks) { + this.startOptionsPingForUnstableNetworks(); + } } catch (error) { this._connectionState = 'ERROR'; throw error; @@ -156,10 +168,10 @@ export class VoIPUser extends Emitter { /** * Re-registration post network recovery should be attempted * if it was previously registered or incall/onhold - * */ + */ if (this.registerer && this.callState !== 'INITIAL') { - this.attemptRegistrationPostRecovery(); + this.attemptRegistration = true; } } @@ -176,8 +188,9 @@ export class VoIPUser extends Emitter { * In case of remote side disconnection, if config.connectionRetryCount is -1, * attemptReconnection attempts continuously. Else stops after |config.connectionRetryCount| * - * */ - this.attemptReconnection(); + */ + // this.attemptReconnection(); + this.attemptReconnection(0, false); } } @@ -193,7 +206,10 @@ export class VoIPUser extends Emitter { * the code will check if the endpoint was previously registered before the disconnection. * If such is the case, it will first unregister and then reregister. * */ - this.attemptReconnection(1, true); + this.attemptReconnection(); + if (this.registerer && this.callState !== 'INITIAL') { + this.attemptRegistration = true; + } } } @@ -528,11 +544,11 @@ export class VoIPUser extends Emitter { * there is a UA using this socket. This is implemented below */ - sendOptions(): void { + sendOptions(outgoingRequestDelegate?: OutgoingRequestDelegate): void { const uri = new URI('sip', this.config.authUserName, this.config.sipRegistrarHostnameOrIP); const outgoingMessage = this.userAgent?.userAgentCore.makeOutgoingRequestMessage('OPTIONS', uri, uri, uri, {}); if (outgoingMessage) { - this.userAgent?.userAgentCore.request(outgoingMessage); + this.userAgent?.userAgentCore.request(outgoingMessage, outgoingRequestDelegate); } } /** @@ -804,7 +820,8 @@ export class VoIPUser extends Emitter { * In case of computer waking from sleep or asterisk getting restored, connect and disconnect events are generated. * In this case, re-registration should be triggered (by calling) only when onConnected gets called and not otherwise. */ - attemptReconnection(reconnectionAttempt = 0, checkRegistration = false): void { + + async attemptReconnection(reconnectionAttempt = 0, checkRegistration = false): Promise { const reconnectionAttempts = this.connectionRetryCount; this._connectionState = 'SERVER_RECONNECTING'; if (!this.userAgent) { @@ -820,6 +837,7 @@ export class VoIPUser extends Emitter { } const reconnectionDelay = Math.pow(2, reconnectionAttempt % 4); + console.error(`Attempting to reconnect with backoff due to network loss. Backoff time [${reconnectionDelay}]`); setTimeout(() => { if (this.stop) { @@ -832,10 +850,6 @@ export class VoIPUser extends Emitter { ?.reconnect() .then(() => { this._connectionState = 'SERVER_CONNECTED'; - if (!checkRegistration || !this.registerer || this.callState === 'INITIAL') { - return; - } - this.attemptRegistrationPostRecovery(); }) .catch(() => { this.attemptReconnection(++reconnectionAttempt, checkRegistration); @@ -843,7 +857,7 @@ export class VoIPUser extends Emitter { }, reconnectionDelay * 1000); } - async attemptRegistrationPostRecovery(): Promise { + async attemptPostRecoveryRoutine(): Promise { /** * It might happen that the whole network loss can happen * while there is ongoing call. In that case, we want to maintain @@ -851,7 +865,91 @@ export class VoIPUser extends Emitter { * * So after re-registration, it should remain in the same state. * */ + this.sendOptions({ + onAccept: (): void => { + this.attemptPostRecoveryRegistrationRoutine(); + }, + onReject: (error: unknown): void => { + console.error(`[${error}] Failed to do options in attemptPostRecoveryRoutine()`); + }, + }); + } + async sendKeepAliveAndWaitForResponse(withDebounce = false): Promise { + const promise = new Promise((resolve) => { + let keepAliveAccepted = false; + let responseWaitTime = this.optionsKeepaliveInterval / 2; + if (withDebounce) { + responseWaitTime += this.optionsKeepAliveDebounceTimeInSec; + } + + this.sendOptions({ + onAccept: (): void => { + keepAliveAccepted = true; + }, + onReject: (_error: unknown): void => { + console.error('Failed to do options.'); + }, + }); + setTimeout(async () => { + if (!keepAliveAccepted) { + resolve(false); + } else { + if (this.attemptRegistration) { + this.attemptPostRecoveryRoutine(); + this.attemptRegistration = false; + } + resolve(true); + } + }, responseWaitTime * 1000); + }); + return promise; + } + + async startOptionsPingForUnstableNetworks(): Promise { + setTimeout(async () => { + if (!this.userAgent || this.stop) { + return; + } + if (this._connectionState !== 'SERVER_RECONNECTING') { + let keepAliveResponse = await this.sendKeepAliveAndWaitForResponse(); + if (!keepAliveResponse) { + const connectivityArray = []; + for (let i = 0; i < this.optionsKeepAliveDebounceCount; i++) { + connectivityArray.push(this.sendKeepAliveAndWaitForResponse(true)); + } + const values = await Promise.all(connectivityArray); + for (const i in values) { + if (values[i]) { + keepAliveResponse = values[i]; + break; + } + } + if (!keepAliveResponse) { + this.networkEmitter.emit('disconnected'); + } + } + /** + * Either we got connected and managed to send keep-alive + * or while attempting keepAlive with debounce, we got connected at moment, + * |keepAliveResponse| will be turned on. + */ + if (keepAliveResponse) { + this.networkEmitter.emit('connected'); + } + } + this.startOptionsPingForUnstableNetworks(); + }, this.optionsKeepaliveInterval * 1000); + } + + async attemptPostRecoveryRegistrationRoutine(): Promise { + /** + * It might happen that the whole network loss can happen + * while there is ongoing call. In that case, we want to maintain + * the call. + * + * So after re-registration, it should remain in the same state. + * */ const promise = new Promise((_resolve, _reject) => { this.registerer?.unregister({ all: true, @@ -867,7 +965,11 @@ export class VoIPUser extends Emitter { }, }); }); - await promise; + try { + await promise; + } catch (error) { + console.error(`[${error}] While waiting for unregister promise`); + } this.registerer?.register({ requestDelegate: { onReject: (error): void => { diff --git a/apps/meteor/client/providers/CallProvider/CallProvider.tsx b/apps/meteor/client/providers/CallProvider/CallProvider.tsx index b0bb041b7fe26..37bb43509f640 100644 --- a/apps/meteor/client/providers/CallProvider/CallProvider.tsx +++ b/apps/meteor/client/providers/CallProvider/CallProvider.tsx @@ -1,5 +1,6 @@ import type { IVoipRoom, IUser } from '@rocket.chat/core-typings'; import { ICallerInfo } from '@rocket.chat/core-typings'; +import { useMutableCallback } from '@rocket.chat/fuselage-hooks'; import { useSetModal, useRoute, useUser, useSetting, useEndpoint, useStream } from '@rocket.chat/ui-contexts'; import { Random } from 'meteor/random'; import React, { useMemo, FC, useRef, useCallback, useEffect, useState } from 'react'; @@ -27,12 +28,12 @@ const stopRingback = (): void => { CustomSounds.remove('telephone'); }; +type NetworkState = 'online' | 'offline'; export const CallProvider: FC = ({ children }) => { const voipEnabled = useSetting('VoIP_Enabled'); const subscribeToNotifyUser = useStream('notify-user'); const result = useVoipClient(); - const user = useUser(); const homeRoute = useRoute('home'); @@ -49,6 +50,8 @@ export const CallProvider: FC = ({ children }) => { const [queueAggregator, setQueueAggregator] = useState(); + const [networkStatus, setNetworkStatus] = useState('online'); + useEffect(() => { if (!result?.voipClient) { return; @@ -205,6 +208,56 @@ export const CallProvider: FC = ({ children }) => { remoteAudioMediaRef.current && result.voipClient.switchMediaRenderer({ remoteMediaElement: remoteAudioMediaRef.current }); }, [result.voipClient]); + const onNetworkConnected = useMutableCallback((): void => { + if (!result.voipClient) { + return; + } + if (networkStatus === 'offline') { + setNetworkStatus('online'); + } + }); + + const onNetworkDisconnected = useMutableCallback((): void => { + if (!result.voipClient) { + return; + } + // Transitioning from online -> offline + // If there is ongoing call, terminate it or if we are processing an incoming/outgoing call + // reject it. + if (networkStatus === 'online') { + setNetworkStatus('offline'); + switch (result.voipClient.callerInfo.state) { + case 'IN_CALL': + case 'ON_HOLD': + result.voipClient?.endCall(); + break; + case 'OFFER_RECEIVED': + case 'ANSWER_SENT': + result.voipClient?.rejectCall(); + break; + } + } + }); + + useEffect(() => { + if (!result.voipClient) { + return; + } + result.voipClient.onNetworkEvent('connected', onNetworkConnected); + result.voipClient.onNetworkEvent('disconnected', onNetworkDisconnected); + result.voipClient.onNetworkEvent('connectionerror', onNetworkDisconnected); + result.voipClient.onNetworkEvent('localnetworkonline', onNetworkConnected); + result.voipClient.onNetworkEvent('localnetworkoffline', onNetworkDisconnected); + + return (): void => { + result.voipClient?.offNetworkEvent('connected', onNetworkConnected); + result.voipClient?.offNetworkEvent('disconnected', onNetworkDisconnected); + result.voipClient?.offNetworkEvent('connectionerror', onNetworkDisconnected); + result.voipClient?.offNetworkEvent('localnetworkonline', onNetworkConnected); + result.voipClient?.offNetworkEvent('localnetworkoffline', onNetworkDisconnected); + }; + }, [onNetworkConnected, onNetworkDisconnected, result.voipClient]); + const visitorEndpoint = useEndpoint('POST', '/v1/livechat/visitor'); const voipEndpoint = useEndpoint('GET', '/v1/voip/room'); const voipCloseRoomEndpoint = useEndpoint('POST', '/v1/voip/room.close'); diff --git a/apps/meteor/client/providers/CallProvider/hooks/useVoipClient.ts b/apps/meteor/client/providers/CallProvider/hooks/useVoipClient.ts index 83cc0cdf2c024..bcf7e2558f4b2 100644 --- a/apps/meteor/client/providers/CallProvider/hooks/useVoipClient.ts +++ b/apps/meteor/client/providers/CallProvider/hooks/useVoipClient.ts @@ -21,6 +21,7 @@ const isSignedResponse = (data: any): data is { result: string } => typeof data? export const useVoipClient = (): UseVoipClientResult => { const [voipEnabled, setVoipEnabled] = useSafely(useState(useSetting('VoIP_Enabled'))); const voipRetryCount = useSetting('VoIP_Retry_Count'); + const enableKeepAlive = useSetting('VoIP_Enable_Keep_Alive_For_Unstable_Networks'); const registrationInfo = useEndpoint('GET', '/v1/connector.extension.getRegistrationInfoByUserId'); const membership = useEndpoint('GET', '/v1/voip/queues.getMembershipSubscription'); const user = useUser(); @@ -63,7 +64,16 @@ export const useVoipClient = (): UseVoipClientResult => { (async (): Promise => { try { const subscription = await membership({ extension }); - client = await SimpleVoipUser.create(extension, password, host, websocketPath, iceServers, Number(voipRetryCount), 'video'); + client = await SimpleVoipUser.create( + extension, + password, + host, + websocketPath, + iceServers, + Number(voipRetryCount), + Boolean(enableKeepAlive), + 'video', + ); // Today we are hardcoding workflow mode. // In future, this should be ready from configuration client.setWorkflowMode(WorkflowTypes.CONTACT_CENTER_USER); @@ -83,7 +93,7 @@ export const useVoipClient = (): UseVoipClientResult => { client.clear(); } }; - }, [iceServers, registrationInfo, setResult, membership, voipEnabled, user?._id, user?.extension, voipRetryCount]); + }, [iceServers, registrationInfo, setResult, membership, voipEnabled, user?._id, user?.extension, voipRetryCount, enableKeepAlive]); return result; }; diff --git a/apps/meteor/packages/rocketchat-i18n/i18n/en.i18n.json b/apps/meteor/packages/rocketchat-i18n/i18n/en.i18n.json index 2344fa6322504..23343234f595c 100644 --- a/apps/meteor/packages/rocketchat-i18n/i18n/en.i18n.json +++ b/apps/meteor/packages/rocketchat-i18n/i18n/en.i18n.json @@ -4791,6 +4791,8 @@ "Visitor_page_URL": "Visitor page URL", "Visitor_time_on_site": "Visitor time on site", "Voice_Call": "Voice Call", + "VoIP_Enable_Keep_Alive_For_Unstable_Networks": "Enable Keep-Alive using SIP-OPTIONS for unstable networks", + "VoIP_Enable_Keep_Alive_For_Unstable_Networks_Description": "Enables or Disables Keep-Alive using SIP-OPTIONS based on network quality", "VoIP_Enabled": "VoIP Enabled", "VoIP_Extension": "VoIP Extension", "Voip_Server_Configuration": "Server Configuration", diff --git a/packages/core-typings/src/voip/VoIPUserConfiguration.ts b/packages/core-typings/src/voip/VoIPUserConfiguration.ts index 9ca379b4546dc..48f9d55cc6f2a 100644 --- a/packages/core-typings/src/voip/VoIPUserConfiguration.ts +++ b/packages/core-typings/src/voip/VoIPUserConfiguration.ts @@ -36,10 +36,15 @@ export interface VoIPUserConfiguration { */ iceServers: Array; /** - * Voip Retru count + * Voip Retry count * @defaultValue undefined */ connectionRetryCount: number; + /** + * Voip Retry count + * @defaultValue undefined + */ + enableKeepAliveUsingOptionsForUnstableNetworks: boolean; } export interface IMediaStreamRenderer {