Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Regression: [VideoConference] Callee client behaves improperly when accepting a call from someone who lost the connection #26101

Merged
merged 3 commits into from
Jul 4, 2022
Merged

Regression: [VideoConference] Callee client behaves improperly when accepting a call from someone who lost the connection #26101

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
79b3a39
[IMPROVE] Changed the way the callee client handles call acceptance.
pierre-lehnen-rc Jul 1, 2022
9165686
Merge remote-tracking branch 'origin/develop' into improve/handling-o…
pierre-lehnen-rc Jul 1, 2022
8319e01
Merge remote-tracking branch 'origin/develop' into improve/handling-o…
pierre-lehnen-rc Jul 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 64 additions & 52 deletions apps/meteor/client/lib/VideoConfManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export type DirectCallParams = {
rid: IRoom['_id'];
callId: string;
dismissed?: boolean;
acceptTimeout?: ReturnType<typeof setTimeout> | undefined;
// TODO: improve this, nowadays there is not possible check if the video call has finished, but ist a nice improvement
// state: 'incoming' | 'outgoing' | 'connected' | 'disconnected' | 'dismissed';
};
Expand Down Expand Up @@ -99,10 +100,6 @@ export const VideoConfManager = new (class VideoConfManager extends Emitter<Vide

private incomingDirectCalls: Map<string, IncomingDirectCall>;

private acceptingCallId: string | undefined;

private acceptingCallTimeout = 0;

private _preferences: CallPreferences;

private _capabilities: ProviderCapabilities;
Expand Down Expand Up @@ -143,7 +140,7 @@ export const VideoConfManager = new (class VideoConfManager extends Emitter<Vide
}

public getIncomingDirectCalls(): DirectCallParams[] {
return [...this.incomingDirectCalls.values()].map(({ timeout: _, ...call }) => ({ ...call }));
return [...this.incomingDirectCalls.values()].map(({ timeout: _, ...call }) => ({ ...call })).filter((call) => !call.acceptTimeout);
}

public async startCall(roomId: IRoom['_id'], title?: string): Promise<void> {
Expand Down Expand Up @@ -186,31 +183,37 @@ export const VideoConfManager = new (class VideoConfManager extends Emitter<Vide
if (!callData) {
throw new Error('Unable to find accepted call information.');
}
if (callData.acceptTimeout) {
debug && console.log(`[VideoConf] We're already trying to accept call ${callId}.`);
return;
}

debug && console.log(`[VideoConf] Accepting incoming call.`);
debug && console.log(`[VideoConf] Accepting incoming call ${callId}.`);

if (callData.timeout) {
clearTimeout(callData.timeout);
this.setIncomingCallAttribute(callId, 'timeout', undefined);
}

// Mute this call Id so any lingering notifications don't trigger it again
this.dismissIncomingCall(callId);

this.acceptingCallId = callId;
this.acceptingCallTimeout = setTimeout(() => {
if (this.acceptingCallId !== callId) {
debug && console.warn(`[VideoConf] Accepting call timeout not properly cleared.`);
return;
}

debug && console.log(`[VideoConf] Attempt to accept call has timed out.`);
this.acceptingCallId = undefined;
this.acceptingCallTimeout = 0;

this.removeIncomingCall(callId);

this.emit('direct/failed', { callId, uid: callData.uid, rid: callData.rid });
}, ACCEPT_TIMEOUT) as unknown as number;
this.setIncomingCallAttribute(
callId,
'acceptTimeout',
setTimeout(() => {
const updatedCallData = this.incomingDirectCalls.get(callId);
if (!updatedCallData?.acceptTimeout) {
return;
}

debug && console.log(`[VideoConf] Attempt to accept call has timed out.`);
this.removeIncomingCall(callId);

this.emit('direct/failed', { callId, uid: callData.uid, rid: callData.rid });
}, ACCEPT_TIMEOUT),
);
this.emit('incoming/changed');

debug && console.log(`[VideoConf] Notifying user ${callData.uid} that we accept their call.`);
Notifications.notifyUser(callData.uid, 'video-conference.accepted', { callId, uid: this.userId, rid: callData.rid });
Expand Down Expand Up @@ -247,23 +250,37 @@ export const VideoConfManager = new (class VideoConfManager extends Emitter<Vide
this.emit('capabilities/changed');
}

private setIncomingCallAttribute<T extends keyof IncomingDirectCall>(
callId: string,
attributeName: T,
value: IncomingDirectCall[T] | undefined,
): void {
const callData = this.incomingDirectCalls.get(callId);
if (!callData) {
return;
}

const newData: IncomingDirectCall = {
...callData,
};

if (value === undefined) {
delete newData[attributeName];
} else {
newData[attributeName] = value;
}

this.incomingDirectCalls.set(callId, newData);
}

private dismissedIncomingCallHelper(callId: string): boolean {
// Muting will stop a callId from ringing, but it doesn't affect any part of the existing workflow
const callData = this.incomingDirectCalls.get(callId);
if (!callData) {
return false;
}
this.incomingDirectCalls.set(callId, { ...callData, dismissed: true });
setTimeout(() => {
const callData = this.incomingDirectCalls.get(callId);

if (!callData) {
return;
}
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { dismissed, ...rest } = callData;
this.incomingDirectCalls.set(callId, { ...rest });
}, CALL_TIMEOUT * 20);
this.setIncomingCallAttribute(callId, 'dismissed', true);
setTimeout(() => this.setIncomingCallAttribute(callId, 'dismissed', undefined), CALL_TIMEOUT * 20);
return true;
}

Expand Down Expand Up @@ -314,13 +331,12 @@ export const VideoConfManager = new (class VideoConfManager extends Emitter<Vide
public async joinCall(callId: string): Promise<void> {
debug && console.log(`[VideoConf] Joining call ${callId}.`);

if (this.acceptingCallTimeout && this.acceptingCallId === callId) {
clearTimeout(this.acceptingCallTimeout);
this.acceptingCallTimeout = 0;
this.acceptingCallId = undefined;
}

if (this.incomingDirectCalls.has(callId)) {
const data = this.incomingDirectCalls.get(callId);
if (data?.acceptTimeout) {
debug && console.log('[VideoConf] Clearing acceptance timeout');
clearTimeout(data.acceptTimeout);
}
this.removeIncomingCall(callId);
}

Expand Down Expand Up @@ -429,19 +445,16 @@ export const VideoConfManager = new (class VideoConfManager extends Emitter<Vide
this.currentCallHandler = undefined;
}

if (this.acceptingCallTimeout) {
clearTimeout(this.acceptingCallTimeout);
this.acceptingCallTimeout = 0;
}

this.incomingDirectCalls.forEach((call) => {
if (call.timeout) {
clearTimeout(call.timeout);
}
if (call.acceptTimeout) {
clearTimeout(call.acceptTimeout);
}
});
this.incomingDirectCalls.clear();
this.currentCallData = undefined;
this.acceptingCallId = undefined;
this._preferences = {};
this.emit('incoming/changed');
this.emit('ringing/changed');
Expand All @@ -466,7 +479,7 @@ export const VideoConfManager = new (class VideoConfManager extends Emitter<Vide

private abortIncomingCall(callId: string): void {
// If we just accepted this call, then ignore the timeout
if (this.acceptingCallId === callId) {
if (this.incomingDirectCalls.get(callId)?.acceptTimeout) {
return;
}

Expand Down Expand Up @@ -552,7 +565,7 @@ export const VideoConfManager = new (class VideoConfManager extends Emitter<Vide

private onDirectCall({ callId, uid, rid }: DirectCallParams): void {
// If we already accepted this call, then don't ring again
if (this.acceptingCallId === callId) {
if (this.incomingDirectCalls.get(callId)?.acceptTimeout) {
return;
}

Expand All @@ -568,11 +581,10 @@ export const VideoConfManager = new (class VideoConfManager extends Emitter<Vide
debug && console.log(`[VideoConf] Call ${callId} was canceled by the remote user.`);

// We had just accepted this call, but the remote user hang up before they got the notification, so cancel our acceptance
if (this.acceptingCallId === callId) {
if (this.acceptingCallTimeout) {
clearTimeout(this.acceptingCallTimeout);
}
this.acceptingCallTimeout = 0;
const callData = this.incomingDirectCalls.get(callId);
if (callData?.acceptTimeout) {
clearTimeout(callData.acceptTimeout);
this.setIncomingCallAttribute(callId, 'acceptTimeout', undefined);
}

this.loseIncomingCall(callId);
Expand Down Expand Up @@ -612,7 +624,7 @@ export const VideoConfManager = new (class VideoConfManager extends Emitter<Vide
}

private onDirectCallConfirmed(params: DirectCallParams): void {
if (!params.callId || params.callId !== this.acceptingCallId) {
if (!params.callId || !this.incomingDirectCalls.get(params.callId)?.acceptTimeout) {
debug && console.log(`[VideoConf] User ${params.uid} confirmed we can join ${params.callId} but we aren't trying to join it.`);
return;
}
Expand Down