Fix rejoin EC crash
Due to a duplcaited key (the key not being specific enough)
This commit is contained in:
@@ -5,15 +5,21 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
|
|||||||
Please see LICENSE in the repository root for full details.
|
Please see LICENSE in the repository root for full details.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { expect, type Page, test, type Request } from "@playwright/test";
|
import {
|
||||||
|
expect,
|
||||||
|
type Page,
|
||||||
|
test,
|
||||||
|
type Request,
|
||||||
|
type Browser,
|
||||||
|
} from "@playwright/test";
|
||||||
|
|
||||||
import { SpaHelpers } from "./spa-helpers";
|
import { SpaHelpers } from "./spa-helpers";
|
||||||
|
|
||||||
test("One to One call using matrix rtc 2.0 aka sticky events", async ({
|
async function setupTwoUserSpaCall(
|
||||||
browser,
|
browser: Browser,
|
||||||
page,
|
page: Page,
|
||||||
browserName,
|
browserName: string,
|
||||||
}) => {
|
): Promise<{ guestPage: Page }> {
|
||||||
test.skip(
|
test.skip(
|
||||||
browserName === "firefox",
|
browserName === "firefox",
|
||||||
"The is test is not working on firefox CI environment. No mic/audio device inputs so cam/mic are disabled",
|
"The is test is not working on firefox CI environment. No mic/audio device inputs so cam/mic are disabled",
|
||||||
@@ -63,13 +69,49 @@ test("One to One call using matrix rtc 2.0 aka sticky events", async ({
|
|||||||
"Pevara",
|
"Pevara",
|
||||||
"2_0",
|
"2_0",
|
||||||
);
|
);
|
||||||
|
// Assert both sides have sent sticky membership events
|
||||||
|
expect(androlHasSentStickyEvent).toEqual(true);
|
||||||
|
expect(pevaraHasSentStickyEvent).toEqual(true);
|
||||||
|
|
||||||
|
return { guestPage };
|
||||||
|
}
|
||||||
|
|
||||||
|
test("One to One call using matrix rtc 2.0 aka sticky events", async ({
|
||||||
|
browser,
|
||||||
|
page,
|
||||||
|
browserName,
|
||||||
|
}) => {
|
||||||
|
const { guestPage } = await setupTwoUserSpaCall(browser, page, browserName);
|
||||||
|
|
||||||
|
await SpaHelpers.expectVideoTilesCount(page, 2);
|
||||||
|
await SpaHelpers.expectVideoTilesCount(guestPage, 2);
|
||||||
|
});
|
||||||
|
|
||||||
|
// This issue occurs when a member leave but does not clean up their sticky event.
|
||||||
|
// If they rejoin they will use a new stickye key (stickyKey = member.id = UUID())
|
||||||
|
// We end up with two memberships with the same user and device id. This previously
|
||||||
|
// was a impossible case since that would be the same state event. Now its possible.
|
||||||
|
// We need to ALWAYS key by userId, deviceId and member.id. This test checks that.
|
||||||
|
test("One to One rejoin after improper leave does not crash EC", async ({
|
||||||
|
browser,
|
||||||
|
page,
|
||||||
|
browserName,
|
||||||
|
}) => {
|
||||||
|
const { guestPage } = await setupTwoUserSpaCall(browser, page, browserName);
|
||||||
|
|
||||||
await SpaHelpers.expectVideoTilesCount(page, 2);
|
await SpaHelpers.expectVideoTilesCount(page, 2);
|
||||||
await SpaHelpers.expectVideoTilesCount(guestPage, 2);
|
await SpaHelpers.expectVideoTilesCount(guestPage, 2);
|
||||||
|
|
||||||
// Assert both sides have sent sticky membership events
|
await guestPage.reload();
|
||||||
expect(androlHasSentStickyEvent).toEqual(true);
|
await expect(guestPage.getByTestId("lobby_joinCall")).toBeVisible();
|
||||||
expect(pevaraHasSentStickyEvent).toEqual(true);
|
|
||||||
|
// Check if rejoining with the same browser context (device) breaks EC.
|
||||||
|
// This has happened on versions that do not consider the member.id as part of the key for a media tile.
|
||||||
|
await guestPage.getByTestId("lobby_joinCall").click();
|
||||||
|
|
||||||
|
// We cannot use the `expectVideoTilesCount` helper here since one of them is expected to show waiting for media
|
||||||
|
await expect(page.getByTestId("videoTile")).toHaveCount(3);
|
||||||
|
await expect(guestPage.getByTestId("videoTile")).toHaveCount(2);
|
||||||
});
|
});
|
||||||
|
|
||||||
function isStickySend(url: string): boolean {
|
function isStickySend(url: string): boolean {
|
||||||
|
|||||||
@@ -111,15 +111,23 @@ export function createMatrixLivekitMembers$({
|
|||||||
: null;
|
: null;
|
||||||
|
|
||||||
yield {
|
yield {
|
||||||
keys: [membership.userId, membership.deviceId],
|
// This could also just be the memberId without the other fields.
|
||||||
|
// In theory we should never have the same memberId for different userIds (they are UUIDs)
|
||||||
|
// This still makes us resilient agains someone who intentionally tries to use the same memberId.
|
||||||
|
// If they want to do this they would now need to also use the same sender which is impossible.
|
||||||
|
keys: [
|
||||||
|
membership.userId,
|
||||||
|
membership.deviceId,
|
||||||
|
membership.memberId,
|
||||||
|
],
|
||||||
data: { membership, participant, connection },
|
data: { membership, participant, connection },
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
// Each update where the key of the generator array do not change will result in updates to the `data$` observable in the factory.
|
// Each update where the key of the generator array do not change will result in updates to the `data$` observable in the factory.
|
||||||
(scope, data$, userId, deviceId) => {
|
(scope, data$, userId, deviceId, memberId) => {
|
||||||
logger.debug(
|
logger.debug(
|
||||||
`Generating member for livekitIdentity: ${data$.value.membership.rtcBackendIdentity}, userId:deviceId: ${userId}${deviceId}`,
|
`Generating member for livekitIdentity: ${data$.value.membership.rtcBackendIdentity},keys userId:deviceId:memberId ${userId}:${deviceId}:${memberId}`,
|
||||||
);
|
);
|
||||||
const { participant$, ...rest } = scope.splitBehavior(data$);
|
const { participant$, ...rest } = scope.splitBehavior(data$);
|
||||||
// will only get called once per `participantId, userId` pair.
|
// will only get called once per `participantId, userId` pair.
|
||||||
|
|||||||
Reference in New Issue
Block a user