fix: Intermittent error "Cannot read properties of undefined" when editing messages

.changeset/shy-dolphins-share.md

@@ -0,0 +1,5 @@
+---
+"@rocket.chat/meteor": patch
+---
+
+Fixes intermittent error "Cannot read properties of undefined" when editing messages

apps/meteor/app/ui/client/lib/ChatMessages.ts

@@ -2,6 +2,7 @@ import type { IMessage, IRoom, IUser } from '@rocket.chat/core-typings';
 import { isVideoConfMessage } from '@rocket.chat/core-typings';
 import type { IActionManager } from '@rocket.chat/ui-contexts';
 
+import { CurrentEditingMessage } from './CurrentEditingMessage';
 import { UserAction } from './UserAction';
 import type { ChatAPI, ComposerAPI, DataAPI, UploadsAPI } from '../../../../client/lib/chats/ChatAPI';
 import { createDataAPI } from '../../../../client/lib/chats/data';
@@ -15,10 +16,7 @@ import { sendMessage } from '../../../../client/lib/chats/flows/sendMessage';
 import { uploadFiles } from '../../../../client/lib/chats/flows/uploadFiles';
 import { ReadStateManager } from '../../../../client/lib/chats/readStateManager';
 import { createUploadsAPI } from '../../../../client/lib/chats/uploads';
-import {
-	setHighlightMessage,
-	clearHighlightMessage,
-} from '../../../../client/views/room/MessageList/providers/messageHighlightSubscription';
+import { setHighlightMessage } from '../../../../client/views/room/MessageList/providers/messageHighlightSubscription';
 
 type DeepWritable<T> = T extends (...args: any) => any
 	? T
