Skip to content

Commit

Permalink
[web] Improve handling of the CF Upload proxy flag (#3951)
Browse files Browse the repository at this point in the history
  • Loading branch information
mnvr authored Nov 5, 2024
2 parents b6e5703 + 4f0ba47 commit ca0ab41
Show file tree
Hide file tree
Showing 13 changed files with 178 additions and 103 deletions.
24 changes: 11 additions & 13 deletions web/apps/photos/src/components/Sidebar/Preferences.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,17 @@ import {
settingsSnapshot,
settingsSubscribe,
syncSettings,
updateCFProxyDisabledPreference,
updateMapEnabled,
} from "@/new/photos/services/settings";
import { AppContext, useAppContext } from "@/new/photos/types/context";
import { useAppContext } from "@/new/photos/types/context";
import { EnteMenuItem } from "@ente/shared/components/Menu/EnteMenuItem";
import ChevronRight from "@mui/icons-material/ChevronRight";
import ScienceIcon from "@mui/icons-material/Science";
import { Box, Stack } from "@mui/material";
import DropdownInput from "components/DropdownInput";
import { t } from "i18next";
import React, {
useCallback,
useContext,
useEffect,
useSyncExternalStore,
} from "react";
import React, { useCallback, useEffect, useSyncExternalStore } from "react";

export const Preferences: React.FC<NestedSidebarDrawerVisibilityProps> = ({
open,
Expand Down Expand Up @@ -234,16 +230,18 @@ export const AdvancedSettings: React.FC<NestedSidebarDrawerVisibilityProps> = ({
onClose,
onRootClose,
}) => {
const appContext = useContext(AppContext);
const { cfUploadProxyDisabled } = useSyncExternalStore(
settingsSubscribe,
settingsSnapshot,
);

const handleRootClose = () => {
onClose();
onRootClose();
};

const toggleCFProxy = () => {
appContext.setIsCFProxyDisabled(!appContext.isCFProxyDisabled);
};
const toggle = () =>
void updateCFProxyDisabledPreference(!cfUploadProxyDisabled);

return (
<NestedSidebarDrawer
Expand All @@ -262,8 +260,8 @@ export const AdvancedSettings: React.FC<NestedSidebarDrawerVisibilityProps> = ({
<MenuItemGroup>
<EnteMenuItem
variant="toggle"
checked={!appContext.isCFProxyDisabled}
onClick={toggleCFProxy}
checked={!cfUploadProxyDisabled}
onClick={toggle}
label={t("faster_upload")}
/>
</MenuItemGroup>
Expand Down
2 changes: 0 additions & 2 deletions web/apps/photos/src/components/Upload/Uploader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,6 @@ export default function Uploader({
},
onUploadFile,
publicCollectionGalleryContext,
appContext.isCFProxyDisabled,
);

if (uploadManager.isUploadRunning()) {
Expand Down Expand Up @@ -287,7 +286,6 @@ export default function Uploader({
publicCollectionGalleryContext.accessedThroughSharedURL,
publicCollectionGalleryContext.token,
publicCollectionGalleryContext.passwordToken,
appContext.isCFProxyDisabled,
]);

// Handle selected files when user selects files for upload through the open
Expand Down
6 changes: 0 additions & 6 deletions web/apps/photos/src/pages/_app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,6 @@ export default function App({ Component, pageProps }: AppProps) {
LS_KEYS.THEME,
THEME_COLOR.DARK,
);
const [isCFProxyDisabled, setIsCFProxyDisabled] = useLocalState(
LS_KEYS.CF_PROXY_DISABLED,
false,
);

useEffect(() => {
void setupI18n().finally(() => setIsI18nReady(true));
Expand Down Expand Up @@ -230,8 +226,6 @@ export default function App({ Component, pageProps }: AppProps) {
showMiniDialog,
somethingWentWrong,
onGenericError,
isCFProxyDisabled,
setIsCFProxyDisabled,
logout,
};

Expand Down
2 changes: 2 additions & 0 deletions web/apps/photos/src/pages/shared-albums.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { useIsSmallWidth, useIsTouchscreen } from "@/base/hooks";
import log from "@/base/log";
import type { Collection } from "@/media/collection";
import { type EnteFile, mergeMetadata } from "@/media/file";
import { updateShouldDisableCFUploadProxy } from "@/media/upload";
import {
GalleryItemsHeaderAdapter,
GalleryItemsSummary,
Expand Down Expand Up @@ -240,6 +241,7 @@ export default function PublicCollectionGallery() {
: await cryptoWorker.fromHex(ck);
token.current = t;
downloadManager.updateToken(token.current);
await updateShouldDisableCFUploadProxy();
collectionKey.current = dck;
url.current = window.location.href;
const localCollection = await getLocalPublicCollection(
Expand Down
7 changes: 7 additions & 0 deletions web/apps/photos/src/services/logout.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { accountLogout } from "@/accounts/services/logout";
import log from "@/base/log";
import { resetUploadState } from "@/media/upload";
import DownloadManager from "@/new/photos/services/download";
import { logoutML, terminateMLWorker } from "@/new/photos/services/ml";
import { logoutSearch } from "@/new/photos/services/search";
Expand Down Expand Up @@ -42,6 +43,12 @@ export const photosLogout = async () => {
ignoreError("settings", e);
}

try {
resetUploadState();
} catch (e) {
ignoreError("upload", e);
}

try {
DownloadManager.logout();
} catch (e) {
Expand Down
33 changes: 4 additions & 29 deletions web/apps/photos/src/services/upload/uploadManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type { Collection } from "@/media/collection";
import { EncryptedEnteFile, EnteFile } from "@/media/file";
import { FileType } from "@/media/file-type";
import { potentialFileTypeFromExtension } from "@/media/live-photo";
import { shouldDisableCFUploadProxy } from "@/media/upload";
import { getLocalFiles } from "@/new/photos/services/files";
import { indexNewUpload } from "@/new/photos/services/ml";
import type { UploadItem } from "@/new/photos/services/upload/types";
Expand All @@ -25,7 +26,6 @@ import {
getLocalPublicFiles,
getPublicCollectionUID,
} from "services/publicCollectionService";
import { getDisableCFUploadProxyFlag } from "services/userService";
import watcher from "services/watch";
import { decryptFile, getUserOwnedFiles } from "utils/file";
import {
Expand Down Expand Up @@ -331,7 +331,6 @@ class UploadManager {
private publicUploadProps: PublicUploadProps;
private uploaderName: string;
private uiService: UIService;
private isCFUploadProxyDisabled: boolean = false;

constructor() {
this.uiService = new UIService();
Expand All @@ -341,15 +340,8 @@ class UploadManager {
progressUpdater: ProgressUpdater,
onUploadFile: (file: EnteFile) => void,
publicCollectProps: PublicUploadProps,
isCFUploadProxyDisabled: boolean,
) {
this.uiService.init(progressUpdater);
const remoteIsCFUploadProxyDisabled =
await getDisableCFUploadProxyFlag();
if (remoteIsCFUploadProxyDisabled) {
isCFUploadProxyDisabled = remoteIsCFUploadProxyDisabled;
}
this.isCFUploadProxyDisabled = isCFUploadProxyDisabled;
UploadService.init(publicCollectProps);
this.onUploadFile = onUploadFile;
this.publicUploadProps = publicCollectProps;
Expand Down Expand Up @@ -543,7 +535,7 @@ class UploadManager {
this.existingFiles,
this.parsedMetadataJSONMap,
worker,
this.isCFUploadProxyDisabled,
shouldDisableCFUploadProxy(),
() => {
this.abortIfCancelled();
},
Expand Down Expand Up @@ -897,25 +889,8 @@ const clusterLivePhotos = async (
};

/**
* [Note: Memory pressure when uploading video files]
*
* A user (Fedora 39 VM on Qubes OS with 32 GB RAM, both AppImage and RPM) has
* reported that their app runs out of memory when the app tries to upload
* multiple large videos simultaneously. For example, 4 parallel uploads of 4
* 700 MB videos.
*
* I am unable to reproduce this: tested on macOS and Linux, with videos up to
* 3.8 G x 1 + 3 x 700 M uploaded in parallel. The memory usage remains constant
* as expected (hovering around 2 G), since we don't pull the entire videos in
* memory and instead do a streaming disk read + encryption + upload.
*
* The JavaScript heap for the renderer process (when we're running in the
* context of our desktop app) is limited to 4 GB. See
* https://www.electronjs.org/blog/v8-memory-cage.
*
* For now, add logs if our usage increases some high water mark. This is solely
* so we can better understand the issue if it arises again (and can deal with
* it in an informed manner).
* Add logs if our usage increases some high water mark. This is solely so that
* we have some indication in the logs if we get a user report of OOM crashes.
*/
const logAboutMemoryPressureIfNeeded = () => {
if (!globalThis.electron) return;
Expand Down
41 changes: 2 additions & 39 deletions web/apps/photos/src/services/userService.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import { putAttributes } from "@/accounts/api/user";
import log from "@/base/log";
import { apiURL, customAPIOrigin, familyAppOrigin } from "@/base/origins";
import { apiURL, familyAppOrigin } from "@/base/origins";
import { ApiError } from "@ente/shared/error";
import HTTPService from "@ente/shared/network/HTTPService";
import { LS_KEYS, getData } from "@ente/shared/storage/localStorage";
import { getToken } from "@ente/shared/storage/localStorage/helpers";
import { HttpStatusCode } from "axios";
import {
DeleteChallengeResponse,
GetFeatureFlagResponse,
UserDetails,
} from "types/user";
import { DeleteChallengeResponse, UserDetails } from "types/user";
import { getLocalFamilyData, isPartOfFamily } from "utils/user/family";

const HAS_SET_KEYS = "hasSetKeys";
Expand Down Expand Up @@ -172,36 +168,3 @@ export const deleteAccount = async (
throw e;
}
};

/**
* Return true to disable the upload of files via Cloudflare Workers.
*
* These workers were introduced as a way of make file uploads faster:
* https://ente.io/blog/tech/making-uploads-faster/
*
* By default, that's the route we take. However, during development or when
* self-hosting it can be convenient to turn this flag on to directly upload to
* the S3-compatible URLs returned by the ente API.
*
* Note the double negative (Enhancement: maybe remove the double negative,
* rename this to say getUseDirectUpload).
*/
export async function getDisableCFUploadProxyFlag(): Promise<boolean> {
// If a custom origin is set, that means we're not running a production
// deployment (maybe we're running locally, or being self-hosted).
//
// In such cases, disable the Cloudflare upload proxy (which won't work for
// self-hosters), and instead just directly use the upload URLs that museum
// gives us.
if (await customAPIOrigin()) return true;

try {
const featureFlags = (
await fetch("https://static.ente.io/feature_flags.json")
).json() as GetFeatureFlagResponse;
return featureFlags.disableCFUploadProxy;
} catch (e) {
log.error("failed to get feature flags", e);
return false;
}
}
4 changes: 0 additions & 4 deletions web/apps/photos/src/types/user/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,3 @@ export interface CreateSRPSessionResponse {
sessionID: string;
srpB: string;
}

export interface GetFeatureFlagResponse {
disableCFUploadProxy?: boolean;
}
95 changes: 95 additions & 0 deletions web/packages/media/upload.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import log from "@/base/log";
import { customAPIOrigin } from "@/base/origins";
import { nullToUndefined } from "@/utils/transform";
import { z } from "zod";

/**
* Internal in-memory state shared by the functions in this module.
*
* This entire object will be reset on logout.
*/
class UploadState {
/**
* `true` if the workers should be disabled for uploads.
*/
shouldDisableCFUploadProxy = false;
}

/** State shared by the functions in this module. See {@link UploadState}. */
let _state = new UploadState();

/**
* Reset any internal state maintained by the module.
*
* This is primarily meant as a way for stateful apps (e.g. photos) to clear any
* user specific state on logout.
*/
export const resetUploadState = () => {
_state = new UploadState();
};

/**
* Return true to disable the upload of files via Cloudflare Workers.
*
* These workers were introduced as a way of make file uploads faster:
* https://ente.io/blog/tech/making-uploads-faster/
*
* By default, that's the route we take. However, there are multiple reasons why
* this might be disabled.
*
* 1. During development and when self-hosting it we disable them to directly
* upload to the S3-compatible URLs returned by the ente API.
*
* 2. In rare cases, the user might have trouble reaching Cloudflare's network
* from their ISP. In such cases, the user can locally turn this off via
* settings.
*
* 3. There is also the original global toggle that was added when this feature
* was introduced.
*
* This function returns the in-memory value. It is updated when #2 changes (if
* we're running in a context where that makes sense). The #3 remote status is
* obtained once, on app start.
*/
export const shouldDisableCFUploadProxy = () =>
_state.shouldDisableCFUploadProxy;

/**
* Update the in-memory value of {@link shouldDisableCFUploadProxy}.
*
* @param savedPreference An optional user preference that the user has
* expressed to disable the proxy.
*/
export const updateShouldDisableCFUploadProxy = async (
savedPreference?: boolean | undefined,
) => {
_state.shouldDisableCFUploadProxy =
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
savedPreference || (await computeShouldDisableCFUploadProxy());
};

const computeShouldDisableCFUploadProxy = async () => {
// If a custom origin is set, that means we're not running a production
// deployment (maybe we're running locally, or being self-hosted).
//
// In such cases, disable the Cloudflare upload proxy (which won't work for
// self-hosters), and instead just directly use the upload URLs that museum
// gives us.
if (await customAPIOrigin()) return true;

// See if the global flag to disable this is set.
try {
const res = await fetch("https://static.ente.io/feature_flags.json");
return (
StaticFeatureFlags.parse(await res.json()).disableCFUploadProxy ??
false
);
} catch (e) {
log.warn("Ignoring error when getting feature_flags.json", e);
return false;
}
};

const StaticFeatureFlags = z.object({
disableCFUploadProxy: z.boolean().nullable().transform(nullToUndefined),
});
6 changes: 2 additions & 4 deletions web/packages/new/photos/services/migrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,9 @@ const m3 = () =>
removeKV("latestUpdatedAt/location"),
]);

// TODO: Not enabled yet.
// TODO: Not enabled yet since it is not critical. Enable with next batch of changes.
// // Added: Nov 2025 (v1.7.7-beta). Prunable.
// const m4 = () => {
// // Delete legacy keys for storing individual settings
// // LS_KEYS.MAP_ENABLED = "mapEnabled",
// // Delete the legacy key that used to store the map preference.
// localStorage.removeItem("mapEnabled");
// localStorage.removeItem("cfProxyDisabled");
// };
Loading

0 comments on commit ca0ab41

Please sign in to comment.