diff --git a/mobile/lib/utils/share_util.dart b/mobile/lib/utils/share_util.dart index 3ec147798d..694bc6cff6 100644 --- a/mobile/lib/utils/share_util.dart +++ b/mobile/lib/utils/share_util.dart @@ -47,12 +47,6 @@ Future share( pathFutures.add( getFile(file, isOrigin: true).then((fetchedFile) => fetchedFile?.path), ); - if (file.fileType == FileType.livePhoto) { - pathFutures.add( - getFile(file, liveVideo: true) - .then((fetchedFile) => fetchedFile?.path), - ); - } } final paths = await Future.wait(pathFutures); await dialog.hide(); diff --git a/web/packages/base/crypto/worker.ts b/web/packages/base/crypto/worker.ts index 693ade2be1..429451d27c 100644 --- a/web/packages/base/crypto/worker.ts +++ b/web/packages/base/crypto/worker.ts @@ -1,3 +1,4 @@ +import { logUnhandledErrorsAndRejectionsInWorker } from "@/base/log-web"; import { expose } from "comlink"; import type { StateAddress } from "libsodium-wrappers-sumo"; import * as ei from "./ente-impl"; @@ -98,3 +99,5 @@ export class CryptoWorker { } expose(CryptoWorker); + +logUnhandledErrorsAndRejectionsInWorker(); diff --git a/web/packages/base/log-web.ts b/web/packages/base/log-web.ts index 6ff6ed0a04..6cfcba31cb 100644 --- a/web/packages/base/log-web.ts +++ b/web/packages/base/log-web.ts @@ -50,6 +50,31 @@ export const logUnhandledErrorsAndRejections = (attach: boolean) => { } }; +/** + * Attach handlers to log any unhandled exceptions and promise rejections in web + * workers. + * + * This is a variant of {@link logUnhandledErrorsAndRejections} that works in + * web workers. It should be called at the top level of the main worker script. + * + * Note: When I tested this, attaching the onerror handler to the worker outside + * the worker (e.g. when creating it in comlink-worker.ts) worked, but attaching + * the "unhandledrejection" event there did not work. Attaching them to `self` + * (the {@link WorkerGlobal}) worked. + */ +export const logUnhandledErrorsAndRejectionsInWorker = () => { + const handleError = (event: ErrorEvent) => { + log.error("Unhandled error", event.error); + }; + + const handleUnhandledRejection = (event: PromiseRejectionEvent) => { + log.error("Unhandled promise rejection", event.reason); + }; + + self.addEventListener("error", handleError); + self.addEventListener("unhandledrejection", handleUnhandledRejection); +}; + interface LogEntry { timestamp: number; logLine: string; diff --git a/web/packages/gallery/services/download.ts b/web/packages/gallery/services/download.ts index c201928c2a..b36a06cc51 100644 --- a/web/packages/gallery/services/download.ts +++ b/web/packages/gallery/services/download.ts @@ -271,7 +271,9 @@ class DownloadManager { } private downloadThumbnail = async (file: EnteFile) => { - const encryptedData = await this._downloadThumbnail(file); + const encryptedData = await wrapErrors(() => + this._downloadThumbnail(file), + ); const decryptionHeader = file.thumbnail.decryptionHeader; return decryptThumbnail({ encryptedData, decryptionHeader }, file.key); }; @@ -385,13 +387,15 @@ class DownloadManager { ): Promise | null> { log.info(`download attempted for file id ${file.id}`); - const res = await this._downloadFile(file); + const res = await wrapErrors(() => this._downloadFile(file)); if ( file.metadata.fileType === FileType.image || file.metadata.fileType === FileType.livePhoto ) { - const encryptedData = new Uint8Array(await res.arrayBuffer()); + const encryptedData = new Uint8Array( + await wrapErrors(() => res.arrayBuffer()), + ); const decrypted = await decryptStreamBytes( { @@ -431,7 +435,9 @@ class DownloadManager { do { // done is a boolean and value is an Uint8Array. When done // is true value will be empty. - const { done, value } = await reader.read(); + const { done, value } = await wrapErrors(() => + reader.read(), + ); let data: Uint8Array; if (done) { @@ -522,6 +528,52 @@ class DownloadManager { */ export const downloadManager = new DownloadManager(); +/** + * A custom Error that is thrown if a download fails during network I/O. + * + * [Note: Identifying network related errors during download] + * + * We dealing with code that touches the network, we often don't specifically + * care about the specific error - there is a lot that can go wrong when a + * network is involved - but need to identify if an error was in the network + * related phase of an action, since these are usually transient and can be + * dealt with more softly than other errors. + * + * To that end, network related phases of download operations are wrapped in + * catches that intercept the error and wrap it in our custom + * {@link NetworkDownloadError} whose presence can be checked using the + * {@link isNetworkDownloadError} predicate. + */ +export class NetworkDownloadError extends Error { + error: unknown; + + constructor(e: unknown) { + super( + `NetworkDownloadError: ${e instanceof Error ? e.message : String(e)}`, + ); + + // Cargo culted from + // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error#custom_error_types + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + if (Error.captureStackTrace) + Error.captureStackTrace(this, NetworkDownloadError); + + this.error = e; + } +} + +export const isNetworkDownloadError = (e: unknown) => + e instanceof NetworkDownloadError; + +/** + * A helper function to convert all rejections of the given promise {@link op} + * into {@link NetworkDownloadError}s. + */ +const wrapErrors = (op: () => Promise) => + op().catch((e: unknown) => { + throw new NetworkDownloadError(e); + }); + /** * Create and return a {@link RenderableSourceURLs} for the given {@link file}, * where {@link originalFileURLPromise} is a promise that resolves with an diff --git a/web/packages/media/file.ts b/web/packages/media/file.ts index f087dbb17e..9499aa4523 100644 --- a/web/packages/media/file.ts +++ b/web/packages/media/file.ts @@ -2,6 +2,7 @@ import { sharedCryptoWorker } from "@/base/crypto"; import { dateFromEpochMicroseconds } from "@/base/date"; import log from "@/base/log"; import { type Metadata, ItemVisibility } from "./file-metadata"; +import { FileType } from "./file-type"; // TODO: Audit this file. @@ -405,12 +406,19 @@ export const mergeMetadata1 = (file: EnteFile): EnteFile => { } } - // In a very rare cases (have found only one so far, a very old file in + // In very rare cases (have found only one so far, a very old file in // Vishnu's account, uploaded by an initial dev version of Ente) the photo // has no modification time. Gracefully handle such cases. if (!file.metadata.modificationTime) file.metadata.modificationTime = file.metadata.creationTime; + // In very rare cases (again, some files shared with Vishnu's account, + // uploaded by dev builds) the photo might not have a file type. Gracefully + // handle these too. The file ID threshold is an arbitrary cutoff so that + // this graceful handling does not mask new issues. + if (!file.metadata.fileType && file.id < 100000000) + file.metadata.fileType = FileType.image; + return file; }; diff --git a/web/packages/media/heic-convert.worker.ts b/web/packages/media/heic-convert.worker.ts index 52c9d7abba..0209267702 100644 --- a/web/packages/media/heic-convert.worker.ts +++ b/web/packages/media/heic-convert.worker.ts @@ -1,3 +1,4 @@ +import { logUnhandledErrorsAndRejectionsInWorker } from "@/base/log-web"; import { wait } from "@/utils/promise"; import { expose } from "comlink"; import HeicConvert from "heic-convert"; @@ -19,6 +20,8 @@ export class HEICConvertWorker { expose(HEICConvertWorker); +logUnhandledErrorsAndRejectionsInWorker(); + const heicToJPEG = async (heicBlob: Blob): Promise => { const buffer = new Uint8Array(await heicBlob.arrayBuffer()); const result = await HeicConvert({ buffer, format: "JPEG" }); diff --git a/web/packages/new/photos/components/sidebar/MLSettings.tsx b/web/packages/new/photos/components/sidebar/MLSettings.tsx index 143ca9ed2e..567c85b5bd 100644 --- a/web/packages/new/photos/components/sidebar/MLSettings.tsx +++ b/web/packages/new/photos/components/sidebar/MLSettings.tsx @@ -304,7 +304,7 @@ const ManageML: React.FC = ({ mlStatus, onDisableML }) => { {status} - + { // // Fetch indexes, or index locally if needed. - await (await worker()).index(); + await worker().then((w) => w.index()); await updateClustersAndPeople(); diff --git a/web/packages/new/photos/services/ml/worker.ts b/web/packages/new/photos/services/ml/worker.ts index 48f6213677..c4148fb555 100644 --- a/web/packages/new/photos/services/ml/worker.ts +++ b/web/packages/new/photos/services/ml/worker.ts @@ -2,7 +2,9 @@ import { clientPackageName } from "@/base/app"; import { assertionFailed } from "@/base/assert"; import { isHTTP4xxError } from "@/base/http"; import log from "@/base/log"; +import { logUnhandledErrorsAndRejectionsInWorker } from "@/base/log-web"; import type { ElectronMLWorker } from "@/base/types/ipc"; +import { isNetworkDownloadError } from "@/gallery/services/download"; import type { UploadItem } from "@/gallery/services/upload"; import { fileLogID, type EnteFile } from "@/media/file"; import { wait } from "@/utils/promise"; @@ -347,6 +349,8 @@ export class MLWorker { expose(MLWorker); +logUnhandledErrorsAndRejectionsInWorker(); + /** * Index the given batch of items. * @@ -370,7 +374,7 @@ const indexNextBatch = async ( } // Keep track if any of the items failed. - let allSuccess = true; + let failureCount = 0; // Index up to 4 items simultaneously. const tasks = new Array | undefined>(4).fill(undefined); @@ -386,8 +390,10 @@ const indexNextBatch = async ( .then(() => { tasks[j] = undefined; }) - .catch(() => { - allSuccess = false; + .catch((e: unknown) => { + const f = fileLogID(item.file); + log.error(`Failed to index ${f}`, e); + failureCount++; tasks[j] = undefined; }))(items[i++]!, j); } @@ -411,8 +417,14 @@ const indexNextBatch = async ( // Clear any cached CLIP indexes, since now we might have new ones. clearCachedCLIPIndexes(); + log.info( + failureCount > 0 + ? `Indexed ${items.length - failureCount} files (${failureCount} failed)` + : `Indexed ${items.length} files`, + ); + // Return true if nothing failed. - return allSuccess; + return failureCount == 0; }; /** @@ -521,25 +533,25 @@ const index = async ( // and return. if (existingFaceIndex && existingCLIPIndex) { - try { - await saveIndexes( - { fileID, ...existingFaceIndex }, - { fileID, ...existingCLIPIndex }, - ); - } catch (e) { - log.error(`Failed to save indexes for ${f}`, e); - throw e; - } + await saveIndexes( + { fileID, ...existingFaceIndex }, + { fileID, ...existingCLIPIndex }, + ); return; } // There is at least one ML data type that still needs to be indexed. - const renderableBlob = await fetchRenderableBlob( - file, - uploadItem, - electron, - ); + let renderableBlob: Blob; + try { + renderableBlob = await fetchRenderableBlob(file, uploadItem, electron); + } catch (e) { + // Network errors are transient and shouldn't be marked. + // + // See: [Note: Transient and permanent indexing failures] + if (!isNetworkDownloadError(e)) await markIndexingFailed(fileID); + throw e; + } let image: ImageBitmapAndData; try { @@ -551,8 +563,7 @@ const index = async ( // reindexing attempt for failed files). // // See: [Note: Transient and permanent indexing failures] - log.error(`Failed to get image data for indexing ${f}`, e); - await markIndexingFailed(file.id); + await markIndexingFailed(fileID); throw e; } @@ -569,8 +580,7 @@ const index = async ( ]); } catch (e) { // See: [Note: Transient and permanent indexing failures] - log.error(`Failed to index ${f}`, e); - await markIndexingFailed(file.id); + await markIndexingFailed(fileID); throw e; } @@ -611,33 +621,21 @@ const index = async ( await putMLData(file, rawMLData); } catch (e) { // See: [Note: Transient and permanent indexing failures] - log.error(`Failed to put ML data for ${f}`, e); - if (isHTTP4xxError(e)) await markIndexingFailed(file.id); + if (isHTTP4xxError(e)) await markIndexingFailed(fileID); throw e; } - try { - await saveIndexes( - { fileID, ...faceIndex }, - { fileID, ...clipIndex }, - ); - } catch (e) { - // Not sure if DB failures should be considered permanent or - // transient. There isn't a known case where writing to the local - // indexedDB should systematically fail. It could fail if there was - // no space on device, but that's eminently retriable. - log.error(`Failed to save indexes for ${f}`, e); - throw e; - } + await saveIndexes({ fileID, ...faceIndex }, { fileID, ...clipIndex }); // This step, saving face crops, is conceptually not part of the // indexing pipeline; we just do it here since we have already have the - // ImageBitmap at hand. Ignore errors that happen during this since it - // does not impact the generated face index. + // ImageBitmap at hand. if (!existingFaceIndex) { try { await saveFaceCrops(image.bitmap, faceIndex); } catch (e) { + // Ignore errors that happen during this since it does not + // impact the generated face index. log.error(`Failed to save face crops for ${f}`, e); } } diff --git a/web/packages/new/photos/services/search/worker.ts b/web/packages/new/photos/services/search/worker.ts index c96dd7425b..9fc9850712 100644 --- a/web/packages/new/photos/services/search/worker.ts +++ b/web/packages/new/photos/services/search/worker.ts @@ -1,4 +1,5 @@ import { HTTPError } from "@/base/http"; +import { logUnhandledErrorsAndRejectionsInWorker } from "@/base/log-web"; import type { Location } from "@/base/types"; import type { Collection } from "@/media/collection"; import type { EnteFile } from "@/media/file"; @@ -113,6 +114,8 @@ export class SearchWorker { expose(SearchWorker); +logUnhandledErrorsAndRejectionsInWorker(); + /** * @param s The normalized form of {@link searchString}. * @param searchString The original search string.