feat: add user avatar update events to a fanout queue (#39907)#39950
Open
NestroyMusoke wants to merge 4 commits intoRocketChat:developfrom
Open
feat: add user avatar update events to a fanout queue (#39907)#39950NestroyMusoke wants to merge 4 commits intoRocketChat:developfrom
NestroyMusoke wants to merge 4 commits intoRocketChat:developfrom
Conversation
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: b8ae598 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds an in-memory Changes
Sequence DiagramsequenceDiagram
actor User
participant FedMatrix as FederationMatrix
participant Queue as FanoutQueue
participant SDK as federationSDK
participant Rooms as RoomService
User->>FedMatrix: Avatar update event
FedMatrix->>FedMatrix: Validate & lookup local user
FedMatrix->>FedMatrix: Build Matrix userId & avatar_url
FedMatrix->>Rooms: Query federated room IDs
Rooms-->>FedMatrix: Room IDs
loop For each room
FedMatrix->>Queue: enqueue(roomId, updateTask)
end
rect rgba(100, 150, 200, 0.5)
Note over Queue: Manage concurrency & retries
Queue->>SDK: updateUserProfile(roomId, profile)
alt Task succeeds
SDK-->>Queue: Success
Queue->>Queue: Resolve promise
else Task fails
SDK-->>Queue: Error
alt Retries remaining
Queue->>Queue: Wait backoff (baseDelayMs * 2^retries) then re-enqueue
else Max retries exhausted
Queue->>Queue: Log error & reject promise
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
bot
added
the
type: feature
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="ee/packages/federation-matrix/src/FanoutQueue.ts">
<violation number="1" location="ee/packages/federation-matrix/src/FanoutQueue.ts:105">
P1: Exhausted queue tasks reject promises even though call sites use fire-and-forget, causing unhandled promise rejections.</violation>
</file>
<file name="ee/packages/federation-matrix/src/FederationMatrix.ts">
<violation number="1" location="ee/packages/federation-matrix/src/FederationMatrix.ts:138">
P2: Avatar fanout sends unvalidated Matrix user IDs to federation SDK, allowing malformed IDs (e.g., when `serverName` is empty) to be retried repeatedly per room.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| msg: `Task "${item.id}" exhausted all ${this.maxRetries} retries, dropping`, | ||
| err: error, | ||
| }); | ||
| item.reject(error); |
Contributor
There was a problem hiding this comment.
P1: Exhausted queue tasks reject promises even though call sites use fire-and-forget, causing unhandled promise rejections.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ee/packages/federation-matrix/src/FanoutQueue.ts, line 105:
<comment>Exhausted queue tasks reject promises even though call sites use fire-and-forget, causing unhandled promise rejections.</comment>
<file context>
@@ -0,0 +1,112 @@
+ msg: `Task "${item.id}" exhausted all ${this.maxRetries} retries, dropping`,
+ err: error,
+ });
+ item.reject(error);
+ }
+ } finally {
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
ee/packages/federation-matrix/src/FederationMatrix.ts (1)
154-161: Handle potential unhandled promise rejection from fire-and-forget enqueue.Using
void this.avatarFanoutQueue.enqueue(...)discards the promise but doesn't prevent unhandled rejection if the task exhausts all retries. While the queue logs the error internally, thereject(error)call inFanoutQueue.process()will trigger an unhandled promise rejection warning/error.Consider adding
.catch()to explicitly handle the rejection:Proposed fix
- void this.avatarFanoutQueue.enqueue( + this.avatarFanoutQueue.enqueue( `avatar:${username}:${externalRoomId}`, async () => { await federationSDK.updateUserProfile(externalRoomId, matrixUserId, profilePayload); this.logger.debug({ msg: 'Sent avatar update', username, roomId: externalRoomId }); }, - ); + ).catch(() => { + // Error already logged by FanoutQueue + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/packages/federation-matrix/src/FederationMatrix.ts` around lines 154 - 161, The enqueue call is fire-and-forget and can still emit an unhandled rejection if FanoutQueue.process ultimately rejects; update the usage of avatarFanoutQueue.enqueue (where `void this.avatarFanoutQueue.enqueue(...)` is used) to attach a rejection handler (e.g., call .catch(...) on the returned promise) that logs the error via this.logger.error and suppresses the unhandled rejection; keep the existing internal queue logging but ensure the outer call (the promise returned by avatarFanoutQueue.enqueue) is explicitly handled so FanoutQueue.process rejections don’t become unhandled.ee/packages/federation-matrix/src/FanoutQueue.ts (1)
88-106: Clarify retry logging and consider unhandled rejection risk.Two observations:
Log message clarity: The log says
attempt ${item.retries + 1}/${this.maxRetries}but this represents "retry count", not "attempt count". WithmaxRetries=3, there are 4 total attempts (1 initial + 3 retries). Consider changing toretry ${item.retries + 1}/${this.maxRetries}for clarity.Unhandled promise rejection: When callers use fire-and-forget (
void queue.enqueue(...)), theitem.reject(error)on line 105 creates an unhandled promise rejection. The queue already logs the error, so the rejection may be unnecessary for fire-and-forget use cases. Consider either:
- Swallowing the rejection internally after logging, or
- Documenting that callers must
.catch()if they discard the promisePotential fix for log clarity
if (item.retries < this.maxRetries) { const delay = this.baseDelayMs * 2 ** item.retries; this.logger.warn({ - msg: `Task "${item.id}" failed (attempt ${item.retries + 1}/${this.maxRetries}), retrying in ${delay}ms`, + msg: `Task "${item.id}" failed (retry ${item.retries + 1}/${this.maxRetries}), retrying in ${delay}ms`, err: error, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/packages/federation-matrix/src/FanoutQueue.ts` around lines 88 - 106, Update the retry log to use "retry" wording so the message reflects retry count (change logger.warn msg from `attempt ${item.retries + 1}/${this.maxRetries}` to `retry ${item.retries + 1}/${this.maxRetries}`) and prevent unhandled promise rejections by not unconditionally calling item.reject(error) when retries are exhausted; instead, add an "awaited" flag on enqueued items in FanoutQueue.enqueue (set true when the caller holds the returned promise, false for fire-and-forget), and in the exhaustion branch of the retry logic only call item.reject(error) if item.awaited is true — otherwise just log the error and drop the item (or resolve/suppress internally). Ensure references: FanoutQueue.enqueue, item.retries, this.maxRetries, logger.warn, logger.error, and item.reject are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ee/packages/federation-matrix/src/FederationMatrix.ts`:
- Around line 44-48: The avatarFanoutQueue property declaration uses
inconsistent/mixed indentation compared to the surrounding class; locate the
private readonly avatarFanoutQueue = new FanoutQueue('avatar-updates', { ... })
in FederationMatrix.ts and normalize its leading spaces to match the class
member indentation style (align with other private readonly members) so the
spacing is consistent with the rest of the class.
- Around line 138-140: The MXC URI is being built from avatarETag (in
FederationMatrix: matrixUserId/avatarUrl), but avatarETag is an HTTP ETag, not a
Matrix media_id; replace usage of avatarETag with the actual file/media id
returned by your FileStore upload (or perform a media upload to Matrix to obtain
media_id) and construct avatarUrl as `mxc://<serverName>/<media_id>` instead;
locate where avatarETag is read (setUserAvatar.ts and FederationMatrix.ts) and
either change the stored field to the fileId/media_id or call an upload helper
(e.g., uploadMediaToMatrix or similar) to obtain and persist the correct
media_id before constructing avatarUrl.
---
Nitpick comments:
In `@ee/packages/federation-matrix/src/FanoutQueue.ts`:
- Around line 88-106: Update the retry log to use "retry" wording so the message
reflects retry count (change logger.warn msg from `attempt ${item.retries +
1}/${this.maxRetries}` to `retry ${item.retries + 1}/${this.maxRetries}`) and
prevent unhandled promise rejections by not unconditionally calling
item.reject(error) when retries are exhausted; instead, add an "awaited" flag on
enqueued items in FanoutQueue.enqueue (set true when the caller holds the
returned promise, false for fire-and-forget), and in the exhaustion branch of
the retry logic only call item.reject(error) if item.awaited is true — otherwise
just log the error and drop the item (or resolve/suppress internally). Ensure
references: FanoutQueue.enqueue, item.retries, this.maxRetries, logger.warn,
logger.error, and item.reject are updated accordingly.
In `@ee/packages/federation-matrix/src/FederationMatrix.ts`:
- Around line 154-161: The enqueue call is fire-and-forget and can still emit an
unhandled rejection if FanoutQueue.process ultimately rejects; update the usage
of avatarFanoutQueue.enqueue (where `void this.avatarFanoutQueue.enqueue(...)`
is used) to attach a rejection handler (e.g., call .catch(...) on the returned
promise) that logs the error via this.logger.error and suppresses the unhandled
rejection; keep the existing internal queue logging but ensure the outer call
(the promise returned by avatarFanoutQueue.enqueue) is explicitly handled so
FanoutQueue.process rejections don’t become unhandled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c4dfae03-97ae-42e3-a329-a1c97dca6223
📒 Files selected for processing (2)
ee/packages/federation-matrix/src/FanoutQueue.tsee/packages/federation-matrix/src/FederationMatrix.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
ee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/FanoutQueue.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2025-12-09T20:01:00.324Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: When reviewing federation invite handling in Rocket.Chat (specifically under ee/packages/federation-matrix), understand that rejecting an invite via federationSDK.rejectInvite() triggers an event-driven cleanup: a leave event is emitted and handled by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, which calls Room.performUserRemoval() to remove the subscription. Do not add explicit cleanup in the reject branch of handleInvite(); rely on the existing leave-event flow for cleanup. If making changes, ensure this invariant remains and that any related paths still funnel cleanup through the leave event to avoid duplicate or missing removals.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/FanoutQueue.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/FanoutQueue.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
ee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/FanoutQueue.ts
🔇 Additional comments (1)
ee/packages/federation-matrix/src/FederationMatrix.ts (1)
118-162: LGTM on the overall avatar update handler logic.The handler correctly:
- Skips Matrix-style usernames (line 119)
- Validates user existence (lines 123-129)
- Filters out remote federated users using
isUserNativeFederated(lines 131-134)- Handles undefined/empty
avatarETagby settingavatar_urltonull(line 140)- Uses fanout queue for concurrent, retry-capable per-room profile updates (lines 149-161)
The integration with
FanoutQueuealigns well with the PR objective to introduce fanout processing for user avatar update events.
Comment on lines
+138
to
+140
| const matrixUserId = `@${localUser.username}:${this.serverName}`; | ||
|
|
||
| const avatarUrl = avatarETag ? `mxc://${this.serverName}/${avatarETag}` : null; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for how avatarETag is used elsewhere in federation context
# and whether there's any avatar upload to Matrix media server
rg -n -C3 'avatarETag|avatar_url|mxc://' ee/packages/federation-matrix/
# Check if there's any media upload logic that might explain this mapping
rg -n -C3 'updateUserProfile|setProfileAvatar' ee/packages/federation-matrix/Repository: RocketChat/Rocket.Chat
Length of output: 10988
🏁 Script executed:
# Find where the user.avatarUpdate event is emitted and what avatarETag represents
rg -n 'user\.avatarUpdate|avatarETag' --type ts --type js -C 5 | head -100Repository: RocketChat/Rocket.Chat
Length of output: 7717
🏁 Script executed:
# Look for the event emission in the main Rocket.Chat app to understand avatarETag payload
rg -n 'emit.*user\.avatarUpdate|avatarUpdate.*emit' --type ts --type js -C 3 | head -80Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Search for FileUpload and avatar-related media handling
rg -n 'FileUpload|avatar.*upload|uploadUserFile' --type ts --type js | head -50Repository: RocketChat/Rocket.Chat
Length of output: 6732
🏁 Script executed:
# Find the event emitter for user.avatarUpdate in the main codebase
rg -n 'user:avatarUpdate|user\.avatarUpdate.*emit|emit.*avatarUpdate' apps/meteor --type ts --type js -C 3Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Look at how avatar changes are handled in the user model or services
rg -n 'setAvatar|updateAvatar|avatarOrigin' apps/meteor --type ts --type js | head -40Repository: RocketChat/Rocket.Chat
Length of output: 3898
🏁 Script executed:
# Check if there's media upload in the federation context for avatars
rg -n 'uploadMedia|media.*upload|avatar.*matrix' ee/packages/federation-matrix --type ts --type js -C 2Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Find the event listener registration in FederationMatrix
rg -n 'onEvent|on.*user\.avatarUpdate' ee/packages/federation-matrix/src --type ts -C 5Repository: RocketChat/Rocket.Chat
Length of output: 10238
🏁 Script executed:
# Look at the FederationMatrix constructor and event setup
rg -n 'class FederationMatrix|constructor' ee/packages/federation-matrix/src/FederationMatrix.ts -A 20Repository: RocketChat/Rocket.Chat
Length of output: 737
🏁 Script executed:
# Check if there's an events module or what triggers the user.avatarUpdate event
rg -n 'observeUser|avatar.*listener|EventEmitter' ee/packages/federation-matrix/src --type tsRepository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Look at MatrixMediaService to understand what fileId should be
cat ee/packages/federation-matrix/src/services/MatrixMediaService.tsRepository: RocketChat/Rocket.Chat
Length of output: 3630
🏁 Script executed:
# Search for where user.avatarUpdate event is actually emitted
rg -n 'emit.*user\.avatarUpdate|user\.avatarUpdate.*emit|notifyUser' packages/ apps/meteor --type ts --type js | head -30Repository: RocketChat/Rocket.Chat
Length of output: 4227
🏁 Script executed:
# Look for file/media handling related to avatars in the codebase
rg -n 'avatarETag.*file|file.*avatarETag|FileUpload.*avatar' --type ts --type js | head -20Repository: RocketChat/Rocket.Chat
Length of output: 275
🏁 Script executed:
# Find where the user.avatarUpdate event is actually emitted
rg -n 'avatar.*event|avatarUpdate|avatarETag' packages/ apps/meteor/server --type ts --type js -C 2 | grep -A 5 -B 5 'emit\|fire\|trigger'Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Look at FileUpload avatarsOnFinishUpload to understand what gets stored as avatarETag
rg -n 'avatarsOnFinishUpload' --type ts --type js -A 30Repository: RocketChat/Rocket.Chat
Length of output: 5321
🏁 Script executed:
# Search for where avatarETag is set on the user document
rg -n 'setAvatarData|avatarETag.*=|avatarOrigin' apps/meteor/app/lib --type ts --type js -C 3Repository: RocketChat/Rocket.Chat
Length of output: 4631
🏁 Script executed:
# Search for the event system that FederationMatrix uses to receive user.avatarUpdate
rg -n 'class.*Service|onEvent.*function|registerListener' ee/packages/federation-matrix/src --type ts | head -20Repository: RocketChat/Rocket.Chat
Length of output: 313
🏁 Script executed:
# Look for where the notification/event for user.avatarUpdate comes from
rg -n 'user\.avatarUpdate|avatarUpdate' apps/meteor --type ts --type js -B 3 -A 3 | head -80Repository: RocketChat/Rocket.Chat
Length of output: 3715
🏁 Script executed:
# Check if there's a file ID available that should be used instead
rg -n 'result\._id|fileId|uploadedFile' apps/meteor/app/lib/server/functions/setUserAvatar.ts -C 2Repository: RocketChat/Rocket.Chat
Length of output: 48
Use file ID instead of avatarETag for MXC URI construction.
The mxc:// URI at line 140 is constructed using avatarETag as the media identifier, but avatarETag is an HTTP ETag (cache validation token from the FileStore upload), not a Matrix media ID. Per the code in setUserAvatar.ts:198, avatarETag = etag || result?.etag || '', which is the HTTP cache header value.
Matrix MXC URIs require a stable media_id from the upload response or file reference. The current implementation will generate invalid MXC URIs that Matrix homeservers cannot resolve, breaking avatar synchronization for federated users.
Either:
- Store and pass the actual file ID (from the FileStore upload) instead of the ETag, or
- Implement a media upload step to Matrix that returns the proper
media_idand store that value instead
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ee/packages/federation-matrix/src/FederationMatrix.ts` around lines 138 -
140, The MXC URI is being built from avatarETag (in FederationMatrix:
matrixUserId/avatarUrl), but avatarETag is an HTTP ETag, not a Matrix media_id;
replace usage of avatarETag with the actual file/media id returned by your
FileStore upload (or perform a media upload to Matrix to obtain media_id) and
construct avatarUrl as `mxc://<serverName>/<media_id>` instead; locate where
avatarETag is read (setUserAvatar.ts and FederationMatrix.ts) and either change
the stored field to the fileId/media_id or call an upload helper (e.g.,
uploadMediaToMatrix or similar) to obtain and persist the correct media_id
before constructing avatarUrl.
Removed TODO comments related to user exclusion, file handling, and common function usage for matrix user ID.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #39907
Changes
Introduces a
FanoutQueueutility and uses it to process user avatarupdate events across federated rooms with concurrency control and retry.
New file:
FanoutQueue.tsvoid queue.enqueue(...)Modified:
FederationMatrix.tsuser.avatarUpdateevent handler insidecreated()mxc://avatar URL from the avatarETaginstead of sequential processing
Summary by CodeRabbit