diff --git a/web/packages/base/http.ts b/web/packages/base/http.ts index de896319dc..6ce7fb6e1f 100644 --- a/web/packages/base/http.ts +++ b/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 ( * 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, + opts?: HTTPRequestRetrierOpts, ) => Promise; +type HTTPRequestRetrierOpts = Pick; + /** * A helper function to adapt {@link retryAsyncOperation} for HTTP fetches. * @@ -248,12 +263,13 @@ export type HTTPRequestRetrier = ( */ export const retryEnsuringHTTPOk: HTTPRequestRetrier = ( request: () => Promise, + 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, + opts?: HTTPRequestRetrierOpts, ) => retryAsyncOperation( async () => { @@ -272,6 +289,7 @@ export const retryEnsuringHTTPOkOr4xx: HTTPRequestRetrier = ( return r; }, { + ...opts, abortIfNeeded(e) { if (isHTTP4xxError(e)) throw e; }, diff --git a/web/packages/gallery/services/file-data.ts b/web/packages/gallery/services/file-data.ts index c28ad9aea0..a811129fe8 100644 --- a/web/packages/gallery/services/file-data.ts +++ b/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); -}; diff --git a/web/packages/gallery/services/upload/upload-service.ts b/web/packages/gallery/services/upload/upload-service.ts index 08a6b181e8..26a0082bf0 100644 --- a/web/packages/gallery/services/upload/upload-service.ts +++ b/web/packages/gallery/services/upload/upload-service.ts @@ -1604,7 +1604,7 @@ const uploadToBucket = async ( */ const createAbortableRetryEnsuringHTTPOk = (abortIfCancelled: () => void): HTTPRequestRetrier => - (request: () => Promise) => + (request, opts) => retryAsyncOperation( async () => { abortIfCancelled(); @@ -1613,6 +1613,7 @@ const createAbortableRetryEnsuringHTTPOk = return r; }, { + ...opts, abortIfNeeded(e) { if ( e instanceof Error && diff --git a/web/packages/gallery/services/video.ts b/web/packages/gallery/services/video.ts index 1c02011467..726664735e 100644 --- a/web/packages/gallery/services/video.ts +++ b/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);