Ensure multiple draft attachment adds don't stomp on each other

Co-authored-by: Scott Nonnenberg <scott@signal.org>

ts/state/ducks/composer.ts

@@ -13,6 +13,7 @@ import { assignWithNoUnnecessaryAllocation } from '../../util/assignWithNoUnnece
 import type { RemoveLinkPreviewActionType } from './linkPreviews';
 import { REMOVE_PREVIEW as REMOVE_LINK_PREVIEW } from './linkPreviews';
 import { writeDraftAttachment } from '../../util/writeDraftAttachment';
+import { deleteDraftAttachment } from '../../util/deleteDraftAttachment';
 import { replaceIndex } from '../../util/replaceIndex';
 import { resolveAttachmentOnDisk } from '../../util/resolveAttachmentOnDisk';
 import type { HandleAttachmentsProcessingArgsType } from '../../util/handleAttachmentsProcessing';
@@ -108,6 +109,10 @@ function addAttachment(
   attachment: AttachmentType
 ): ThunkAction<void, RootStateType, unknown, ReplaceAttachmentsActionType> {
   return async (dispatch, getState) => {
+    // We do async operations first so multiple in-process addAttachments don't stomp on
+    //   each other.
+    const onDisk = await writeDraftAttachment(attachment);
+
     const isSelectedConversation =
       getState().conversations.selectedConversationId === conversationId;
 
@@ -122,11 +127,10 @@ function addAttachment(
 
     // User has canceled the draft so we don't need to continue processing
     if (!hasDraftAttachmentPending) {
+      await deleteDraftAttachment(onDisk);
       return;
     }
 
-    const onDisk = await writeDraftAttachment(attachment);
-
     // Remove any pending attachments that were transcoding
     const index = draftAttachments.findIndex(
       draftAttachment => draftAttachment.path === attachment.path
@@ -198,9 +202,16 @@ function removeAttachment(
   conversationId: string,
   filePath: string
 ): ThunkAction<void, RootStateType, unknown, ReplaceAttachmentsActionType> {
-  return (dispatch, getState) => {
+  return async (dispatch, getState) => {
     const { attachments } = getState().composer;
 
+    const [targetAttachment] = attachments.filter(
+      attachment => attachment.path === filePath
+    );
+    if (!targetAttachment) {
+      return;
+    }
+
     const nextAttachments = attachments.filter(
       attachment => attachment.path !== filePath
     );
@@ -217,6 +228,13 @@ function removeAttachment(
       getState,
       null
     );
+
+    if (
+      targetAttachment.path &&
+      targetAttachment.fileName !== targetAttachment.path
+    ) {
+      await deleteDraftAttachment(targetAttachment);
+    }
   };
 }
 

ts/util/handleAttachmentsProcessing.ts

@@ -8,6 +8,7 @@ import {
 } from './processAttachment';
 import type { AttachmentType } from '../types/Attachment';
 import { AttachmentToastType } from '../types/AttachmentToastType';
+import * as log from '../logging/log';
 
 export type AddAttachmentActionType = (
   conversationId: string,
@@ -74,6 +75,10 @@ export async function handleAttachmentsProcessing({
         }
         addAttachment(conversationId, attachment);
       } catch (err) {
+        log.error(
+          'handleAttachmentsProcessing: failed to process attachment:',
+          err.stack
+        );
         removeAttachment(conversationId, file.path);
         onShowToast(AttachmentToastType.ToastUnableToLoadAttachment);
       }