Skip to content

Commit

Permalink
[FIX] Fixing Network connectivity issues with SIP client. (#25391)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
amolghode1981 authored and ggazzo committed May 24, 2022
1 parent c7f3090 commit 5090a36
Show file tree
Hide file tree
Showing 7 changed files with 203 additions and 20 deletions.
9 changes: 9 additions & 0 deletions apps/meteor/app/lib/server/startup/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
2 changes: 2 additions & 0 deletions apps/meteor/client/lib/voip/SimpleVoipUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export class SimpleVoipUser {
webSocketPath: string,
iceServers: Array<object>,
voipRetryCount: number,
enableKeepAliveForUnstableNetworks: boolean,
callType?: 'audio' | 'video',
mediaStreamRendered?: IMediaStreamRenderer,
): Promise<VoIPUser> {
Expand All @@ -21,6 +22,7 @@ export class SimpleVoipUser {
enableVideo: callType === 'video',
iceServers,
connectionRetryCount: voipRetryCount,
enableKeepAliveUsingOptionsForUnstableNetworks: enableKeepAliveForUnstableNetworks,
};

return VoIPUser.create(config, mediaStreamRendered);
Expand Down
134 changes: 118 additions & 16 deletions apps/meteor/client/lib/voip/VoIPUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -83,6 +83,14 @@ export class VoIPUser extends Emitter<VoipEvents> {

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;
Expand Down Expand Up @@ -136,12 +144,16 @@ export class VoIPUser extends Emitter<VoipEvents> {
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;
Expand All @@ -156,10 +168,10 @@ export class VoIPUser extends Emitter<VoipEvents> {
/**
* 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;
}
}

Expand All @@ -176,8 +188,9 @@ export class VoIPUser extends Emitter<VoipEvents> {
* 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);
}
}

Expand All @@ -193,7 +206,10 @@ export class VoIPUser extends Emitter<VoipEvents> {
* 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;
}
}
}

Expand Down Expand Up @@ -528,11 +544,11 @@ export class VoIPUser extends Emitter<VoipEvents> {
* 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);
}
}
/**
Expand Down Expand Up @@ -804,7 +820,8 @@ export class VoIPUser extends Emitter<VoipEvents> {
* 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<void> {
const reconnectionAttempts = this.connectionRetryCount;
this._connectionState = 'SERVER_RECONNECTING';
if (!this.userAgent) {
Expand All @@ -820,6 +837,7 @@ export class VoIPUser extends Emitter<VoipEvents> {
}

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) {
Expand All @@ -832,26 +850,106 @@ export class VoIPUser extends Emitter<VoipEvents> {
?.reconnect()
.then(() => {
this._connectionState = 'SERVER_CONNECTED';
if (!checkRegistration || !this.registerer || this.callState === 'INITIAL') {
return;
}
this.attemptRegistrationPostRecovery();
})
.catch(() => {
this.attemptReconnection(++reconnectionAttempt, checkRegistration);
});
}, reconnectionDelay * 1000);
}

async attemptRegistrationPostRecovery(): Promise<void> {
async attemptPostRecoveryRoutine(): Promise<void> {
/**
* 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.
* */
this.sendOptions({
onAccept: (): void => {
this.attemptPostRecoveryRegistrationRoutine();
},
onReject: (error: unknown): void => {
console.error(`[${error}] Failed to do options in attemptPostRecoveryRoutine()`);
},
});
}

async sendKeepAliveAndWaitForResponse(withDebounce = false): Promise<boolean> {
const promise = new Promise<boolean>((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<void> {
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<void> {
/**
* 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<void>((_resolve, _reject) => {
this.registerer?.unregister({
all: true,
Expand All @@ -867,7 +965,11 @@ export class VoIPUser extends Emitter<VoipEvents> {
},
});
});
await promise;
try {
await promise;
} catch (error) {
console.error(`[${error}] While waiting for unregister promise`);
}
this.registerer?.register({
requestDelegate: {
onReject: (error): void => {
Expand Down
55 changes: 54 additions & 1 deletion apps/meteor/client/providers/CallProvider/CallProvider.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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');

Expand All @@ -49,6 +50,8 @@ export const CallProvider: FC = ({ children }) => {

const [queueAggregator, setQueueAggregator] = useState<QueueAggregator>();

const [networkStatus, setNetworkStatus] = useState<NetworkState>('online');

useEffect(() => {
if (!result?.voipClient) {
return;
Expand Down Expand Up @@ -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');
Expand Down
14 changes: 12 additions & 2 deletions apps/meteor/client/providers/CallProvider/hooks/useVoipClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -63,7 +64,16 @@ export const useVoipClient = (): UseVoipClientResult => {
(async (): Promise<void> => {
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);
Expand All @@ -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;
};
2 changes: 2 additions & 0 deletions apps/meteor/packages/rocketchat-i18n/i18n/en.i18n.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading

0 comments on commit 5090a36

Please sign in to comment.