Skip to content

Commit 81e42b9

Browse files
authored
Add probablyLeft event to the MatrixRTCSession (#4962)
* Add probablyLeft emission to the MatrixRTCSession Signed-off-by: Timo K <[email protected]> * add docstring Signed-off-by: Timo K <[email protected]> * Review: add additional test + fix pending promises issue. Signed-off-by: Timo K <[email protected]> * review: `Pick` only a subset of membership manager events Signed-off-by: Timo K <[email protected]> * reveiw: update probablyLeft logic to be more straight forward Signed-off-by: Timo K <[email protected]> * fix test Signed-off-by: Timo K <[email protected]> * make test not wait for 5s Signed-off-by: Timo K <[email protected]> * review Signed-off-by: Timo K <[email protected]> * fix linter (rebase) Signed-off-by: Timo K <[email protected]> * fix import Signed-off-by: Timo K <[email protected]> --------- Signed-off-by: Timo K <[email protected]>
1 parent 7f7ecd0 commit 81e42b9

File tree

4 files changed

+133
-15
lines changed

4 files changed

+133
-15
lines changed

spec/unit/matrixrtc/MembershipManager.spec.ts

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,14 @@ limitations under the License.
1616

1717
import { type MockedFunction, type Mock } from "jest-mock";
1818

19-
import { EventType, HTTPError, MatrixError, UnsupportedDelayedEventsEndpointError, type Room } from "../../../src";
19+
import {
20+
type EmptyObject,
21+
EventType,
22+
HTTPError,
23+
MatrixError,
24+
UnsupportedDelayedEventsEndpointError,
25+
type Room,
26+
} from "../../../src";
2027
import {
2128
MembershipManagerEvent,
2229
Status,
@@ -611,6 +618,7 @@ describe("MembershipManager", () => {
611618
await testExpires(10_000, 1_000);
612619
});
613620
});
621+
614622
describe("status updates", () => {
615623
it("starts 'Disconnected'", () => {
616624
const manager = new MembershipManager({}, room, client, () => undefined, callSession);
@@ -836,6 +844,63 @@ describe("MembershipManager", () => {
836844
expect(client.sendStateEvent).toHaveBeenCalled();
837845
});
838846
});
847+
describe("probablyLeft", () => {
848+
it("emits probablyLeft when the membership manager could not hear back from the server for the duration of the delayed event", async () => {
849+
const manager = new MembershipManager(
850+
{ delayedLeaveEventDelayMs: 10000 },
851+
room,
852+
client,
853+
() => undefined,
854+
callSession,
855+
);
856+
const { promise: stuckPromise, reject: rejectStuckPromise } = Promise.withResolvers<EmptyObject>();
857+
const probablyLeftEmit = jest.fn();
858+
manager.on(MembershipManagerEvent.ProbablyLeft, probablyLeftEmit);
859+
manager.join([focus], focusActive);
860+
try {
861+
// Let the scheduler run one iteration so that we can send the join state event
862+
await waitForMockCall(client._unstable_updateDelayedEvent);
863+
864+
// We never resolve the delayed event so that we can test the probablyLeft event.
865+
// This simulates the case where the server does not respond to the delayed event.
866+
client._unstable_updateDelayedEvent = jest.fn(() => stuckPromise);
867+
expect(client.sendStateEvent).toHaveBeenCalledTimes(1);
868+
expect(manager.status).toBe(Status.Connected);
869+
expect(probablyLeftEmit).not.toHaveBeenCalledWith(true);
870+
// We expect the probablyLeft event to be emitted after the `delayedLeaveEventDelayMs` = 10000.
871+
// We also track the calls to updated the delayed event that all will never resolve to simulate the server not responding.
872+
// The numbers are a bit arbitrary since we use the local timeout that does not perfectly match the 5s check interval in this test.
873+
await jest.advanceTimersByTimeAsync(5000);
874+
// No emission after 5s
875+
expect(probablyLeftEmit).not.toHaveBeenCalledWith(true);
876+
expect(client._unstable_updateDelayedEvent).toHaveBeenCalledTimes(1);
877+
878+
await jest.advanceTimersByTimeAsync(4999);
879+
expect(client._unstable_updateDelayedEvent).toHaveBeenCalledTimes(3);
880+
expect(probablyLeftEmit).not.toHaveBeenCalledWith(true);
881+
882+
// Reset mocks before we setup the next delayed event restart by advancing the timers 1 more ms.
883+
(client._unstable_updateDelayedEvent as Mock<any>).mockResolvedValue({});
884+
885+
// Emit after 10s
886+
await jest.advanceTimersByTimeAsync(1);
887+
expect(client._unstable_updateDelayedEvent).toHaveBeenCalledTimes(4);
888+
expect(probablyLeftEmit).toHaveBeenCalledWith(true);
889+
890+
// Mock a sync which does not include our own membership
891+
await manager.onRTCSessionMemberUpdate([]);
892+
// Wait for the current ongoing delayed event sending to finish
893+
await jest.advanceTimersByTimeAsync(1);
894+
// We should send a new state event and an associated delayed leave event.
895+
expect(client._unstable_sendDelayedStateEvent).toHaveBeenCalledTimes(2);
896+
expect(client.sendStateEvent).toHaveBeenCalledTimes(2);
897+
// At the same time we expect the probablyLeft event to be emitted with false so we are back operational.
898+
expect(probablyLeftEmit).toHaveBeenCalledWith(false);
899+
} finally {
900+
rejectStuckPromise();
901+
}
902+
});
903+
});
839904
});
840905

