[desktop] Improve error bifurcation during ML indexing (#5113)

This commit is contained in:
Manav Rathi
2025-02-19 19:24:52 +05:30
committed by GitHub
3 changed files with 79 additions and 13 deletions

View File

@@ -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<ReadableStream<Uint8Array> | 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 = <T>(op: () => Promise<T>) =>
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

View File

@@ -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;
};

View File

@@ -4,6 +4,7 @@ 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";
@@ -541,11 +542,16 @@ const index = async (
// 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 {
@@ -557,7 +563,7 @@ const index = async (
// reindexing attempt for failed files).
//
// See: [Note: Transient and permanent indexing failures]
await markIndexingFailed(file.id);
await markIndexingFailed(fileID);
throw e;
}
@@ -574,7 +580,7 @@ const index = async (
]);
} catch (e) {
// See: [Note: Transient and permanent indexing failures]
await markIndexingFailed(file.id);
await markIndexingFailed(fileID);
throw e;
}
@@ -615,7 +621,7 @@ const index = async (
await putMLData(file, rawMLData);
} catch (e) {
// See: [Note: Transient and permanent indexing failures]
if (isHTTP4xxError(e)) await markIndexingFailed(file.id);
if (isHTTP4xxError(e)) await markIndexingFailed(fileID);
throw e;
}