Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
106 changes: 90 additions & 16 deletions spec/unit/matrixrtc/MembershipManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,14 @@
type Room,
MAX_STICKY_DURATION_MS,
} from "../../../src";
import { MembershipManagerEvent, Status, type Transport, type LivekitFocusSelection } from "../../../src/matrixrtc";
import {
MembershipManagerEvent,
Status,
type Transport,
type LivekitFocusSelection,
type LeaveReason,
LEAVE_REASON_DELAYED,
} from "../../../src/matrixrtc";
import {
makeMockClient,
makeMockRoom,
Expand All @@ -36,6 +43,7 @@
} from "./mocks.ts";
import { MembershipManager, StickyEventMembershipManager } from "../../../src/matrixrtc/MembershipManager.ts";
import { type SessionMembershipData } from "../../../src/matrixrtc/membershipData/index.ts";
import { logger } from "../../../src/logger.ts";

/**
* Create a promise that will resolve once a mocked method is called.
Expand Down Expand Up @@ -75,6 +83,10 @@

const callSession = { id: "ROOM", application: "m.call" };

const membershipDelayedLeaveContent = {
leave_reason: LEAVE_REASON_DELAYED,
};

describe("MembershipManager", () => {
let client: MockClient;
let room: Room;
Expand Down Expand Up @@ -158,7 +170,7 @@
room.roomId,
{ delay: 8000 },
"org.matrix.msc3401.call.member",
{},
membershipDelayedLeaveContent,
"_@alice:example.org_AAAAAAA_m.call",
);
expect(client._unstable_sendDelayedStateEvent).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -197,7 +209,7 @@
room.roomId,
{ delay: 8000 },
"org.matrix.msc3401.call.member",
{},
membershipDelayedLeaveContent,
"_@alice:example.org_AAAAAAA_m.callcustom",
);
expect(client._unstable_sendDelayedStateEvent).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -286,7 +298,13 @@
await sendDelayedStateExceedAttempt.then(); // needed to resolve after the send attempt catches
await sendDelayedStateAttempt;
const callProps = (d: number) => {
return [room!.roomId, { delay: d }, "org.matrix.msc3401.call.member", {}, userStateKey];
return [
room!.roomId,
{ delay: d },
"org.matrix.msc3401.call.member",
membershipDelayedLeaveContent,
userStateKey,
];
};
expect(client._unstable_sendDelayedStateEvent).toHaveBeenNthCalledWith(1, ...callProps(9000));
expect(client._unstable_sendDelayedStateEvent).toHaveBeenNthCalledWith(2, ...callProps(7500));
Expand Down Expand Up @@ -364,7 +382,7 @@
room.roomId,
{ delay: 123456 },
"org.matrix.msc3401.call.member",
{},
membershipDelayedLeaveContent,
"_@alice:example.org_AAAAAAA_m.call",
);
});
Expand Down Expand Up @@ -452,23 +470,35 @@

describe("leave()", () => {
// TODO add rate limit cases.
it("resolves delayed leave event when leave is called", async () => {
const manager = new MembershipManager({}, room, client, callSession);
it("canceled delayed leave event when leave is called", async () => {
const manager = new MembershipManager({}, room, client, callSession, logger);
manager.join([focus]);
await vi.advanceTimersByTimeAsync(1);
await manager.leave();
expect(client._unstable_sendScheduledDelayedEvent).toHaveBeenLastCalledWith("id");
expect(client.sendStateEvent).toHaveBeenCalled();
await vi.runOnlyPendingTimersAsync();
const aReason: LeaveReason = {
code: "test_leave",
reason: "the test leave",
};
await manager.leave(0, aReason);
expect(client._unstable_cancelScheduledDelayedEvent).toHaveBeenLastCalledWith("id");
expect(client.sendStateEvent).toHaveBeenCalledTimes(2);
expect(client.sendStateEvent).toHaveBeenLastCalledWith(
expect.anything(),
expect.anything(),
{
leave_reason: aReason,
},
expect.anything(),
);
expect(manager.delayId).toBe(undefined);
});
it("send leave event when leave is called and resolving delayed leave fails unknown error", async () => {
it("send leave event when leave is called and clearing delayed leave fails unknown error", async () => {
const manager = new MembershipManager({}, room, client, callSession);
manager.join([focus]);
await vi.advanceTimersByTimeAsync(1);
(client._unstable_sendScheduledDelayedEvent as Mock<any>).mockRejectedValue("unknown");
(client._unstable_cancelScheduledDelayedEvent as Mock<any>).mockRejectedValue("unknown");
await manager.leave();

// We send a normal leave event since we failed using sendScheduledDelayedEvent.
// We send the leave event even if cancelScheduledDelayedEvent fails.
expect(client.sendStateEvent).toHaveBeenLastCalledWith(
room.roomId,
"org.matrix.msc3401.call.member",
Expand All @@ -483,7 +513,7 @@
const manager = new MembershipManager({}, room, client, callSession);
manager.join([focus]);
await vi.advanceTimersByTimeAsync(1);
(client._unstable_sendScheduledDelayedEvent as Mock<any>).mockRejectedValue(
(client._unstable_cancelScheduledDelayedEvent as Mock<any>).mockRejectedValue(
new MatrixError({ errcode: "M_NOT_FOUND" }, 404),
);
await manager.leave();
Expand All @@ -495,11 +525,13 @@
{},
"_@alice:example.org_AAAAAAA_m.call",
);
expect(client._unstable_sendScheduledDelayedEvent).not.toHaveBeenCalled();
expect(manager.delayId).toBe(undefined);
});
it("does nothing if not joined", async () => {
const manager = new MembershipManager({}, room, client, callSession);
await expect(manager.leave()).resolves.toBeTruthy();
expect(client._unstable_cancelScheduledDelayedEvent).not.toHaveBeenCalled();
expect(client._unstable_sendDelayedStateEvent).not.toHaveBeenCalled();
expect(client.sendStateEvent).not.toHaveBeenCalled();
});
Expand Down Expand Up @@ -815,6 +847,7 @@
for (let i = 0; i < 10; i++) {
await vi.advanceTimersByTimeAsync(2000);
}
await vi.runAllTimersAsync();
expect(delayEventSendError).toHaveBeenCalled();
});
// because legacy does not have a retry limit and no mechanism to communicate unrecoverable errors.
Expand All @@ -829,7 +862,7 @@
new Headers({ "Retry-After": "1" }),
),
);
const manager = new MembershipManager({}, room, client, callSession);
const manager = new MembershipManager({}, room, client, callSession, logger);
manager.join([focus], focusActive, delayEventRestartError);

for (let i = 0; i < 10; i++) {
Expand Down Expand Up @@ -1031,7 +1064,9 @@
{ delay: 8000 },
null,
"org.matrix.msc4143.rtc.member",

{
leave_reason: membershipDelayedLeaveContent.leave_reason,
msc4354_sticky_key: "@alice:example.org:AAAAAAA_m.call",
},
);
Expand Down Expand Up @@ -1061,6 +1096,45 @@
expect(unrecoverableError.mock.lastCall![0].cause).toBe(stickyError);
});
});

describe("leave()", () => {
it("canceled delayed leave event when leave is called", async () => {
const manager = new StickyEventMembershipManager(
undefined,
room,
client,
callSession,
"@alice:example.org:AAAAAAA_m.call",
logger,
);
manager.join([], focus);

await waitForMockCall(client._unstable_sendStickyEvent, Promise.resolve({ event_id: "id" }));

const aReason: LeaveReason = {
code: "test_leave",
reason: "the test leave",
};
await manager.leave(0, aReason);
// The delayed leave
expect(client._unstable_sendStickyDelayedEvent).toHaveBeenCalledTimes(1);
// The cancel of the delayed
expect(client._unstable_cancelScheduledDelayedEvent).toHaveBeenLastCalledWith("id");
// The join and the leave
expect(client._unstable_sendStickyEvent).toHaveBeenCalledTimes(2);
expect(client._unstable_sendStickyEvent).toHaveBeenLastCalledWith(
expect.anything(),
expect.anything(),
null,
"org.matrix.msc4143.rtc.member",
{
leave_reason: aReason,
msc4354_sticky_key: expect.anything(),
},
);
expect(manager.delayId).toBe(undefined);

Check warning on line 1135 in spec/unit/matrixrtc/MembershipManager.spec.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Prefer "expect(manager.delayId).toBeUndefined()" over this generic assertion; dedicated matchers read better and report clearer failures.

See more on https://sonarcloud.io/project/issues?id=matrix-js-sdk&issues=AZ-D2xmcv60IJetWUKjV&open=AZ-D2xmcv60IJetWUKjV&pullRequest=5437
});
});
});
});

Expand Down
10 changes: 8 additions & 2 deletions src/@types/event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ import {
type IRTCDeclineContent,
type EncryptionKeysEventContent,
type ICallNotifyContent,
type LeaveMembershipEventContent,
type LeaveReason,
type RtcSlotEventContent,
} from "../matrixrtc/types.ts";
import { type M_POLL_END, type M_POLL_START, type PollEndEventContent, type PollStartEventContent } from "./polls.ts";
Expand Down Expand Up @@ -361,7 +363,7 @@ export interface TimelineEvents {
[M_BEACON.name]: MBeaconEventContent;
[M_POLL_START.name]: PollStartEventContent;
[M_POLL_END.name]: PollEndEventContent;
[EventType.RTCMembership]: RtcMembershipData | { msc4354_sticky_key: string }; // An object containing just the sticky key is empty.
[EventType.RTCMembership]: RtcMembershipData | { msc4354_sticky_key: string; leave_reason?: LeaveReason };

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't there be more properties in this variant, like slot_id and member?

}

/**
Expand Down Expand Up @@ -396,7 +398,11 @@ export interface StateEvents {

// MSC3401
[EventType.GroupCallPrefix]: IGroupCallRoomState;
[EventType.GroupCallMemberPrefix]: IGroupCallRoomMemberState | SessionMembershipData | EmptyObject;
[EventType.GroupCallMemberPrefix]:
| IGroupCallRoomMemberState
| SessionMembershipData
| LeaveMembershipEventContent
| EmptyObject;
Comment on lines +404 to +405

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EmptyObject variant is redundant to the LeaveMembershipEventContent variant, since that interface also admits an empty object

[EventType.RTCMembership]: RtcMembershipData | EmptyObject;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, should this state event type still exist? (If so, should it also get a LeaveMembershipEventContent variant?)

[EventType.RTCSlot]: RtcSlotEventContent | EmptyObject;
// MSC3089
Expand Down
5 changes: 3 additions & 2 deletions src/matrixrtc/IMembershipManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ limitations under the License.
*/

