From 4b59dc06ba1bb3bc7ed363fdb8930fa0d4a4236c Mon Sep 17 00:00:00 2001 From: Andrew Ferrazzutti Date: Wed, 6 Sep 2023 09:34:50 -0400 Subject: [PATCH 1/4] Catch when CryptoClient can't find room members CryptoClient#onRoomEvent may throw when looking up the members of a room with a client that isn't in the room, which can happen if appservice namespaces are broad & the appservice receives events for rooms that some of its managed clients are not members of. --- src/e2ee/CryptoClient.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/e2ee/CryptoClient.ts b/src/e2ee/CryptoClient.ts index 74ddfb50..4a756aa4 100644 --- a/src/e2ee/CryptoClient.ts +++ b/src/e2ee/CryptoClient.ts @@ -108,7 +108,7 @@ export class CryptoClient { * @param roomId The room ID. * @param event The event. */ - public async onRoomEvent(roomId: string, event: any) { + public async onRoomEvent(roomId: string, event: any): Promise { await this.roomTracker.onRoomEvent(roomId, event); if (typeof event['state_key'] !== 'string') return; if (event['type'] === 'm.room.member') { @@ -116,8 +116,10 @@ export class CryptoClient { if (membership.effectiveMembership !== 'join' && membership.effectiveMembership !== 'invite') return; await this.engine.addTrackedUsers([membership.membershipFor]); } else if (event['type'] === 'm.room.encryption') { - const members = await this.client.getRoomMembers(roomId, null, ['join', 'invite']); - await this.engine.addTrackedUsers(members.map(e => e.membershipFor)); + return this.client.getRoomMembers(roomId, null, ['join', 'invite']).then( + members => this.engine.addTrackedUsers(members.map(e => e.membershipFor)), + e => void LogService.error("CryptoClient", `Error getting members of room ${roomId}:`, e), + ); } } From dab204a2e0a610d5cdc6b31a38db7e309fca7a84 Mon Sep 17 00:00:00 2001 From: Andrew Ferrazzutti Date: Wed, 6 Sep 2023 18:23:59 -0400 Subject: [PATCH 2/4] Warn instead of error because: - the failure will print its own logs - the point of catching this is that it's _not_ an error --- src/e2ee/CryptoClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/e2ee/CryptoClient.ts b/src/e2ee/CryptoClient.ts index 4a756aa4..70f2134f 100644 --- a/src/e2ee/CryptoClient.ts +++ b/src/e2ee/CryptoClient.ts @@ -118,7 +118,7 @@ export class CryptoClient { } else if (event['type'] === 'm.room.encryption') { return this.client.getRoomMembers(roomId, null, ['join', 'invite']).then( members => this.engine.addTrackedUsers(members.map(e => e.membershipFor)), - e => void LogService.error("CryptoClient", `Error getting members of room ${roomId}:`, e), + e => void LogService.warn("CryptoClient", `Error getting members of room ${roomId}:`, e), ); } } From 8fbe5e0f949be0b7dba8cafe07f717f30bc8ef16 Mon Sep 17 00:00:00 2001 From: Andrew Ferrazzutti Date: Wed, 6 Sep 2023 18:52:46 -0400 Subject: [PATCH 3/4] Don't print error object in warning because the failure already logs the error --- src/e2ee/CryptoClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/e2ee/CryptoClient.ts b/src/e2ee/CryptoClient.ts index 70f2134f..66f5ff13 100644 --- a/src/e2ee/CryptoClient.ts +++ b/src/e2ee/CryptoClient.ts @@ -118,7 +118,7 @@ export class CryptoClient { } else if (event['type'] === 'm.room.encryption') { return this.client.getRoomMembers(roomId, null, ['join', 'invite']).then( members => this.engine.addTrackedUsers(members.map(e => e.membershipFor)), - e => void LogService.warn("CryptoClient", `Error getting members of room ${roomId}:`, e), + e => void LogService.warn("CryptoClient", `Unable to get members of room ${roomId}`), ); } } From 47d7350cd47b927508f5c13c330ef5b098aad3df Mon Sep 17 00:00:00 2001 From: Andrew Ferrazzutti Date: Tue, 31 Oct 2023 09:18:30 -0400 Subject: [PATCH 4/4] Use try/catch pattern --- src/e2ee/CryptoClient.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/e2ee/CryptoClient.ts b/src/e2ee/CryptoClient.ts index 66f5ff13..7a875b89 100644 --- a/src/e2ee/CryptoClient.ts +++ b/src/e2ee/CryptoClient.ts @@ -116,10 +116,14 @@ export class CryptoClient { if (membership.effectiveMembership !== 'join' && membership.effectiveMembership !== 'invite') return; await this.engine.addTrackedUsers([membership.membershipFor]); } else if (event['type'] === 'm.room.encryption') { - return this.client.getRoomMembers(roomId, null, ['join', 'invite']).then( - members => this.engine.addTrackedUsers(members.map(e => e.membershipFor)), - e => void LogService.warn("CryptoClient", `Unable to get members of room ${roomId}`), - ); + let members: MembershipEvent[] | undefined; + try { + members = await this.client.getRoomMembers(roomId, null, ['join', 'invite']); + } catch (e) { + LogService.warn("CryptoClient", `Unable to get members of room ${roomId}`); + return; + } + await this.engine.addTrackedUsers(members.map(e => e.membershipFor)); } }