841906
it("Should prefix log with MembershipManager used", () => {

src/matrixrtc/IMembershipManager.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,21 @@ limitations under the License.
1717
import type { CallMembership } from "./CallMembership.ts";
1818
import type { Focus } from "./focus.ts";
1919
import type { Status } from "./types.ts";
20+
import { type TypedEventEmitter } from "../models/typed-event-emitter.ts";
2021

2122
export enum MembershipManagerEvent {
2223
StatusChanged = "StatusChanged",
24+
/**
25+
* Emitted when the membership manager has not heard back from the server for the duration
26+
* of the delayed event and hence failed to restart the delayed event.
27+
* This means that the user is probably not joined anymore and the leave event was distributed to other session members.
28+
*/
29+
ProbablyLeft = "ProbablyLeft",
2330
}
2431

2532
export type MembershipManagerEventHandlerMap = {
2633
[MembershipManagerEvent.StatusChanged]: (prefStatus: Status, newStatus: Status) => void;
34+
[MembershipManagerEvent.ProbablyLeft]: (probablyLeft: boolean) => void;
2735
};
2836

2937
/**
@@ -33,7 +41,8 @@ export type MembershipManagerEventHandlerMap = {
3341
*
3442
* @internal
3543
*/
36-
export interface IMembershipManager {
44+
export interface IMembershipManager
45+
extends TypedEventEmitter<MembershipManagerEvent, MembershipManagerEventHandlerMap> {
3746
/**
3847
* If we are trying to join, or have successfully joined the session.
3948
* It does not reflect if the room state is already configured to represent us being joined.
@@ -85,8 +94,4 @@ export interface IMembershipManager {
8594
* @returns the used active focus in the currently joined session or undefined if not joined.
8695
*/
8796
getActiveFocus(): Focus | undefined;
88-
89-
// TypedEventEmitter methods:
90-
on(event: MembershipManagerEvent.StatusChanged, listener: (oldStatus: Status, newStatus: Status) => void): this;
91-
off(event: MembershipManagerEvent.StatusChanged, listener: (oldStatus: Status, newStatus: Status) => void): this;
9297
}

src/matrixrtc/MatrixRTCSession.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,11 @@ import { EncryptionManager, type IEncryptionManager } from "./EncryptionManager.
2929
import { deepCompare, logDurationSync } from "../utils.ts";
3030
import { type Statistics, type RTCNotificationType } from "./types.ts";
3131
import { RoomKeyTransport } from "./RoomKeyTransport.ts";
32-
import type { IMembershipManager } from "./IMembershipManager.ts";
32+
import {
33+
MembershipManagerEvent,
34+
type MembershipManagerEventHandlerMap,
35+
type IMembershipManager,
36+
} from "./IMembershipManager.ts";
3337
import { RTCEncryptionManager } from "./RTCEncryptionManager.ts";
3438
import {
3539
RoomAndToDeviceEvents,
@@ -220,8 +224,10 @@ export type JoinSessionConfig = SessionConfig & MembershipConfig & EncryptionCon
220224
* This class doesn't deal with media at all, just membership & properties of a session.
221225
*/
222226
export class MatrixRTCSession extends TypedEventEmitter<
223-
MatrixRTCSessionEvent | RoomAndToDeviceEvents,
224-
MatrixRTCSessionEventHandlerMap & RoomAndToDeviceEventsHandlerMap
227+
MatrixRTCSessionEvent | RoomAndToDeviceEvents | MembershipManagerEvent.ProbablyLeft,
228+
MatrixRTCSessionEventHandlerMap &
229+
RoomAndToDeviceEventsHandlerMap &
230+
Pick<MembershipManagerEventHandlerMap, MembershipManagerEvent.ProbablyLeft>
225231
> {
226232
private membershipManager?: IMembershipManager;
227233
private encryptionManager?: IEncryptionManager;
@@ -456,8 +462,8 @@ export class MatrixRTCSession extends TypedEventEmitter<
456462
roomState?.off(RoomStateEvent.Members, this.onRoomMemberUpdate);
457463
}
458464
private reEmitter = new TypedReEmitter<
459-
MatrixRTCSessionEvent | RoomAndToDeviceEvents,
460-
MatrixRTCSessionEventHandlerMap & RoomAndToDeviceEventsHandlerMap
465+
MatrixRTCSessionEvent | RoomAndToDeviceEvents | MembershipManagerEvent,
466+
MatrixRTCSessionEventHandlerMap & RoomAndToDeviceEventsHandlerMap & MembershipManagerEventHandlerMap
461467
>(this);
462468

463469
/**
@@ -490,6 +496,7 @@ export class MatrixRTCSession extends TypedEventEmitter<
490496
this.logger,
491497
);
492498

499+
this.reEmitter.reEmit(this.membershipManager!, [MembershipManagerEvent.ProbablyLeft]);
493500
// Create Encryption manager
494501
let transport;
495502
if (joinConfig?.useExperimentalToDeviceTransport) {

src/matrixrtc/MembershipManager.ts

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,12 @@ export interface MembershipManagerState {
129129
rateLimitRetries: Map<MembershipActionType, number>;
130130
/** Retry counter for other errors */
131131
networkErrorRetries: Map<MembershipActionType, number>;
132+
/** The time at which we expect the server to send the delayed leave event. */
133+
expectedServerDelayLeaveTs?: number;
134+
/** This is used to track if the client expects the scheduled delayed leave event to have
135+
* been sent because restarting failed during the available time.
136+
* Once we resend the delayed event or successfully restarted it will get unset. */
137+
probablyLeft: boolean;
132138
}
133139

134140
/**
@@ -343,6 +349,7 @@ export class MembershipManager
343349
rateLimitRetries: new Map(),
344350
networkErrorRetries: new Map(),
345351
expireUpdateIterations: 1,
352+
probablyLeft: false,
346353
};
347354
}
348355
// Membership Event static parameters:
@@ -466,6 +473,8 @@ export class MembershipManager
466473
this.stateKey,
467474
)
468475
.then((response) => {
476+
this.state.expectedServerDelayLeaveTs = Date.now() + this.delayedLeaveEventDelayMs;
477+
this.setAndEmitProbablyLeft(false);
469478
// On success we reset retries and set delayId.
470479
this.resetRateLimitCounter(MembershipActionType.SendDelayedEvent);
471480
this.state.delayId = response.delay_id;
@@ -545,27 +554,58 @@ export class MembershipManager
545554
});
546555
}
547556

557+
private setAndEmitProbablyLeft(probablyLeft: boolean): void {
558+
if (this.state.probablyLeft === probablyLeft) {
559+
return;
560+
}
561+
this.state.probablyLeft = probablyLeft;
562+
this.emit(MembershipManagerEvent.ProbablyLeft, this.state.probablyLeft);
563+
}
564+
548565
private async restartDelayedEvent(delayId: string): Promise<ActionUpdate> {
566+
// Compute the duration until we expect the server to send the delayed leave event.
567+
const durationUntilServerDelayedLeave = this.state.expectedServerDelayLeaveTs
568+
? this.state.expectedServerDelayLeaveTs - Date.now()
569+
: undefined;
549570
const abortPromise = new Promise((_, reject) => {
550-
setTimeout(() => {
551-
reject(new AbortError("Restart delayed event timed out before the HS responded"));
552-
}, this.delayedLeaveEventRestartLocalTimeoutMs);
571+
setTimeout(
572+
() => {
573+
reject(new AbortError("Restart delayed event timed out before the HS responded"));
574+
},
575+
// We abort immediately at the time where we expect the server to send the delayed leave event.
576+
// At this point we want the catch block to run and set the `probablyLeft` state.
577+
//
578+
// While we are already in probablyLeft state, we use the unaltered delayedLeaveEventRestartLocalTimeoutMs.
579+
durationUntilServerDelayedLeave !== undefined && !this.state.probablyLeft
580+
? Math.min(this.delayedLeaveEventRestartLocalTimeoutMs, durationUntilServerDelayedLeave)
581+
: this.delayedLeaveEventRestartLocalTimeoutMs,
582+
);
553583
});
554584

555585
// The obvious choice here would be to use the `IRequestOpts` to set the timeout. Since this call might be forwarded
556-
// to the widget driver this information would ge lost. That is why we mimic the AbortError using the race.
586+
// to the widget driver this information would get lost. That is why we mimic the AbortError using the race.
557587
return await Promise.race([
558588
this.client._unstable_updateDelayedEvent(delayId, UpdateDelayedEventAction.Restart),
559589
abortPromise,
560590
])
561591
.then(() => {
592+
// Whenever we successfully restart the delayed event we update the `state.expectedServerDelayLeaveTs`
593+
// which stores the predicted timestamp at which the server will send the delayed leave event if there wont be any further
594+
// successful restart requests.
595+
this.state.expectedServerDelayLeaveTs = Date.now() + this.delayedLeaveEventDelayMs;
562596
this.resetRateLimitCounter(MembershipActionType.RestartDelayedEvent);
597+
this.setAndEmitProbablyLeft(false);
563598
return createInsertActionUpdate(
564599
MembershipActionType.RestartDelayedEvent,
565600
this.delayedLeaveEventRestartMs,
566601
);
567602
})
568603
.catch((e) => {
604+
if (this.state.expectedServerDelayLeaveTs && this.state.expectedServerDelayLeaveTs <= Date.now()) {
605+
// Once we reach this point it's likely that the server is sending the delayed leave event so we emit `probablyLeft = true`.
606+
// It will emit `probablyLeft = false` once we notice about our leave through sync and successfully setup a new state event.
607+
this.setAndEmitProbablyLeft(true);
608+
}
569609
const repeatActionType = MembershipActionType.RestartDelayedEvent;
570610
if (this.isNotFoundError(e)) {
571611
this.state.delayId = undefined;
@@ -620,6 +660,7 @@ export class MembershipManager
620660
this.stateKey,
621661
)
622662
.then(() => {
663+
this.setAndEmitProbablyLeft(false);
623664
this.state.startTime = Date.now();
624665
// The next update should already use twice the membershipEventExpiryTimeout
625666
this.state.expireUpdateIterations = 1;

0 commit comments

Comments
 (0)