@@ -29,6 +27,8 @@ type DeepWritable<T> = T extends (...args: any) => any
 export class ChatMessages implements ChatAPI {
 	public uid: string | null;
 
+	public tmid?: IMessage['_id'];
+
 	public composer: ComposerAPI | undefined;
 
 	public setComposerAPI = (composer?: ComposerAPI): void => {
@@ -38,6 +38,8 @@ export class ChatMessages implements ChatAPI {
 
 	public data: DataAPI;
 
+	public currentEditingMessage: CurrentEditingMessage;
+
 	public readStateManager: ReadStateManager;
 
 	public uploads: UploadsAPI;
@@ -55,15 +57,15 @@ export class ChatMessages implements ChatAPI {
 		performContinuously(action: 'recording' | 'uploading' | 'playing'): Promise<void> | void;
 	};
 
-	private currentEditingMID?: string;
-
 	public messageEditing: ChatAPI['messageEditing'] = {
 		toPreviousMessage: async () => {
 			if (!this.composer) {
 				return;
 			}
 
-			if (!this.currentEditing) {
+			const mid = this.currentEditingMessage.getMID();
+
+			if (!mid) {
 				let lastMessage = await this.data.findPreviousOwnMessage();
 
 				// Videoconf messages should not be edited
@@ -79,7 +81,7 @@ export class ChatMessages implements ChatAPI {
 				return;
 			}
 
-			const currentMessage = await this.data.findMessageByID(this.currentEditing.mid);
+			const currentMessage = await this.data.findMessageByID(mid);
 			let previousMessage = currentMessage ? await this.data.findPreviousOwnMessage(currentMessage) : undefined;
 
 			// Videoconf messages should not be edited
@@ -92,14 +94,17 @@ export class ChatMessages implements ChatAPI {
 				return;
 			}
 
-			await this.currentEditing.cancel();
+			await this.currentEditingMessage.cancel();
+			await this.currentEditingMessage.stop();
 		},
 		toNextMessage: async () => {
-			if (!this.composer || !this.currentEditing) {
+			const mid = this.currentEditingMessage.getMID();
+
+			if (!this.composer || !mid) {
 				return;
 			}
 
-			const currentMessage = await this.data.findMessageByID(this.currentEditing.mid);
+			const currentMessage = await this.data.findMessageByID(mid);
 			let nextMessage = currentMessage ? await this.data.findNextOwnMessage(currentMessage) : undefined;
 
 			// Videoconf messages should not be edited
@@ -112,18 +117,19 @@ export class ChatMessages implements ChatAPI {
 				return;
 			}
 
-			await this.currentEditing.cancel();
+			await this.currentEditingMessage.cancel();
+			await this.currentEditingMessage.stop();
 		},
 		editMessage: async (message: IMessage, { cursorAtStart = false }: { cursorAtStart?: boolean } = {}) => {
 			const text = (await this.data.getDraft(message._id)) || message.attachments?.[0]?.description || message.msg;
 
-			await this.currentEditing?.stop();
+			await this.currentEditingMessage.stop();
 
 			if (!this.composer || !(await this.data.canUpdateMessage(message))) {
 				return;
 			}
 
-			this.currentEditingMID = message._id;
+			this.currentEditingMessage.setMID(message._id);
 			setHighlightMessage(message._id);
 			this.composer.setEditingMode(true);
 
@@ -136,19 +142,14 @@ export class ChatMessages implements ChatAPI {
 
 	public flows: DeepWritable<ChatAPI['flows']>;
 
-	public constructor(
-		private params: {
-			rid: IRoom['_id'];
-			tmid?: IMessage['_id'];
-			uid: IUser['_id'] | null;
-			actionManager: IActionManager;
-		},
-	) {
+	public constructor(params: { rid: IRoom['_id']; tmid?: IMessage['_id']; uid: IUser['_id'] | null; actionManager: IActionManager }) {
 		const { rid, tmid } = params;
+		this.tmid = tmid;
 		this.uid = params.uid;
 		this.data = createDataAPI({ rid, tmid });
 		this.uploads = createUploadsAPI({ rid, tmid });
 		this.ActionManager = params.actionManager;
+		this.currentEditingMessage = new CurrentEditingMessage(this);
 
 		const unimplemented = () => {
 			throw new Error('Flow is not implemented');
@@ -185,60 +186,11 @@ export class ChatMessages implements ChatAPI {
 		};
 	}
 
-	public get currentEditing() {
-		if (!this.composer || !this.currentEditingMID) {
-			return undefined;
-		}
-
-		return {
-			mid: this.currentEditingMID,
-			reset: async (): Promise<boolean> => {
-				if (!this.composer || !this.currentEditingMID) {
-					return false;
-				}
-
-				const message = await this.data.findMessageByID(this.currentEditingMID);
-				if (this.composer.text !== message?.msg) {
-					this.composer.setText(message?.msg ?? '');
-					return true;
-				}
-
-				return false;
-			},
-			stop: async (): Promise<void> => {
-				if (!this.composer || !this.currentEditingMID) {
-					return;
-				}
-
-				const message = await this.data.findMessageByID(this.currentEditingMID);
-				const draft = this.composer.text;
-
-				if (draft === message?.msg) {
-					await this.data.discardDraft(this.currentEditingMID);
-				} else {
-					await this.data.saveDraft(this.currentEditingMID, (await this.data.getDraft(this.currentEditingMID)) || draft);
-				}
-
-				this.composer.setEditingMode(false);
-				this.currentEditingMID = undefined;
-				clearHighlightMessage();
-			},
-			cancel: async (): Promise<void> => {
-				if (!this.currentEditingMID) {
-					return;
-				}
-
-				await this.data.discardDraft(this.currentEditingMID);
-				await this.currentEditing?.stop();
-				this.composer?.setText((await this.data.getDraft(undefined)) ?? '');
-			},
-		};
-	}
-
 	public async release() {
-		if (this.currentEditing) {
-			if (!this.params.tmid) {
-				await this.currentEditing.cancel();
+		if (this.currentEditingMessage.getMID()) {
+			if (!this.tmid) {
+				await this.currentEditingMessage.cancel();
+				await this.currentEditingMessage.stop();
 			}
 			this.composer?.clear();
 		}

apps/meteor/app/ui/client/lib/CurrentEditingMessage.ts

@@ -0,0 +1,114 @@
+import type { ChatAPI } from '../../../../client/lib/chats/ChatAPI';
+import { clearHighlightMessage } from '../../../../client/views/room/MessageList/providers/messageHighlightSubscription';
+
+export class CurrentEditingMessage {
+	private lock = false;
+
+	private mid?: string;
+
+	private queue: {
+		resolve: (release: () => void) => void;
+	}[] = [];
+
+	private chat: ChatAPI;
+
+	constructor(chat: ChatAPI) {
+		this.chat = chat;
+	}
+
+	public reset = async () => {
+		return this.runExclusive(async () => {
+			if (!this.chat.composer || !this.mid) {
+				return false;
+			}
+
+			const message = await this.chat.data.findMessageByID(this.mid);
+
+			if (this.chat.composer.text !== message?.msg) {
+				this.chat.composer.setText(message?.msg ?? '');
+				return true;
+			}
+
+			return false;
+		});
+	};
+
+	public stop = async () => {
+		await this.runExclusive(async () => {
+			if (!this.chat.composer || !this.mid) {
+				return;
+			}
+
+			const message = await this.chat.data.findMessageByID(this.mid);
+			const draft = this.chat.composer.text;
+
+			if (draft === message?.msg) {
+				await this.chat.data.discardDraft(this.mid);
+			} else {
+				await this.chat.data.saveDraft(this.mid, (await this.chat.data.getDraft(this.mid)) || draft);
+			}
+
+			this.chat.composer.setEditingMode(false);
+			this.mid = undefined;
+			clearHighlightMessage();
+		});
+	};
+
+	public cancel = async () => {
+		await this.runExclusive(async () => {
+			if (!this.mid) {
+				return;
+			}
+
+			await this.chat.data.discardDraft(this.mid);
+			this.chat.composer?.setText((await this.chat.data.getDraft(undefined)) ?? '');
+		});
+	};
+
+	private acquire = async () => {
+		return new Promise<() => void>((resolve) => {
+			this.queue.push({ resolve });
+			this.dispatch();
+		});
+	};
+
+	private dispatch() {
+		if (this.lock) {
+			return;
+		}
+
+		const next = this.queue.shift();
+
+		if (!next) {
+			return;
+		}
+
+		this.lock = true;
+		next.resolve(this.buildRelease());
+	}
+
+	private buildRelease = () => {
+		return async () => {
+			this.lock = false;
+			this.dispatch();
+		};
+	};
+
+	public getMID() {
+		return this.mid;
+	}
+
+	public setMID(mid: string) {
+		this.mid = mid;
+	}
+
+	private async runExclusive<T>(callback: () => Promise<T>) {
+		const release = await this.acquire();
+
+		try {
+			return await callback();
+		} finally {
+			release();
+		}
+	}
+}

apps/meteor/client/lib/chats/ChatAPI.ts

@@ -118,14 +118,13 @@ export type ChatAPI = {
 		editMessage(message: IMessage, options?: { cursorAtStart?: boolean }): Promise<void>;
 	};
 
-	readonly currentEditing:
-		| {
-				readonly mid: IMessage['_id'];
-				reset(): Promise<boolean>;
-				stop(): Promise<void>;
-				cancel(): Promise<void>;
-		  }
-		| undefined;
+	readonly currentEditingMessage: {
+		setMID(mid: IMessage['_id']): void;
+		getMID(): string | undefined;
+		reset(): Promise<boolean>;
+		stop(): Promise<void>;
+		cancel(): Promise<void>;
+	};
 
 	readonly emojiPicker: {
 		open(el: Element, cb: (emoji: string) => void): void;

apps/meteor/client/lib/chats/data.ts

@@ -176,7 +176,7 @@ export const createDataAPI = ({ rid, tmid }: { rid: IRoom['_id']; tmid: IMessage
 	};
 
 	const updateMessage = async (message: IEditedMessage, previewUrls?: string[]): Promise<void> =>
-		sdk.call('updateMessage', message, previewUrls);
+		void sdk.call('updateMessage', message, previewUrls);
 
 	const canDeleteMessage = async (message: IMessage): Promise<boolean> => {
 		const uid = Meteor.userId();

apps/meteor/client/lib/chats/flows/processMessageEditing.ts

@@ -9,7 +9,8 @@ export const processMessageEditing = async (
 	message: Pick<IMessage, '_id' | 't'> & Partial<Omit<IMessage, '_id' | 't'>>,
 	previewUrls?: string[],
 ): Promise<boolean> => {
-	if (!chat.currentEditing) {
+	const mid = chat.currentEditingMessage.getMID();
+	if (!mid) {
 		return false;
 	}
 
@@ -22,12 +23,12 @@ export const processMessageEditing = async (
 	}
 
 	try {
-		await chat.data.updateMessage({ ...message, _id: chat.currentEditing.mid }, previewUrls);
+		await chat.data.updateMessage({ ...message, _id: mid }, previewUrls);
 	} catch (error) {
 		dispatchToastMessage({ type: 'error', message: error });
 	}
 
-	chat.currentEditing.stop();
+	chat.currentEditingMessage.stop();
 
 	return true;
 };

apps/meteor/client/lib/chats/flows/processTooLongMessage.ts

@@ -15,7 +15,7 @@ export const processTooLongMessage = async (chat: ChatAPI, { msg }: Pick<IMessag
 	const fileUploadsEnabled = settings.get('FileUpload_Enabled');
 	const convertLongMessagesToAttachment = settings.get('Message_AllowConvertLongMessagesToAttachment');
 
-	if (chat.currentEditing || !fileUploadsEnabled || !convertLongMessagesToAttachment) {
+	if (chat.currentEditingMessage.getMID() || !fileUploadsEnabled || !convertLongMessagesToAttachment) {
 		dispatchToastMessage({ type: 'error', message: new Error(t('Message_too_long')) });
 		chat.composer?.setText(msg);
 		return true;