[desktop] Make video upload retries idempotent (#6131)

web/packages/base/http.ts

@@ -193,6 +193,16 @@ interface RetryAsyncOperationOpts {
  * Retry a async operation on failure up to 4 times (1 original + 3 retries)
  * with exponential backoff.
  *
+ * [Note: Retries of network requests should be idempotent]
+ *
+ * When dealing with network requests, avoid using this function directly, use
+ * one of its wrappers like {@link retryEnsuringHTTPOk} instead. Those wrappers
+ * ultimately use this function only, and there is nothing wrong with this
+ * function generally, however since this function allows retrying arbitrary
+ * promises, it is easy accidentally try and attempt retries of non-idemponent
+ * requests, while the more restricted API of {@link retryEnsuringHTTPOk} and
+ * other {@link HTTPRequestRetrier}s makes such misuse less likely.
+ *
  * @param op A function that performs the operation, returning the promise for
  * its completion.
  *
@@ -235,11 +245,16 @@ export const retryAsyncOperation = async <T>(
  * See {@link retryEnsuringHTTPOk} for the canonical example. This typedef is to
  * allow us to talk about and pass functions that behave similar to
  * {@link retryEnsuringHTTPOk}, but perhaps with other additional checks.
+ *
+ * See also: [Note: Retries of network requests should be idempotent]
  */
 export type HTTPRequestRetrier = (
     request: () => Promise<Response>,
+    opts?: HTTPRequestRetrierOpts,
 ) => Promise<Response>;
 
+type HTTPRequestRetrierOpts = Pick<RetryAsyncOperationOpts, "retryProfile">;
+
 /**
  * A helper function to adapt {@link retryAsyncOperation} for HTTP fetches.
  *
@@ -248,12 +263,13 @@ export type HTTPRequestRetrier = (
  */
 export const retryEnsuringHTTPOk: HTTPRequestRetrier = (
     request: () => Promise<Response>,
+    opts?: HTTPRequestRetrierOpts,
 ) =>
     retryAsyncOperation(async () => {
         const r = await request();
         ensureOk(r);
         return r;
-    });
+    }, opts);
 
 /**
  * A helper function to adapt {@link retryAsyncOperation} for HTTP fetches, but
@@ -264,6 +280,7 @@ export const retryEnsuringHTTPOk: HTTPRequestRetrier = (
  */
 export const retryEnsuringHTTPOkOr4xx: HTTPRequestRetrier = (
     request: () => Promise<Response>,
+    opts?: HTTPRequestRetrierOpts,
 ) =>
     retryAsyncOperation(
         async () => {
@@ -272,6 +289,7 @@ export const retryEnsuringHTTPOkOr4xx: HTTPRequestRetrier = (
             return r;
         },
         {
+            ...opts,
             abortIfNeeded(e) {
                 if (isHTTP4xxError(e)) throw e;
             },

web/packages/gallery/services/file-data.ts

@@ -1,8 +1,10 @@
 import { encryptBlobB64 } from "ente-base/crypto";
+import type { EncryptedBlobB64 } from "ente-base/crypto/types";
 import {
     authenticatedPublicAlbumsRequestHeaders,
     authenticatedRequestHeaders,
     ensureOk,
+    retryEnsuringHTTPOk,
     type PublicAlbumsCredentials,
 } from "ente-base/http";
 import { apiURL } from "ente-base/origins";
@@ -394,8 +396,8 @@ export const fetchFilePreviewData = async (
  *
  * @param file {@link EnteFile} which this data is associated with.
  *
- * @param playlistData The playlist data, suitably encoded in a form ready for
- * encryption.
+ * @param encryptedPlaylist The encrypted playlist data (along with the nonce
+ * used during encryption) encryption.
  *
  * @param objectID Object ID of an already uploaded "file preview data" (see
  * {@link getFilePreviewDataUploadURL}).
@@ -405,25 +407,22 @@ export const fetchFilePreviewData = async (
  */
 export const putVideoData = async (
     file: EnteFile,
-    playlistData: Uint8Array,
+    encryptedPlaylist: EncryptedBlobB64,
     objectID: string,
     objectSize: number,
-) => {
-    const { encryptedData, decryptionHeader } = await encryptBlobB64(
-        playlistData,
-        file.key,
+) =>
+    retryEnsuringHTTPOk(
+        async () =>
+            fetch(await apiURL("/files/video-data"), {
+                method: "PUT",
+                headers: await authenticatedRequestHeaders(),
+                body: JSON.stringify({
+                    fileID: file.id,
+                    objectID,
+                    objectSize,
+                    playlist: encryptedPlaylist.encryptedData,
+                    playlistHeader: encryptedPlaylist.decryptionHeader,
+                }),
+            }),
+        { retryProfile: "background" },
     );
-
-    const res = await fetch(await apiURL("/files/video-data"), {
-        method: "PUT",
-        headers: await authenticatedRequestHeaders(),
-        body: JSON.stringify({
-            fileID: file.id,
-            objectID,
-            objectSize,
-            playlist: encryptedData,
-            playlistHeader: decryptionHeader,
-        }),
-    });
-    ensureOk(res);
-};

web/packages/gallery/services/upload/upload-service.ts

@@ -1604,7 +1604,7 @@ const uploadToBucket = async (
  */
 const createAbortableRetryEnsuringHTTPOk =
     (abortIfCancelled: () => void): HTTPRequestRetrier =>
-    (request: () => Promise<Response>) =>
+    (request, opts) =>
         retryAsyncOperation(
             async () => {
                 abortIfCancelled();
@@ -1613,6 +1613,7 @@ const createAbortableRetryEnsuringHTTPOk =
                 return r;
             },
             {
+                ...opts,
                 abortIfNeeded(e) {
                     if (
                         e instanceof Error &&

web/packages/gallery/services/video.ts

@@ -1,13 +1,9 @@
 import { isDesktop } from "ente-base/app";
 import { assertionFailed } from "ente-base/assert";
-import { decryptBlob } from "ente-base/crypto";
+import { decryptBlob, encryptBlobB64 } from "ente-base/crypto";
 import type { EncryptedBlob } from "ente-base/crypto/types";
 import { ensureElectron } from "ente-base/electron";
-import {
-    isHTTP4xxError,
-    retryAsyncOperation,
-    type PublicAlbumsCredentials,
-} from "ente-base/http";
+import { isHTTP4xxError, type PublicAlbumsCredentials } from "ente-base/http";
 import { getKV, getKVB, getKVN, setKV } from "ente-base/kv";
 import { ensureAuthToken, ensureLocalUser } from "ente-base/local-user";
 import log from "ente-base/log";
@@ -1060,11 +1056,14 @@ const processQueueItem = async ({
             size: videoSize,
         });
 
+        const encryptedPlaylist = await encryptBlobB64(playlistData, file.key);
+
         try {
-            await retryAsyncOperation(
-                () =>
-                    putVideoData(file, playlistData, videoObjectID, videoSize),
-                { retryProfile: "background" },
+            await putVideoData(
+                file,
+                encryptedPlaylist,
+                videoObjectID,
+                videoSize,
             );
         } catch (e) {
             if (isHTTP4xxError(e)) await markFailedVideoFile(file);