import type { CallMembership } from "./CallMembership.ts";
import type { RTCCallIntent, Status, Transport } from "./types.ts";
import type { LeaveReason, RTCCallIntent, Status, Transport } from "./types.ts";
import { type TypedEventEmitter } from "../models/typed-event-emitter.ts";

export enum MembershipManagerEvent {
Expand Down Expand Up @@ -103,10 +103,11 @@ export interface IMembershipManager extends TypedEventEmitter<
/**
* Send all necessary events to make this user leave the RTC session.
* @param timeout the maximum duration in ms until the promise is forced to resolve.
* @param leaveReason the reason to send in the leave event. If `undefined`, no reason is sent.
* @returns It resolves with true in case the leave was sent successfully.
* It resolves with false in case we hit the timeout before sending successfully.
*/
leave(timeout?: number): Promise<boolean>;
leave(timeout?: number, leaveReason?: LeaveReason): Promise<boolean>;
/**
* Call this if the MatrixRTC session members have changed.
*/
Expand Down
13 changes: 10 additions & 3 deletions src/matrixrtc/MatrixRTCSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import type {
RTCCallIntent,
Transport,
SlotDescription,
LeaveReason,
RtcSlotEventContent,
} from "./types.ts";
import {
Expand Down Expand Up @@ -586,9 +587,15 @@ export class MatrixRTCSession extends TypedEventEmitter<
* The membership update required to leave the session will retry if it fails.
* Without network connection the promise will never resolve.
* A timeout can be provided so that there is a guarantee for the promise to resolve.
*
* @param timeout - Optional timeout in milliseconds that fires if the leave takes too long to complete.
* @param leaveReason - The reason for the leave
* @returns Whether the membership update was attempted and did not time out.
*/
public async leaveRoomSession(timeout: number | undefined = undefined): Promise<boolean> {
public async leaveRoomSession(
timeout: number | undefined = undefined,
leaveReason: LeaveReason | undefined = undefined,
): Promise<boolean> {
if (!this.isJoined()) {
this.logger.info(`Not joined to session in room ${this.roomSubset.roomId}: ignoring leave call`);
return false;
Expand All @@ -598,7 +605,7 @@ export class MatrixRTCSession extends TypedEventEmitter<

this.encryptionManager!.leave();

const leavePromise = this.membershipManager!.leave(timeout);
const leavePromise = this.membershipManager!.leave(timeout, leaveReason);
this.emit(MatrixRTCSessionEvent.JoinStateChanged, false);

return await leavePromise;
Expand Down Expand Up @@ -887,7 +894,7 @@ function quickFilterNonRelevantContents(content: IContent, logger: Logger): bool
// Ignore sticky keys for the count
const eventKeysCount = Object.keys(content).filter((k) => k !== "msc4354_sticky_key").length;
// Don't even bother about empty events (saves us from costly type/"key in" checks in bigger rooms)
if (eventKeysCount === 0) return false;
if (eventKeysCount === 0 || (eventKeysCount === 1 && "leave_reason" in content)) return false;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (eventKeysCount === 0 || (eventKeysCount === 1 && "leave_reason" in content)) return false;
if (eventKeysCount === 0 || "leave_reason" in content) return false;

To account for the other keys that can be part of a leave membership (slot_id etc.)


// We first decide if it's a MSC4143 event (per device state key)
if (eventKeysCount > 1 && "application" in content) {
Expand Down
Loading