fix(app): back off image, reaction and settings retries on 429 (#2743)

fluxer_app/src/features/messaging/commands/ReactionCommands.tsx

@@ -8,10 +8,11 @@ import {TooManyReactionsModal} from '@app/features/messaging/components/alerts/T
 import MessageReactions from '@app/features/messaging/state/MessageReactions';
 import Messages from '@app/features/messaging/state/MessagingMessages';
 import type {ReactionEmoji} from '@app/features/messaging/utils/ReactionUtils';
+import {resolveRetryAfterMs} from '@app/features/messaging/utils/RetryAfterUtils';
 import {http} from '@app/features/platform/transport/RestTransport';
 import {HttpError} from '@app/features/platform/types/EndpointError';
 import {Logger} from '@app/features/platform/utils/AppLogger';
-import {failureCode, failureRetryAfter} from '@app/features/platform/utils/ResponseInspection';
+import {failureCode} from '@app/features/platform/utils/ResponseInspection';
 import * as ModalCommands from '@app/features/ui/commands/ModalCommands';
 import {modal} from '@app/features/ui/commands/ModalCommands';
 import * as ToastCommands from '@app/features/ui/commands/ToastCommands';
@@ -41,7 +42,7 @@ type ReactionOptimisticType =
 	| 'MESSAGE_REACTION_REMOVE_ALL'
 	| 'MESSAGE_REACTION_REMOVE_EMOJI';
 
-const checkReactionResponse = (i18n: I18n, error: HttpError, retry: () => void): boolean => {
+const checkReactionResponse = (i18n: I18n, error: HttpError): boolean => {
 	const errorCode = failureCode(error);
 	if (error.status === 403) {
 		if (errorCode === APIErrorCodes.FEATURE_TEMPORARILY_DISABLED) {
@@ -62,12 +63,6 @@ const checkReactionResponse = (i18n: I18n, error: HttpError, retry: () => void):
 			return true;
 		}
 	}
-	if (error.status === 429) {
-		const retryAfter = failureRetryAfter(error) || 1000;
-		logger.debug(`Rate limited, retrying after ${retryAfter}ms`);
-		setTimeout(retry, retryAfter);
-		return false;
-	}
 	if (error.status === 400) {
 		switch (errorCode) {
 			case APIErrorCodes.MAX_REACTIONS:
@@ -220,12 +215,11 @@ async function retryWithExponentialBackoff<T>(func: () => Promise<T>, attempts =
 	try {
 		return await func();
 	} catch (error) {
-		const status = error instanceof HttpError ? error.status : undefined;
+		if (!(error instanceof HttpError) || error.status !== 429) {
-		if (status !== 429) {
 			throw error;
 		}
 		if (attempts < MAX_RETRIES) {
-			const backoffTime = 2 ** attempts * 1000;
+			const backoffTime = Math.max(2 ** attempts * 1000, resolveRetryAfterMs(error) ?? 0);
 			logger.debug(`Rate limited, retrying in ${backoffTime}ms (attempt ${attempts + 1}/${MAX_RETRIES})`);
 			await delay(backoffTime);
 			return retryWithExponentialBackoff(func, attempts + 1);
@@ -246,11 +240,7 @@ const performReactionAction = (
 ): void => {
 	optimisticUpdate(type, channelId, messageId, emoji, userId);
 	retryWithExponentialBackoff(apiFunc).catch((error) => {
-		if (
+		if (checkReactionResponse(i18n, error)) {
-			checkReactionResponse(i18n, error, () =>
-				performReactionAction(i18n, type, apiFunc, channelId, messageId, emoji, userId),
-			)
-		) {
 			logger.debug(`Reverting optimistic update for reaction in message ${messageId}`);
 			optimisticUpdate(
 				type === 'MESSAGE_REACTION_ADD' ? 'MESSAGE_REACTION_REMOVE' : 'MESSAGE_REACTION_ADD',
@@ -329,7 +319,7 @@ export function removeAllReactions(i18n: I18n, channelId: string, messageId: str
 	logger.debug(`Removing all reactions from message ${messageId} in channel ${channelId}`);
 	const apiFunc = () => removeAllReactionsRequest(channelId, messageId);
 	retryWithExponentialBackoff(apiFunc).catch((error) => {
-		checkReactionResponse(i18n, error, () => removeAllReactions(i18n, channelId, messageId));
+		checkReactionResponse(i18n, error);
 	});
 }
 
@@ -338,6 +328,6 @@ export function removeReactionEmoji(i18n: I18n, channelId: string, messageId: st
 	optimisticUpdate('MESSAGE_REACTION_REMOVE_EMOJI', channelId, messageId, emoji);
 	const apiFunc = () => removeReactionEmojiRequest(channelId, messageId, emoji);
 	retryWithExponentialBackoff(apiFunc).catch((error) => {
-		checkReactionResponse(i18n, error, () => removeReactionEmoji(i18n, channelId, messageId, emoji));
+		checkReactionResponse(i18n, error);
 	});
 }

fluxer_app/src/features/messaging/utils/ImageCacheUtils.test.ts

@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+import * as ImageCacheUtils from '@app/features/messaging/utils/ImageCacheUtils';
+import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest';
+
+const AVATAR_URL = 'https://fluxerusercontent.com/avatars/1471779547901222947/93e5e9b9.webp?size=160';
+
+class FailingImage {
+	static created: Array<FailingImage> = [];
+	decoding = '';
+	complete = false;
+	naturalWidth = 0;
+	naturalHeight = 0;
+	onload: (() => void) | null = null;
+	onerror: (() => void) | null = null;
+	private source = '';
+
+	constructor() {
+		FailingImage.created.push(this);
+	}
+
+	get src(): string {
+		return this.source;
+	}
+
+	set src(value: string) {
+		this.source = value;
+		this.onerror?.();
+	}
+}
+
+function failImageLoad(): void {
+	const onError = vi.fn();
+	ImageCacheUtils.loadImage(AVATAR_URL, () => {}, onError);
+	vi.runAllTimers();
+	expect(onError).toHaveBeenCalledTimes(1);
+}
+
+describe('ImageCacheUtils failure cooldown', () => {
+	beforeEach(() => {
+		FailingImage.created = [];
+		vi.useFakeTimers();
+		vi.stubGlobal('window', globalThis);
+		vi.stubGlobal('Image', FailingImage);
+		ImageCacheUtils._clearForTests();
+	});
+
+	afterEach(() => {
+		ImageCacheUtils._clearForTests();
+		vi.unstubAllGlobals();
+		vi.useRealTimers();
+	});
+
+	it('gives up on a failing image after two retries', () => {
+		failImageLoad();
+		expect(FailingImage.created).toHaveLength(3);
+		expect(ImageCacheUtils.hasFailedImage(AVATAR_URL)).toBe(true);
+	});
+
+	it('rejects new loads of a failed image without requesting it again during the cooldown', () => {
+		failImageLoad();
+		const onError = vi.fn();
+		ImageCacheUtils.loadImage(AVATAR_URL, () => {}, onError);
+		vi.advanceTimersByTime(59_000);
+		ImageCacheUtils.loadImage(AVATAR_URL, () => {}, onError);
+		expect(onError).toHaveBeenCalledTimes(2);
+		expect(FailingImage.created).toHaveLength(3);
+	});
+
+	it('requests the image again once the cooldown has passed', () => {
+		failImageLoad();
+		vi.advanceTimersByTime(60_000);
+		expect(ImageCacheUtils.hasFailedImage(AVATAR_URL)).toBe(false);
+		failImageLoad();
+		expect(FailingImage.created).toHaveLength(6);
+	});
+
+	it('requests the image again straight away after it is forgotten', () => {
+		failImageLoad();
+		ImageCacheUtils.forgetImage(AVATAR_URL);
+		expect(ImageCacheUtils.hasFailedImage(AVATAR_URL)).toBe(false);
+		ImageCacheUtils.loadImage(AVATAR_URL, () => {});
+		expect(FailingImage.created).toHaveLength(4);
+	});
+});

fluxer_app/src/features/messaging/utils/ImageCacheUtils.ts

@@ -21,6 +21,7 @@ interface ImageCacheEntry {
 	subscribers: Set<ImageSubscriber>;
 	failedAttempts: number;
 	retryDelayMs: number;
+	failedUntil: number;
 	loadTimeoutId: number;
 	retryTimeoutId: number;
 	connectivityListener: (() => void) | null;
@@ -30,9 +31,10 @@ const MAX_CACHE_ENTRIES = 1000;
 const MAX_IMAGE_SOURCE_LENGTH = 16 * 1024;
 const MAX_PENDING_IMAGE_CALLBACKS_PER_LOAD = 256;
 const IMAGE_LOAD_TIMEOUT_MS = 30_000;
-const IMAGE_RETRY_ATTEMPT_LIMIT = 5;
+const IMAGE_RETRY_ATTEMPT_LIMIT = 2;
-const IMAGE_RETRY_INITIAL_DELAY_MS = 500;
+const IMAGE_RETRY_INITIAL_DELAY_MS = 1000;
 const IMAGE_RETRY_MAX_DELAY_MS = IMAGE_RETRY_INITIAL_DELAY_MS * 10;
+const IMAGE_FAILURE_COOLDOWN_MS = 60_000;
 
 const imageCache = new LRUCache<string, ImageCacheEntry>({
 	max: MAX_CACHE_ENTRIES,
@@ -66,6 +68,8 @@ const imageHasSource = (image: HTMLImageElement, src: string): boolean => {
 
 const ownsCacheKey = (entry: ImageCacheEntry): boolean => imageCache.peek(entry.src) === entry;
 
+const isCoolingDown = (entry: ImageCacheEntry): boolean => entry.failedUntil > Date.now();
+
 function clearRetryState(entry: ImageCacheEntry): void {
 	if (entry.retryTimeoutId !== 0) {
 		window.clearTimeout(entry.retryTimeoutId);
@@ -110,11 +114,18 @@ function abandonEntry(entry: ImageCacheEntry): void {
 	notifySubscribers(entry, false);
 }
 
-function failEntry(entry: ImageCacheEntry): void {
+function dropEntry(entry: ImageCacheEntry): void {
 	if (ownsCacheKey(entry)) imageCache.delete(entry.src);
 	abandonEntry(entry);
 }
 
+function failEntry(entry: ImageCacheEntry): void {
+	entry.failedAttempts = 0;
+	entry.retryDelayMs = IMAGE_RETRY_INITIAL_DELAY_MS;
+	entry.failedUntil = Date.now() + IMAGE_FAILURE_COOLDOWN_MS;
+	abandonEntry(entry);
+}
+
 function settleLoaded(entry: ImageCacheEntry, image: HTMLImageElement): void {
 	clearRetryState(entry);
 	detachImageLoad(entry);
@@ -123,6 +134,7 @@ function settleLoaded(entry: ImageCacheEntry, image: HTMLImageElement): void {
 	entry.height = image.naturalHeight;
 	entry.failedAttempts = 0;
 	entry.retryDelayMs = IMAGE_RETRY_INITIAL_DELAY_MS;
+	entry.failedUntil = 0;
 	notifySubscribers(entry, true);
 }
 
@@ -187,6 +199,7 @@ function createEntry(src: string): ImageCacheEntry {
 		subscribers: new Set(),
 		failedAttempts: 0,
 		retryDelayMs: IMAGE_RETRY_INITIAL_DELAY_MS,
+		failedUntil: 0,
 		loadTimeoutId: 0,
 		retryTimeoutId: 0,
 		connectivityListener: null,
@@ -205,6 +218,12 @@ export function hasImage(src: string | null | undefined): boolean {
 	return imageCache.get(src)?.loaded === true;
 }
 
+export function hasFailedImage(src: string | null | undefined): boolean {
+	if (!acceptsImageSource(src)) return false;
+	const entry = imageCache.peek(src);
+	return entry != null && isCoolingDown(entry);
+}
+
 export function getImageSize(src: string | null | undefined): CachedImageSize | undefined {
 	if (!acceptsImageSource(src)) return undefined;
 	const entry = imageCache.get(src);
@@ -223,7 +242,7 @@ export function forgetImage(src: string | null | undefined): void {
 	if (!acceptsImageSource(src)) return;
 	const entry = imageCache.get(src);
 	if (entry == null) return;
-	failEntry(entry);
+	dropEntry(entry);
 }
 
 export function loadImage(src: string | null | undefined, onLoad: () => void, onError?: () => void): () => void {
@@ -233,13 +252,16 @@ export function loadImage(src: string | null | undefined, onLoad: () => void, on
 		onLoad();
 		return () => {};
 	}
-	if (cached != null && cached.subscribers.size >= MAX_PENDING_IMAGE_CALLBACKS_PER_LOAD) {
+	if (cached != null && (isCoolingDown(cached) || cached.subscribers.size >= MAX_PENDING_IMAGE_CALLBACKS_PER_LOAD)) {
 		return rejectImageLoad(onError);
 	}
 	const entry = cached ?? createEntry(src);
 	const subscriber: ImageSubscriber = {onLoad, onError};
 	entry.subscribers.add(subscriber);
-	if (cached == null) startImageLoad(entry);
+	if (cached == null || cached.failedUntil !== 0) {
+		entry.failedUntil = 0;
+		startImageLoad(entry);
+	}
 	return () => {
 		entry.subscribers.delete(subscriber);
 	};

fluxer_app/src/features/ui/components/BaseAvatar.tsx

@@ -175,7 +175,7 @@ export const BaseAvatar = React.forwardRef<HTMLDivElement, BaseAvatarProps>(
 		const maskIsMobileOnline = shouldShowCustomStatusBadge ? false : isMobileOnline;
 		const reducedMotion = Accessibility.useReducedMotion;
 		const candidateUrl = animatedMediaPlaybackEnabled ? hoverAvatarUrl || '' : avatarUrl;
-		const [imgError, setImgError] = useState(false);
+		const [imgError, setImgError] = useState(() => ImageCacheUtils.hasFailedImage(candidateUrl));
 		const [imgRetryCount, setImgRetryCount] = useState(0);
 		const [wasCachedAtMount] = useState(() => ImageCacheUtils.hasImage(candidateUrl));
 		const [mountedAt] = useState(() => Date.now());
@@ -183,7 +183,7 @@ export const BaseAvatar = React.forwardRef<HTMLDivElement, BaseAvatarProps>(
 			wasCachedAtMount ? {url: candidateUrl, fade: 'instant'} : null,
 		);
 		useEffect(() => {
-			setImgError(false);
+			setImgError(ImageCacheUtils.hasFailedImage(candidateUrl));
 			setImgRetryCount(0);
 		}, [candidateUrl]);
 		useEffect(() => {

fluxer_app/src/features/user/state/UserSettings.ts

@@ -11,8 +11,10 @@ import {
 	selectEffectiveGifAutoPlay,
 } from '@app/features/accessibility/state/MotionPreferencesMachine';
 import {Endpoints} from '@app/features/app/constants/Endpoints';
+import {resolveRetryAfterMs} from '@app/features/messaging/utils/RetryAfterUtils';
 import AppStorage from '@app/features/platform/state/PersistentStorage';
 import {http} from '@app/features/platform/transport/RestTransport';
+import {HttpError} from '@app/features/platform/types/EndpointError';
 import {Logger} from '@app/features/platform/utils/AppLogger';
 import LocalPresence, {setLocalPresenceUserSettings} from '@app/features/presence/state/LocalPresence';
 import Theme from '@app/features/theme/state/Theme';
@@ -1020,7 +1022,7 @@ class UserSettingsState {
 			}
 			if (this.isRateLimitError(error)) {
 				this.syncConsecutive429s += 1;
-				const retryAfterMs = this.extractRetryAfterMs(error) ?? this.syncBackoffMs();
+				const retryAfterMs = this.syncRetryDelayMs(error);
 				logger.warn(
 					`synced_preferences PATCH rate-limited; retry in ${Math.round(retryAfterMs / 1000)}s ` +
 						`(attempt ${this.syncConsecutive429s})`,
@@ -1055,23 +1057,9 @@ class UserSettingsState {
 		return status === 429;
 	}
 
-	private extractRetryAfterMs(error: unknown): number | null {
+	private syncRetryDelayMs(error: unknown): number {
-		if (error == null || typeof error !== 'object') return null;
+		const advertisedMs = error instanceof HttpError ? resolveRetryAfterMs(error) : null;
-		const candidate =
+		return Math.min(60000, Math.max(advertisedMs ?? 0, this.syncBackoffMs()));
-			(
-				error as {
-					retryAfter?: unknown;
-				}
-			).retryAfter ??
-			(
-				error as {
-					body?: {
-						retry_after?: unknown;
-					};
-				}
-			).body?.retry_after;
-		if (typeof candidate !== 'number' || !Number.isFinite(candidate) || candidate < 0) return null;
-		return Math.min(60000, Math.max(250, candidate * 1000));
 	}
 
 	async saveSettings(settings: Partial<UserSettings>): Promise<void> {