From 745772f6720d7cf5b86512976da36babed161be6 Mon Sep 17 00:00:00 2001 From: Timo K Date: Tue, 20 Jan 2026 13:45:07 +0100 Subject: [PATCH] Fix rejoin EC crash Due to a duplcaited key (the key not being specific enough) --- playwright/spa-call-sticky.spec.ts | 60 ++++++++++++++++--- .../remoteMembers/MatrixLivekitMembers.ts | 14 ++++- 2 files changed, 62 insertions(+), 12 deletions(-) diff --git a/playwright/spa-call-sticky.spec.ts b/playwright/spa-call-sticky.spec.ts index ea2355d1..246b4a73 100644 --- a/playwright/spa-call-sticky.spec.ts +++ b/playwright/spa-call-sticky.spec.ts @@ -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. */ -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"; -test("One to One call using matrix rtc 2.0 aka sticky events", async ({ - browser, - page, - browserName, -}) => { +async function setupTwoUserSpaCall( + browser: Browser, + page: Page, + browserName: string, +): Promise<{ guestPage: Page }> { test.skip( browserName === "firefox", "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", "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(guestPage, 2); - // Assert both sides have sent sticky membership events - expect(androlHasSentStickyEvent).toEqual(true); - expect(pevaraHasSentStickyEvent).toEqual(true); + await guestPage.reload(); + await expect(guestPage.getByTestId("lobby_joinCall")).toBeVisible(); + + // 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 { diff --git a/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts b/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts index c98cb581..b54a50df 100644 --- a/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts +++ b/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts @@ -111,15 +111,23 @@ export function createMatrixLivekitMembers$({ : null; 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 }, }; } }, // 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( - `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$); // will only get called once per `participantId, userId` pair.