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.

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

fix(plugin-meetings): remove all llm events added by meetings when meeting is cleaned up #4072

Open
wants to merge 8 commits into
base: next
Choose a base branch
from
16 changes: 16 additions & 0 deletions packages/@webex/plugin-meetings/src/annotation/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,22 @@ class AnnotationChannel extends WebexPlugin implements IAnnotationChannel {
}
}

/**
* Remove event listeners
* @returns {undefined}
*/
public deregisterEvents() {
if (this.hasSubscribedToEvents) {
// @ts-ignore
this.webex.internal.mercury.off('event:locus.approval_request', this.eventCommandProcessor);

// @ts-ignore
this.webex.internal.llm.off('event:relay.event', this.eventDataProcessor);

this.hasSubscribedToEvents = false;
}
}

/**
* set locusUrl
* @param {string} locusUrl
Expand Down
19 changes: 18 additions & 1 deletion packages/@webex/plugin-meetings/src/meeting/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5295,7 +5295,16 @@ export default class Meeting extends StatelessWebexPlugin {
(this.config.receiveReactions || options.receiveReactions) &&
this.isReactionsSupported()
) {
const {name} = this.members.membersCollection.get(e.data.sender.participantId);
const member = this.members.membersCollection.get(e.data.sender.participantId);
if (!member) {
// @ts-ignore -- fix type
LoggerProxy.logger.warn(
`Meeting:index#processRelayEvent --> Skipping handling of ${REACTION_RELAY_TYPES.REACTION} for ${this.id}. participantId ${e.data.sender.participantId} does not exist in membersCollection.`
);
break;
}

const {name} = member;
const processedReaction: ProcessedReaction = {
reaction: e.data.reaction,
sender: {
Expand Down Expand Up @@ -5349,6 +5358,9 @@ export default class Meeting extends StatelessWebexPlugin {
this.voiceaListenerCallbacks[VOICEAEVENTS.NEW_CAPTION]
);

// @ts-ignore
this.webex.internal.voicea.deregisterEvents();

this.areVoiceaEventsSetup = false;
this.triggerStopReceivingTranscriptionEvent();
}
Expand Down Expand Up @@ -8699,6 +8711,11 @@ export default class Meeting extends StatelessWebexPlugin {
this.stopTranscription();
this.transcription = undefined;
}

this.annotation.deregisterEvents();

// @ts-ignore - fix types
this.webex.internal.llm.off('event:relay.event', this.processRelayEvent);
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,51 @@ describe('live-annotation', () => {
});
});
});
});

describe('#deregisterEvents', () => {
let llmOn;
let llmOff;
let mercuryOn;
let mercuryOff;

beforeEach(() => {
llmOn = sinon.spy(webex.internal.llm, 'on');
llmOff = sinon.spy(webex.internal.llm, 'off');
mercuryOn = sinon.spy(webex.internal.mercury, 'on');
mercuryOff = sinon.spy(webex.internal.mercury, 'off');
});

it('cleans up events', () => {
annotationService.locusUrlUpdate(locusUrl);
assert.calledWith(
mercuryOn,
'event:locus.approval_request',
annotationService.eventCommandProcessor,
annotationService
);
assert.calledWith(
llmOn,
'event:relay.event',
annotationService.eventDataProcessor,
annotationService
);
assert.match(annotationService.hasSubscribedToEvents, true);

annotationService.deregisterEvents();
assert.calledWith(llmOff, 'event:relay.event', annotationService.eventDataProcessor);
assert.calledWith(
mercuryOff,
'event:locus.approval_request',
annotationService.eventCommandProcessor
);
assert.match(annotationService.hasSubscribedToEvents, false);
});

it('does not call llm off if events have not been registered', () => {
annotationService.deregisterEvents();
assert.notCalled(llmOff);
assert.notCalled(mercuryOff);
});
});
});
});
64 changes: 64 additions & 0 deletions packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1238,6 +1238,7 @@ describe('plugin-meetings', () => {
webex.internal.voicea.off = sinon.stub();
webex.internal.voicea.listenToEvents = sinon.stub();
webex.internal.voicea.turnOnCaptions = sinon.stub();
webex.internal.voicea.deregisterEvents = sinon.stub();
});

it('should stop listening to voicea events and also trigger a stop event', () => {
Expand Down Expand Up @@ -1566,6 +1567,55 @@ describe('plugin-meetings', () => {
fakeProcessedReaction
);
});

it('should fail quietly if participantId does not exist in membersCollection', () => {
LoggerProxy.logger.warn = sinon.stub();
meeting.isReactionsSupported = sinon.stub().returns(true);
meeting.config.receiveReactions = true;
const fakeSendersName = 'Fake reactors name';
const fakeReactionPayload = {
type: 'fake_type',
codepoints: 'fake_codepoints',
shortcodes: 'fake_shortcodes',
tone: {
type: 'fake_tone_type',
codepoints: 'fake_tone_codepoints',
shortcodes: 'fake_tone_shortcodes',
},
};
const fakeSenderPayload = {
participantId: 'fake_participant_id',
};
const fakeProcessedReaction = {
reaction: fakeReactionPayload,
sender: {
id: fakeSenderPayload.participantId,
name: fakeSendersName,
},
};
const fakeRelayEvent = {
data: {
relayType: REACTION_RELAY_TYPES.REACTION,
reaction: fakeReactionPayload,
sender: fakeSenderPayload,
},
};
meeting.processRelayEvent(fakeRelayEvent);
assert.calledWith(
LoggerProxy.logger.warn,
`Meeting:index#processRelayEvent --> Skipping handling of react for ${meeting.id}. participantId fake_participant_id does not exist in membersCollection.`
);
assert.neverCalledWith(
TriggerProxy.trigger,
sinon.match.instanceOf(Meeting),
{
file: 'meeting/index',
function: 'join',
},
EVENT_TRIGGERS.MEETING_RECEIVE_REACTIONS,
fakeProcessedReaction
);
});
});

describe('#handleLLMOnline', () => {
Expand Down Expand Up @@ -5055,6 +5105,11 @@ describe('plugin-meetings', () => {
meeting.logger.error = sinon.stub().returns(true);
meeting.updateLLMConnection = sinon.stub().returns(Promise.resolve());
webex.internal.voicea.off = sinon.stub().returns(true);
meeting.stopTranscription = sinon.stub();
meeting.transcription = {};

meeting.annotation.deregisterEvents = sinon.stub();
webex.internal.llm.off = sinon.stub();

// A meeting needs to be joined to leave
meeting.meetingState = 'ACTIVE';
Expand All @@ -5075,6 +5130,9 @@ describe('plugin-meetings', () => {
assert.calledOnce(meeting.closePeerConnections);
assert.calledOnce(meeting.unsetRemoteStreams);
assert.calledOnce(meeting.unsetPeerConnections);
assert.calledOnce(meeting.stopTranscription);
assert.calledOnce(meeting.annotation.deregisterEvents);
assert.calledWith(webex.internal.llm.off, 'event:relay.event', meeting.processRelayEvent);
});

it('should reset call diagnostic latencies correctly', async () => {
Expand Down Expand Up @@ -6957,6 +7015,9 @@ describe('plugin-meetings', () => {
meeting.transcription = {};
meeting.stopTranscription = sinon.stub();

meeting.annotation.deregisterEvents = sinon.stub();
webex.internal.llm.off = sinon.stub();

// A meeting needs to be joined to end
meeting.meetingState = 'ACTIVE';
meeting.state = 'JOINED';
Expand All @@ -6977,6 +7038,9 @@ describe('plugin-meetings', () => {
assert.calledOnce(meeting?.unsetRemoteStreams);
assert.calledOnce(meeting?.unsetPeerConnections);
assert.calledOnce(meeting?.stopTranscription);

assert.called(meeting.annotation.deregisterEvents);
assert.calledWith(webex.internal.llm.off, 'event:relay.event', meeting.processRelayEvent);
});
});

Expand Down
Loading