Skip to content

Commit

Permalink
[web] Reduce use of exceptions for control flow (#4031)
Browse files Browse the repository at this point in the history
  • Loading branch information
mnvr authored Nov 14, 2024
2 parents b8981be + 65a2b5a commit 3d0cc31
Show file tree
Hide file tree
Showing 12 changed files with 97 additions and 154 deletions.
162 changes: 78 additions & 84 deletions web/apps/photos/src/components/PhotoFrame.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import type { GalleryBarMode } from "@/new/photos/components/gallery/reducer";
import { TRASH_SECTION } from "@/new/photos/services/collection";
import DownloadManager from "@/new/photos/services/download";
import { PHOTOS_PAGES } from "@ente/shared/constants/pages";
import { CustomError } from "@ente/shared/error";
import useMemoSingleThreaded from "@ente/shared/hooks/useMemoSingleThreaded";
import { styled } from "@mui/material";
import PhotoViewer, { type PhotoViewerProps } from "components/PhotoViewer";
Expand Down Expand Up @@ -192,6 +191,8 @@ const PhotoFrame = ({
return <div />;
}

// Return a function which will return true if the URL was updated (for the
// given params), and false otherwise.
const updateURL =
(index: number) => (id: number, url: string, forceUpdate?: boolean) => {
const file = displayFiles[index];
Expand All @@ -200,14 +201,17 @@ const PhotoFrame = ({
log.info(
`[${id}]PhotoSwipe: updateURL: file id mismatch: ${file.id} !== ${id}`,
);
throw Error(CustomError.UPDATE_URL_FILE_ID_MISMATCH);
throw Error("update url file id mismatch");
}
if (file.msrc && !forceUpdate) {
throw Error(CustomError.URL_ALREADY_SET);
return false;
}
updateFileMsrcProps(file, url);
return true;
};

// Return true if the URL was updated (for the given params), and false
// otherwise.
const updateSrcURL = async (
index: number,
id: number,
Expand All @@ -220,16 +224,17 @@ const PhotoFrame = ({
log.info(
`[${id}]PhotoSwipe: updateSrcURL: file id mismatch: ${file.id}`,
);
throw Error(CustomError.UPDATE_URL_FILE_ID_MISMATCH);
throw Error("update url file id mismatch");
}
if (file.isSourceLoaded && !forceUpdate) {
throw Error(CustomError.URL_ALREADY_SET);
return false;
} else if (file.conversionFailed) {
log.info(`[${id}]PhotoSwipe: updateSrcURL: conversion failed`);
throw Error(CustomError.FILE_CONVERSION_FAILED);
throw Error("file conversion failed");
}

await updateFileSrcProps(file, srcURLs, enableDownload);
return true;
};

const handleClose = (needUpdate) => {
Expand Down Expand Up @@ -344,21 +349,20 @@ const PhotoFrame = ({
thumbFetching[item.id] = true;
const url = await DownloadManager.getThumbnailForPreview(item);
try {
updateURL(index)(item.id, url);
log.info(
`[${item.id}] calling invalidateCurrItems for thumbnail msrc: ${!!item.msrc}`,
);
instance.invalidateCurrItems();
if ((instance as any).isOpen()) {
instance.updateSize(true);
}
} catch (e) {
if (e.message !== CustomError.URL_ALREADY_SET) {
log.error(
"updating photoswipe after msrc url update failed",
e,
if (updateURL(index)(item.id, url)) {
log.info(
`[${item.id}] calling invalidateCurrItems for thumbnail msrc: ${!!item.msrc}`,
);
instance.invalidateCurrItems();
if ((instance as any).isOpen()) {
instance.updateSize(true);
}
}
} catch (e) {
log.error(
"updating photoswipe after msrc url update failed",
e,
);
// ignore
}
} catch (e) {
Expand Down Expand Up @@ -396,21 +400,20 @@ const PhotoFrame = ({
type: "normal",
};
try {
await updateSrcURL(index, item.id, dummyImgSrcUrl);
log.info(
`[${item.id}] calling invalidateCurrItems for live photo imgSrc, source loaded: ${item.isSourceLoaded}`,
);
instance.invalidateCurrItems();
if ((instance as any).isOpen()) {
instance.updateSize(true);
}
} catch (e) {
if (e.message !== CustomError.URL_ALREADY_SET) {
log.error(
"updating photoswipe after for live photo imgSrc update failed",
e,
if (await updateSrcURL(index, item.id, dummyImgSrcUrl)) {
log.info(
`[${item.id}] calling invalidateCurrItems for live photo imgSrc, source loaded: ${item.isSourceLoaded}`,
);
instance.invalidateCurrItems();
if ((instance as any).isOpen()) {
instance.updateSize(true);
}
}
} catch (e) {
log.error(
"updating photoswipe after for live photo imgSrc update failed",
e,
);
}
if (!imageURL) {
// no image url, no need to load video
Expand All @@ -425,44 +428,43 @@ const PhotoFrame = ({
type: "livePhoto",
};
try {
await updateSrcURL(
const updated = await updateSrcURL(
index,
item.id,
loadedLivePhotoSrcURL,
true,
);
log.info(
`[${item.id}] calling invalidateCurrItems for live photo complete, source loaded: ${item.isSourceLoaded}`,
);
instance.invalidateCurrItems();
if ((instance as any).isOpen()) {
instance.updateSize(true);
}
} catch (e) {
if (e.message !== CustomError.URL_ALREADY_SET) {
log.error(
"updating photoswipe for live photo complete update failed",
e,
if (updated) {
log.info(
`[${item.id}] calling invalidateCurrItems for live photo complete, source loaded: ${item.isSourceLoaded}`,
);
instance.invalidateCurrItems();
if ((instance as any).isOpen()) {
instance.updateSize(true);
}
}
} catch (e) {
log.error(
"updating photoswipe for live photo complete update failed",
e,
);
}
} else {
try {
await updateSrcURL(index, item.id, srcURLs);
log.info(
`[${item.id}] calling invalidateCurrItems for src, source loaded: ${item.isSourceLoaded}`,
);
instance.invalidateCurrItems();
if ((instance as any).isOpen()) {
instance.updateSize(true);
}
} catch (e) {
if (e.message !== CustomError.URL_ALREADY_SET) {
log.error(
"updating photoswipe after src url update failed",
e,
if (await updateSrcURL(index, item.id, srcURLs)) {
log.info(
`[${item.id}] calling invalidateCurrItems for src, source loaded: ${item.isSourceLoaded}`,
);
instance.invalidateCurrItems();
if ((instance as any).isOpen()) {
instance.updateSize(true);
}
}
} catch (e) {
log.error(
"updating photoswipe after src url update failed",
e,
);
}
}
} catch (e) {
Expand Down Expand Up @@ -491,21 +493,17 @@ const PhotoFrame = ({
return;
}
try {
updateURL(index)(item.id, item.msrc, true);
log.info(
`[${item.id}] calling invalidateCurrItems for thumbnail msrc: ${!!item.msrc}`,
);
instance.invalidateCurrItems();
if ((instance as any).isOpen()) {
instance.updateSize(true);
}
} catch (e) {
if (e.message !== CustomError.URL_ALREADY_SET) {
log.error(
"updating photoswipe after msrc url update failed",
e,
if (updateURL(index)(item.id, item.msrc, true)) {
log.info(
`[${item.id}] calling invalidateCurrItems for thumbnail msrc: ${!!item.msrc}`,
);
instance.invalidateCurrItems();
if ((instance as any).isOpen()) {
instance.updateSize(true);
}
}
} catch (e) {
log.error("updating photoswipe after msrc url update failed", e);
// ignore
}
try {
Expand All @@ -519,21 +517,17 @@ const PhotoFrame = ({
});

try {
await updateSrcURL(index, item.id, srcURL, true);
log.info(
`[${item.id}] calling invalidateCurrItems for src, source loaded: ${item.isSourceLoaded}`,
);
instance.invalidateCurrItems();
if ((instance as any).isOpen()) {
instance.updateSize(true);
}
} catch (e) {
if (e.message !== CustomError.URL_ALREADY_SET) {
log.error(
"updating photoswipe after src url update failed",
e,
if (await updateSrcURL(index, item.id, srcURL, true)) {
log.info(
`[${item.id}] calling invalidateCurrItems for src, source loaded: ${item.isSourceLoaded}`,
);
instance.invalidateCurrItems();
if ((instance as any).isOpen()) {
instance.updateSize(true);
}
}
} catch (e) {
log.error("updating photoswipe after src url update failed", e);
throw e;
}
} catch (e) {
Expand Down
5 changes: 1 addition & 4 deletions web/apps/photos/src/components/pages/gallery/PreviewCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
} from "@/new/photos/components/PlaceholderThumbnails";
import { TRASH_SECTION } from "@/new/photos/services/collection";
import DownloadManager from "@/new/photos/services/download";
import { CustomError } from "@ente/shared/error";
import useLongPress from "@ente/shared/hooks/useLongPress";
import AlbumOutlined from "@mui/icons-material/AlbumOutlined";
import Favorite from "@mui/icons-material/FavoriteRounded";
Expand Down Expand Up @@ -286,9 +285,7 @@ export default function PreviewCard(props: IProps) {
setImgSrc(url);
updateURL(file.id, url);
} catch (e) {
if (e.message !== CustomError.URL_ALREADY_SET) {
log.error("preview card useEffect failed", e);
}
log.error("preview card useEffect failed", e);
// no-op
}
};
Expand Down
3 changes: 1 addition & 2 deletions web/apps/photos/src/services/collectionService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import { getLocalFiles, sortFiles } from "@/new/photos/services/files";
import { updateMagicMetadata } from "@/new/photos/services/magic-metadata";
import type { FamilyData } from "@/new/photos/services/user-details";
import { batch } from "@/utils/array";
import { CustomError } from "@ente/shared/error";
import HTTPService from "@ente/shared/network/HTTPService";
import localForage from "@ente/shared/storage/localForage";
import { LS_KEYS, getData } from "@ente/shared/storage/localStorage";
Expand Down Expand Up @@ -423,7 +422,7 @@ export const removeFromFavorites = async (file: EnteFile) => {
try {
const favCollection = await getFavCollection();
if (!favCollection) {
throw Error(CustomError.FAV_COLLECTION_MISSING);
throw Error("favorite collection missing");
}
await removeFromCollection(favCollection.id, [file]);
} catch (e) {
Expand Down
16 changes: 2 additions & 14 deletions web/apps/photos/src/services/export/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
import { getAllLocalFiles } from "@/new/photos/services/files";
import { safeDirectoryName, safeFileName } from "@/new/photos/utils/native-fs";
import { writeStream } from "@/new/photos/utils/native-stream";
import { wait } from "@/utils/promise";
import { CustomError } from "@ente/shared/error";
import { LS_KEYS, getData, setData } from "@ente/shared/storage/localStorage";
import QueueProcessor, {
Expand Down Expand Up @@ -877,7 +876,7 @@ class ExportService {
}
}

async getExportRecord(folder: string, retry = true): Promise<ExportRecord> {
async getExportRecord(folder: string): Promise<ExportRecord> {
const electron = ensureElectron();
const fs = electron.fs;
try {
Expand All @@ -887,19 +886,8 @@ class ExportService {
return this.createEmptyExportRecord(exportRecordJSONPath);
}
const recordFile = await fs.readTextFile(exportRecordJSONPath);
try {
return JSON.parse(recordFile);
} catch (e) {
throw Error(CustomError.EXPORT_RECORD_JSON_PARSING_FAILED);
}
return JSON.parse(recordFile);
} catch (e) {
if (
e.message === CustomError.EXPORT_RECORD_JSON_PARSING_FAILED &&
retry
) {
await wait(1000);
return await this.getExportRecord(folder, false);
}
if (e.message !== CustomError.EXPORT_FOLDER_DOES_NOT_EXIST) {
log.error("export Record JSON parsing failed", e);
}
Expand Down
4 changes: 1 addition & 3 deletions web/apps/photos/src/services/upload/uploadHttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import HTTPService from "@ente/shared/network/HTTPService";
import { getToken } from "@ente/shared/storage/localStorage/helpers";
import { MultipartUploadURLs, UploadFile, UploadURL } from "./upload-service";

const MAX_URL_REQUESTS = 50;

class UploadHttpClient {
private uploadURLFetchInProgress = null;

Expand Down Expand Up @@ -44,7 +42,7 @@ class UploadHttpClient {
this.uploadURLFetchInProgress = HTTPService.get(
await apiURL("/files/upload-urls"),
{
count: Math.min(MAX_URL_REQUESTS, count * 2),
count: Math.min(50, count * 2),
},
{ "X-Auth-Token": token },
);
Expand Down
3 changes: 1 addition & 2 deletions web/apps/photos/src/services/upload/uploadManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,7 @@ class UIService {
index = 0,
) {
const cancel: { exec: Canceler } = { exec: () => {} };
const cancelTimedOutRequest = () =>
cancel.exec(CustomError.REQUEST_TIMEOUT);
const cancelTimedOutRequest = () => cancel.exec("Request timed out");

const cancelCancelledUploadRequest = () =>
cancel.exec(CustomError.UPLOAD_CANCELLED);
Expand Down
3 changes: 1 addition & 2 deletions web/apps/photos/src/utils/collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
import { getAllLocalFiles, getLocalFiles } from "@/new/photos/services/files";
import { updateMagicMetadata } from "@/new/photos/services/magic-metadata";
import { safeDirectoryName } from "@/new/photos/utils/native-fs";
import { CustomError } from "@ente/shared/error";
import { LS_KEYS, getData } from "@ente/shared/storage/localStorage";
import type { User } from "@ente/shared/user/types";
import bs58 from "bs58";
Expand Down Expand Up @@ -74,7 +73,7 @@ export async function handleCollectionOps(
await unhideToCollection(collection, selectedFiles);
break;
default:
throw Error(CustomError.INVALID_COLLECTION_OPERATION);
throw Error("Invalid collection operation");
}
}

Expand Down
Loading

0 comments on commit 3d0cc31

Please sign in to comment.