Fix out of order edit message read syncs

ts/messageModifiers/ReadSyncs.ts

@@ -6,6 +6,7 @@ import type { MessageModel } from '../models/messages';
 import * as Errors from '../types/errors';
 import * as log from '../logging/log';
 import { StartupQueue } from '../util/StartupQueue';
+import { drop } from '../util/drop';
 import { getMessageIdForLogging } from '../util/idForLogging';
 import { getMessageSentTimestamp } from '../util/getMessageSentTimestamp';
 import { isIncoming } from '../state/selectors/message';
@@ -41,13 +42,10 @@ async function maybeItIsAReactionReadSync(
   );
 
   if (!readReaction) {
-    log.info(`${logId}: ReadSync-3 ${sync.envelopeId}`);
     log.info(`${logId} not found:`, sync.senderId, sync.sender, sync.senderAci);
     return;
   }
 
-  log.info(`${logId}: ReadSync-4 ${sync.envelopeId}`);
-
   remove(sync);
 
   notificationService.removeBy({
@@ -93,8 +91,6 @@ export async function onSync(sync: ReadSyncAttributesType): Promise<void> {
 
   const logId = `ReadSyncs.onSync(timestamp=${sync.timestamp})`;
 
-  log.info(`${logId}: ReadSync-1 ${sync.envelopeId}`);
-
   try {
     const messages = await window.Signal.Data.getMessagesBySentAt(
       sync.timestamp
@@ -111,13 +107,10 @@ export async function onSync(sync: ReadSyncAttributesType): Promise<void> {
     });
 
     if (!found) {
-      log.info(`${logId}: ReadSync-2 ${sync.envelopeId}`);
       await maybeItIsAReactionReadSync(sync);
       return;
     }
 
-    log.info(`${logId}: ReadSync-5 ${sync.envelopeId}`);
-
     notificationService.removeBy({ messageId: found.id });
 
     const message = window.MessageController.register(found.id, found);
@@ -127,24 +120,20 @@ export async function onSync(sync: ReadSyncAttributesType): Promise<void> {
     //   timer to the time specified by the read sync if it's earlier than
     //   the previous read time.
     if (isMessageUnread(message.attributes)) {
-      log.info(`${logId}: ReadSync-6 ${sync.envelopeId}`);
       // TODO DESKTOP-1509: use MessageUpdater.markRead once this is TS
       message.markRead(readAt, { skipSave: true });
 
       const updateConversation = async () => {
-        log.info(`${logId}: ReadSync-7 ${sync.envelopeId}`);
         // onReadMessage may result in messages older than this one being
         //   marked read. We want those messages to have the same expire timer
         //   start time as this one, so we pass the readAt value through.
-        void message.getConversation()?.onReadMessage(message, readAt);
+        drop(message.getConversation()?.onReadMessage(message, readAt));
       };
 
       // only available during initialization
       if (StartupQueue.isAvailable()) {
-        log.info(`${logId}: ReadSync-8 ${sync.envelopeId}`);
         const conversation = message.getConversation();
         if (conversation) {
-          log.info(`${logId}: ReadSync-9 ${sync.envelopeId}`);
           StartupQueue.add(
             conversation.get('id'),
             message.get('sent_at'),
@@ -152,13 +141,11 @@ export async function onSync(sync: ReadSyncAttributesType): Promise<void> {
           );
         }
       } else {
-        log.info(`${logId}: ReadSync-10 ${sync.envelopeId}`);
         // not awaiting since we don't want to block work happening in the
         // eventHandlerQueue
-        void updateConversation();
+        drop(updateConversation());
       }
     } else {
-      log.info(`${logId}: ReadSync-11 ${sync.envelopeId}`);
       const now = Date.now();
       const existingTimestamp = message.get('expirationStartTimestamp');
       const expirationStartTimestamp = Math.min(
@@ -168,12 +155,10 @@ export async function onSync(sync: ReadSyncAttributesType): Promise<void> {
       message.set({ expirationStartTimestamp });
     }
 
-    log.info(`${logId}: ReadSync-12 ${sync.envelopeId}`);
     queueUpdateMessage(message.attributes);
 
     remove(sync);
   } catch (error) {
-    log.info(`${logId}: ReadSync-13 ${sync.envelopeId}`);
     remove(sync);
     log.error(`${logId} error:`, Errors.toLogFormat(error));
   }

ts/models/messages.ts

@@ -2915,6 +2915,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
     return modifyTargetMessage(this, conversation, {
       isFirstRun,
       skipEdits: false,
+      skipSave: false,
     });
   }
 

ts/util/handleEditMessage.ts

@@ -326,8 +326,9 @@ export async function handleEditMessage(
     drop(mainMessageConversation.updateLastMessage());
     // Apply any other operations, excluding edits that target this message
     await modifyTargetMessage(mainMessageModel, mainMessageConversation, {
-      isFirstRun: true,
+      isFirstRun: false,
       skipEdits: true,
+      skipSave: true,
     });
   }
 

ts/util/modifyTargetMessage.ts

@@ -34,9 +34,13 @@ import { strictAssert } from './assert';
 export async function modifyTargetMessage(
   message: MessageModel,
   conversation: ConversationModel,
-  options?: { isFirstRun: boolean; skipEdits: boolean }
+  options?: { isFirstRun: boolean; skipEdits: boolean; skipSave: boolean }
 ): Promise<void> {
-  const { isFirstRun = false, skipEdits = false } = options ?? {};
+  const {
+    isFirstRun = false,
+    skipEdits = false,
+    skipSave = false,
+  } = options ?? {};
 
   const logId = `modifyTargetMessage/${message.idForLogging()}`;
   const type = message.get('type');
@@ -111,13 +115,10 @@ export async function modifyTargetMessage(
 
     const viewSyncs = ViewSyncs.forMessage(message);
 
-    log.info(`${logId}: ReadSync-1`, { length: readSyncs.length });
-
     const isGroupStoryReply =
       isGroup(conversation.attributes) && message.get('storyId');
 
     if (readSyncs.length !== 0 || viewSyncs.length !== 0) {
-      log.info(`${logId}: ReadSync-2`);
       const markReadAt = Math.min(
         Date.now(),
         ...readSyncs.map(sync => sync.readAt),
@@ -152,7 +153,6 @@ export async function modifyTargetMessage(
       });
       changed = true;
 
-      log.info(`${logId}: ReadSync-3`);
       message.setPendingMarkRead(
         Math.min(message.getPendingMarkRead() ?? Date.now(), markReadAt)
       );
@@ -161,12 +161,10 @@ export async function modifyTargetMessage(
       !isGroupStoryReply &&
       canConversationBeUnarchived(conversation.attributes)
     ) {
-      log.info(`${logId}: ReadSync-4`);
       conversation.setArchived(false);
     }
 
     if (!isFirstRun && message.getPendingMarkRead()) {
-      log.info(`${logId}: ReadSync-5`);
       const markReadAt = message.getPendingMarkRead();
       message.setPendingMarkRead(undefined);
 
@@ -263,7 +261,7 @@ export async function modifyTargetMessage(
     );
   }
 
-  if (changed && !isFirstRun) {
+  if (!skipSave && changed && !isFirstRun) {
     log.info(`${logId}: Changes in second run; saving.`);
     await window.Signal.Data.saveMessage(message.attributes, {
       ourAci,