-
Notifications
You must be signed in to change notification settings - Fork 13k
fix: process joins from full state #37156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||||||||||||||||||||||||||||
| import { Room } from '@rocket.chat/core-services'; | ||||||||||||||||||||||||||||||||
| import { FederationMatrix, Room } from '@rocket.chat/core-services'; | ||||||||||||||||||||||||||||||||
| import { isUserNativeFederated, type IUser } from '@rocket.chat/core-typings'; | ||||||||||||||||||||||||||||||||
| import { eventIdSchema, roomIdSchema } from '@rocket.chat/federation-sdk'; | ||||||||||||||||||||||||||||||||
| import type { | ||||||||||||||||||||||||||||||||
|
|
@@ -172,7 +172,7 @@ async function joinRoom({ | |||||||||||||||||||||||||||||||
| await room.joinUser(inviteEvent.roomId, inviteEvent.event.state_key); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // now we create the room we saved post joining | ||||||||||||||||||||||||||||||||
| const matrixRoom = await state.getFullRoomState2(inviteEvent.roomId); | ||||||||||||||||||||||||||||||||
| const matrixRoom = await state.getLatestRoomState2(inviteEvent.roomId); | ||||||||||||||||||||||||||||||||
| if (!matrixRoom) { | ||||||||||||||||||||||||||||||||
| throw new Error('room not found not processing invite'); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
@@ -265,6 +265,11 @@ async function joinRoom({ | |||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| await Room.addUserToRoom(internalRoomId, { _id: user._id }, { _id: senderUserId, username: inviteEvent.sender }); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| for await (const event of matrixRoom.getMemberJoinEvents()) { | ||||||||||||||||||||||||||||||||
| console.log('emitting another join', event.toStrippedJson()); | ||||||||||||||||||||||||||||||||
| await FederationMatrix.emitJoin(event); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
Comment on lines
+269
to
+272
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replace console.log with proper logger and add error handling. The loop has several issues:
Apply this diff to use proper logging and add error handling: await Room.addUserToRoom(internalRoomId, { _id: user._id }, { _id: senderUserId, username: inviteEvent.sender });
for await (const event of matrixRoom.getMemberJoinEvents()) {
- console.log('emitting another join', event.toStrippedJson());
- await FederationMatrix.emitJoin(event);
+ try {
+ await FederationMatrix.emitJoin(event);
+ } catch (error) {
+ console.error('Failed to emit join event', { eventId: event.eventId, error });
+ }
}
}Note: If a proper logger instance is available in this scope, replace 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| async function startJoiningRoom(...opts: Parameters<typeof joinRoom>) { | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -23,4 +23,5 @@ export interface IFederationMatrixService { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inviteUsersToRoom(room: IRoomFederated, usersUserName: string[], inviter: IUser): Promise<void>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| notifyUserTyping(rid: string, user: string, isTyping: boolean): Promise<void>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| verifyMatrixIds(matrixIds: string[]): Promise<{ [key: string]: string }>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| emitJoin(membershipEvent: any): Promise<void>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Replace The interface uses Apply this diff to align the interface with the implementation: +import type { PersistentEventBase, RoomVersion } from '@rocket.chat/federation-sdk';
+
export interface IFederationMatrixService {
createRoom(room: IRoomFederated, owner: IUser, members: string[]): Promise<{ room_id: string; event_id: string }>;
ensureFederatedUsersExistLocally(members: string[]): Promise<void>;
createDirectMessageRoom(room: IRoomFederated, members: IUser[], creatorId: IUser['_id']): Promise<void>;
sendMessage(message: IMessage, room: IRoomFederated, user: IUser): Promise<void>;
deleteMessage(matrixRoomId: string, message: IMessage): Promise<void>;
sendReaction(messageId: string, reaction: string, user: IUser): Promise<void>;
removeReaction(messageId: string, reaction: string, user: IUser, oldMessage: IMessage): Promise<void>;
getEventById(eventId: string): Promise<any | null>;
leaveRoom(rid: IRoomFederated['_id'], user: IUser): Promise<void>;
kickUser(room: IRoomNativeFederated, removedUser: IUser, userWhoRemoved: IUser): Promise<void>;
updateMessage(room: IRoomNativeFederated, message: IMessage): Promise<void>;
updateRoomName(rid: string, displayName: string, user: IUser): Promise<void>;
updateRoomTopic(room: IRoomNativeFederated, topic: string, user: IUser): Promise<void>;
addUserRoleRoomScoped(
room: IRoomNativeFederated,
senderId: string,
userId: string,
role: 'moderator' | 'owner' | 'leader' | 'user',
): Promise<void>;
inviteUsersToRoom(room: IRoomFederated, usersUserName: string[], inviter: IUser): Promise<void>;
notifyUserTyping(rid: string, user: string, isTyping: boolean): Promise<void>;
verifyMatrixIds(matrixIds: string[]): Promise<{ [key: string]: string }>;
- emitJoin(membershipEvent: any): Promise<void>;
+ emitJoin(membershipEvent: PersistentEventBase<RoomVersion, 'm.room.member'>): Promise<void>;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix warning message and preserve original event data.
The method has several issues:
membership: 'join', but this should preserve the original membership state from the event parameterDate.now()fororigin_server_tsloses the original event timestampApply this diff to fix these issues:
async emitJoin(membershipEvent: PersistentEventBase<RoomVersion, 'm.room.member'>) { if (!this.homeserverServices) { - this.logger.warn('Homeserver services not available, skipping user role room scoped'); + this.logger.warn('Homeserver services not available, skipping join event emission'); return; } this.homeserverServices.emitter.emit('homeserver.matrix.membership', { event_id: membershipEvent.eventId, event: membershipEvent.event, room_id: membershipEvent.roomId, state_key: membershipEvent.stateKey, - content: { membership: 'join' }, + content: membershipEvent.event.content, sender: membershipEvent.sender, - origin_server_ts: Date.now(), + origin_server_ts: membershipEvent.event.origin_server_ts, }); }📝 Committable suggestion
🤖 Prompt for AI Agents