Merge pull request matrix-org#359 from matrix-org/af/catch-update-pro…

…file Catch errors on profile updates
vkleen · Feb 23, 2024 · f0b97d3 · f0b97d3
2 parents ff801d9 + a17a7d6
commit f0b97d3
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 23 deletions.
diff --git a/changelog.d/359.bugfix b/changelog.d/359.bugfix
@@ -0,0 +1 @@
+Catch errors when profile updates fail. Notably, prevent fatal errors when an inbound displayname change fails.
diff --git a/src/GatewayHandler.ts b/src/GatewayHandler.ts
@@ -189,18 +189,29 @@ export class GatewayHandler {
                 throw Error("This room has been denied");
             }
             await intent.ensureRegistered();
+            let profileUpdateSuccess = false;
             if (this.config.tuning.waitOnProfileBeforeSend) {
-                await this.profileSync.updateProfile(protocol, data.sender, this.purple.gateway);
+                try {
+                    await this.profileSync.updateProfile(protocol, data.sender, this.purple.gateway);
+                    profileUpdateSuccess = true;
+                } catch (e) {
+                    log.error(`Failed to update profile: ${e}`);
+                }
             }
             log.info(`Attempting to join ${data.roomAlias}`)
             roomId = await intent.join(data.roomAlias);
             if (this.config.getRoomRule(roomId) === "deny") {
                 throw Error("This room has been denied");
             }
             if (!this.config.tuning.waitOnProfileBeforeSend) {
-                await this.profileSync.updateProfile(protocol, data.sender, this.purple.gateway);
+                try {
+                    await this.profileSync.updateProfile(protocol, data.sender, this.purple.gateway);
+                    profileUpdateSuccess = true;
+                } catch (e) {
+                    log.error(`Failed to update profile: ${e}`);
+                }
             }
-            if (data.nick) {
+            if (data.nick && profileUpdateSuccess) {
                 // Set the user's displayname in the room to their nickname.
                 // Do this after a short delay, so that we don't have a race on
                 // the server setting the global displayname.

diff --git a/src/MatrixRoomHandler.ts b/src/MatrixRoomHandler.ts
@@ -298,7 +298,11 @@ export class MatrixRoomHandler {
         // Update the user if needed.
         const account = this.purple.getAccount(data.account.username, data.account.protocol_id, matrixUser.getId());
         if (account) {
-            await this.profileSync.updateProfile(protocol, data.sender, account);
+            try {
+                await this.profileSync.updateProfile(protocol, data.sender, account);
+            } catch (e) {
+                log.error(`Failed to update profile: ${e}`);
+            }
         }
 
         const intent = this.bridge.getIntent(senderMatrixUser.getId());
@@ -379,13 +383,17 @@ export class MatrixRoomHandler {
         );
         const account = this.purple.getAccount(data.account.username, data.account.protocol_id);
         if (account) {
-            await this.profileSync.updateProfile(
-                protocol,
-                data.sender,
-                account,
-                false,
-                ProtoHacks.getSenderIdToLookup(protocol, data.sender, data.conv.name),
-            );
+            try {
+                await this.profileSync.updateProfile(
+                    protocol,
+                    data.sender,
+                    account,
+                    false,
+                    ProtoHacks.getSenderIdToLookup(protocol, data.sender, data.conv.name),
+                );
+            } catch (e) {
+                log.error(`Failed to update profile: ${e}`);
+            }
         }
 
         const intent = this.bridge.getIntent(senderMatrixUser.getId());

diff --git a/src/ProfileSync.ts b/src/ProfileSync.ts
@@ -63,19 +63,30 @@ export class ProfileSync {
             remoteProfileSet.avatar_uri = buddy.icon_path;
         }
 
-        const intent = this.bridge.getIntent(matrixUser.getId());
+        const errors: Error[] = [];
 
-        if (remoteProfileSet.nick && matrixUser.get("displayname") !== remoteProfileSet.nick) {
-            log.debug(`Got a nick "${remoteProfileSet.nick}", setting`);
-            await intent.setDisplayName(remoteProfileSet.nick);
-            matrixUser.set("displayname", remoteProfileSet.nick);
-        } else if (!matrixUser.get("displayname") && remoteProfileSet.name) {
-            log.debug(`Got a name "${remoteProfileSet.name}", setting`);
-            // Don't ever set the name (ugly) over the nick unless we have never set it.
-            // Nicks come and go depending on the libpurple cache and whether the user
-            // is online (in XMPPs case at least).
-            await intent.setDisplayName(remoteProfileSet.name);
-            matrixUser.set("displayname", remoteProfileSet.name);
+        const intent = this.bridge.getIntent(matrixUser.getId());
+        {
+            let displayName: string | undefined;
+            if (remoteProfileSet.nick && matrixUser.get("displayname") !== remoteProfileSet.nick) {
+                log.debug(`Got a nick "${remoteProfileSet.nick}", setting`);
+                displayName = remoteProfileSet.nick;
+            } else if (!matrixUser.get("displayname") && remoteProfileSet.name) {
+                log.debug(`Got a name "${remoteProfileSet.name}", setting`);
+                // Don't ever set the name (ugly) over the nick unless we have never set it.
+                // Nicks come and go depending on the libpurple cache and whether the user
+                // is online (in XMPPs case at least).
+                displayName = remoteProfileSet.name;
+            }
+            if (displayName !== undefined) {
+                try {
+                    await intent.setDisplayName(displayName);
+                    matrixUser.set("displayname", displayName);
+                } catch (e) {
+                    log.error("Failed to set display_name for user:", e);
+                    errors.push(e);
+                }
+            }
         }
 
         if (remoteProfileSet.avatar_uri && matrixUser.get("avatar_url") !== remoteProfileSet.avatar_uri) {
@@ -90,9 +101,14 @@ export class ProfileSync {
                 matrixUser.set("avatar_url", remoteProfileSet.avatar_uri);
             } catch (e) {
                 log.error("Failed to update avatar_url for user:", e);
+                errors.push(e);
             }
         }
         await this.store.setMatrixUser(matrixUser);
+
+        if (errors.length) {
+            throw new AggregateError(errors, "Failed to fully update profile for user");
+        }
     }
 
     private async getOrCreateStoreUsers(protocol: BifrostProtocol, senderId: string)
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Catch errors when profile updates fail. Notably, prevent fatal errors when an inbound displayname change fails.