Allow backfill for more undownloadable attachments

ts/jobs/AttachmentDownloadManager.ts

@@ -12,10 +12,7 @@ import {
   AttachmentDownloadUrgency,
   coreAttachmentDownloadJobSchema,
 } from '../types/AttachmentDownload';
-import {
-  AttachmentPermanentlyUndownloadableError,
-  downloadAttachment as downloadAttachmentUtil,
-} from '../util/downloadAttachment';
+import { downloadAttachment as downloadAttachmentUtil } from '../util/downloadAttachment';
 import { DataWriter } from '../sql/Client';
 import { getValue } from '../RemoteConfig';
 
@@ -24,6 +21,7 @@ import {
   AttachmentSizeError,
   type AttachmentType,
   AttachmentVariant,
+  AttachmentPermanentlyUndownloadableError,
   mightBeOnBackupTier,
 } from '../types/Attachment';
 import { type ReadonlyMessageAttributesType } from '../model-types.d';
@@ -61,6 +59,7 @@ import { markAttachmentAsPermanentlyErrored } from '../util/attachments/markAtta
 import {
   AttachmentBackfill,
   isPermanentlyUndownloadable,
+  isPermanentlyUndownloadableWithoutBackfill,
 } from './helpers/attachmentBackfill';
 
 export { isPermanentlyUndownloadable };
@@ -365,6 +364,16 @@ async function runDownloadAttachmentJob({
   try {
     log.info(`${logId}: Starting job`);
 
+    if (
+      job.source !== AttachmentDownloadSource.BACKFILL &&
+      isPermanentlyUndownloadableWithoutBackfill(job.attachment)
+    ) {
+      // We should only get to here only if
+      throw new AttachmentPermanentlyUndownloadableError(
+        'Not downloadable without backfill'
+      );
+    }
+
     const result = await runDownloadAttachmentJobInner({
       job,
       abortSignal: options.abortSignal,

ts/jobs/helpers/attachmentBackfill.ts

@@ -113,17 +113,6 @@ export class AttachmentBackfill {
       AttachmentData: { Status },
     } = Proto.SyncMessage.AttachmentBackfillResponse;
 
-    if ('error' in response) {
-      if (response.error === ErrorEnum.MESSAGE_NOT_FOUND) {
-        window.reduxActions.globalModals.showBackfillFailureModal({
-          kind: BackfillFailureKind.NotFound,
-        });
-      } else {
-        throw missingCaseError(response.error);
-      }
-      return;
-    }
-
     const { targetMessage, targetConversation } = response;
 
     const convo = getConversationFromTarget(targetConversation);
@@ -141,13 +130,29 @@ export class AttachmentBackfill {
 
     const message = window.MessageCache.register(new MessageModel(attributes));
 
-    // IMPORTANT: no awaits until we finish modifying attachments
-
     const timer = this.#pendingRequests.get(message.id);
     if (timer != null) {
       clearTimeout(timer);
     }
 
+    if ('error' in response) {
+      // Don't show error if we didn't request the data or already timed out
+      if (timer == null) {
+        return;
+      }
+
+      if (response.error === ErrorEnum.MESSAGE_NOT_FOUND) {
+        window.reduxActions.globalModals.showBackfillFailureModal({
+          kind: BackfillFailureKind.NotFound,
+        });
+      } else {
+        throw missingCaseError(response.error);
+      }
+      return;
+    }
+
+    // IMPORTANT: no awaits until we finish modifying attachments
+
     // Since we are matching remote attachments with local attachments we need
     // to make sure things are normalized before starting.
     message.set(
@@ -463,3 +468,24 @@ export function isPermanentlyUndownloadable(
   // at this time.
   return !AttachmentBackfill.isEnabledForJob(disposition, message);
 }
+
+export function isPermanentlyUndownloadableWithoutBackfill(
+  attachment: AttachmentType
+): boolean {
+  // Attachment is downloadable or user have not failed to download it yet
+  if (isDownloadable(attachment) || !attachment.error) {
+    return false;
+  }
+
+  // Too big attachments cannot be retried anymore
+  if (attachment.wasTooBig) {
+    return true;
+  }
+
+  // Previous backfill failed
+  if (attachment.backfillError) {
+    return true;
+  }
+
+  return true;
+}

ts/test-electron/util/downloadAttachment_test.ts

@@ -7,16 +7,16 @@ import { noop } from 'lodash';
 
 import { DataWriter } from '../../sql/Client';
 import { IMAGE_PNG } from '../../types/MIME';
-import {
-  AttachmentPermanentlyUndownloadableError,
-  downloadAttachment,
-} from '../../util/downloadAttachment';
+import { downloadAttachment } from '../../util/downloadAttachment';
 import { MediaTier } from '../../types/AttachmentDownload';
 import { HTTPError } from '../../textsecure/Errors';
 import { getCdnNumberForBackupTier } from '../../textsecure/downloadAttachment';
 import { MASTER_KEY, MEDIA_ROOT_KEY } from '../backup/helpers';
 import { getMediaIdFromMediaName } from '../../services/backups/util/mediaId';
-import { AttachmentVariant } from '../../types/Attachment';
+import {
+  AttachmentVariant,
+  AttachmentPermanentlyUndownloadableError,
+} from '../../types/Attachment';
 
 describe('utils/downloadAttachment', () => {
   const baseAttachment = {

ts/textsecure/MessageReceiver.ts

@@ -3825,6 +3825,16 @@ export default class MessageReceiver
       logId
     );
 
+    strictAssert(
+      targetMessage != null,
+      'MessageReceiver.handleAttachmentBackfillResponse: invalid target message'
+    );
+    strictAssert(
+      targetConversation != null,
+      'MessageReceiver.handleAttachmentBackfillResponse: ' +
+        'invalid target conversation'
+    );
+
     let eventData: AttachmentBackfillResponseSyncEventData;
     if (response.error != null) {
       eventData = {
@@ -3833,16 +3843,6 @@ export default class MessageReceiver
         targetConversation,
       };
     } else {
-      strictAssert(
-        targetMessage != null,
-        'MessageReceiver.handleAttachmentBackfillResponse: invalid target message'
-      );
-      strictAssert(
-        targetConversation != null,
-        'MessageReceiver.handleAttachmentBackfillResponse: ' +
-          'invalid target conversation'
-      );
-
       const { attachments } = response;
       strictAssert(
         attachments != null,

ts/textsecure/downloadAttachment.ts

@@ -17,6 +17,7 @@ import {
   mightBeOnBackupTier,
   type AttachmentType,
   AttachmentVariant,
+  AttachmentPermanentlyUndownloadableError,
 } from '../types/Attachment';
 import * as Bytes from '../Bytes';
 import {
@@ -115,9 +116,13 @@ export async function downloadAttachment(
 
   const { digest, incrementalMac, chunkSize, key, size } = attachment;
 
-  strictAssert(digest, `${logId}: missing digest`);
-  strictAssert(key, `${logId}: missing key`);
-  strictAssert(isNumber(size), `${logId}: missing size`);
+  try {
+    strictAssert(digest, `${logId}: missing digest`);
+    strictAssert(key, `${logId}: missing key`);
+    strictAssert(isNumber(size), `${logId}: missing size`);
+  } catch (error) {
+    throw new AttachmentPermanentlyUndownloadableError(error.message);
+  }
 
   const mediaTier =
     options?.mediaTier ??

ts/textsecure/messageReceiverEvents.ts

@@ -595,17 +595,18 @@ export type AttachmentBackfillAttachmentType = Readonly<
 >;
 
 export type AttachmentBackfillResponseSyncEventData = Readonly<
-  | {
-      error: Proto.SyncMessage.AttachmentBackfillResponse.Error;
-      targetMessage?: AddressableMessage;
-      targetConversation?: ConversationIdentifier;
-    }
-  | {
-      attachments: ReadonlyArray<AttachmentBackfillAttachmentType>;
-      longText: AttachmentBackfillAttachmentType | undefined;
-      targetMessage: AddressableMessage;
-      targetConversation: ConversationIdentifier;
-    }
+  {
+    targetMessage: AddressableMessage;
+    targetConversation: ConversationIdentifier;
+  } & (
+    | {
+        error: Proto.SyncMessage.AttachmentBackfillResponse.Error;
+      }
+    | {
+        attachments: ReadonlyArray<AttachmentBackfillAttachmentType>;
+        longText: AttachmentBackfillAttachmentType | undefined;
+      }
+  )
 >;
 
 export class AttachmentBackfillResponseSyncEvent extends ConfirmableEvent {

ts/types/Attachment.ts

@@ -1,5 +1,6 @@
 // Copyright 2018 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
+/* eslint-disable max-classes-per-file */
 
 import moment from 'moment';
 import {
@@ -46,6 +47,14 @@ const MIN_HEIGHT = 50;
 
 export class AttachmentSizeError extends Error {}
 
+// Used for downlaods
+
+export class AttachmentPermanentlyUndownloadableError extends Error {
+  constructor(message: string) {
+    super(`AttachmentPermanentlyUndownloadableError: ${message}`);
+  }
+}
+
 type ScreenshotType = Omit<AttachmentType, 'size'> & {
   height: number;
   width: number;

ts/util/downloadAttachment.ts

@@ -5,6 +5,7 @@ import {
   type AttachmentType,
   mightBeOnBackupTier,
   AttachmentVariant,
+  AttachmentPermanentlyUndownloadableError,
   getAttachmentIdForLogging,
 } from '../types/Attachment';
 import { downloadAttachment as doDownloadAttachment } from '../textsecure/downloadAttachment';
@@ -14,8 +15,6 @@ import { HTTPError } from '../textsecure/Errors';
 import { toLogFormat } from '../types/errors';
 import type { ReencryptedAttachmentV2 } from '../AttachmentCrypto';
 
-export class AttachmentPermanentlyUndownloadableError extends Error {}
-
 export async function downloadAttachment({
   attachment,
   options: { variant = AttachmentVariant.Default, onSizeUpdate, abortSignal },
@@ -105,7 +104,7 @@ export async function downloadAttachment({
       error instanceof HTTPError &&
       (error.code === 404 || error.code === 403)
     ) {
-      throw new AttachmentPermanentlyUndownloadableError();
+      throw new AttachmentPermanentlyUndownloadableError(`HTTP ${error.code}`);
     } else {
       throw error;
     }