diff --git a/fluxer_app/src/features/messaging/commands/ReactionCommands.tsx b/fluxer_app/src/features/messaging/commands/ReactionCommands.tsx index 8c785d8fb..aa930e3f7 100644 --- a/fluxer_app/src/features/messaging/commands/ReactionCommands.tsx +++ b/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(func: () => Promise, attempts = try { return await func(); } catch (error) { - const status = error instanceof HttpError ? error.status : undefined; - if (status !== 429) { + if (!(error instanceof HttpError) || error.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 ( - checkReactionResponse(i18n, error, () => - performReactionAction(i18n, type, apiFunc, channelId, messageId, emoji, userId), - ) - ) { + if (checkReactionResponse(i18n, error)) { 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); }); } diff --git a/fluxer_app/src/features/messaging/utils/ImageCacheUtils.test.ts b/fluxer_app/src/features/messaging/utils/ImageCacheUtils.test.ts new file mode 100644 index 000000000..590dcb85f --- /dev/null +++ b/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 = []; + 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); + }); +}); diff --git a/fluxer_app/src/features/messaging/utils/ImageCacheUtils.ts b/fluxer_app/src/features/messaging/utils/ImageCacheUtils.ts index 71c559b3c..59403b55f 100644 --- a/fluxer_app/src/features/messaging/utils/ImageCacheUtils.ts +++ b/fluxer_app/src/features/messaging/utils/ImageCacheUtils.ts @@ -21,6 +21,7 @@ interface ImageCacheEntry { subscribers: Set; 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_INITIAL_DELAY_MS = 500; +const IMAGE_RETRY_ATTEMPT_LIMIT = 2; +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({ 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); }; diff --git a/fluxer_app/src/features/ui/components/BaseAvatar.tsx b/fluxer_app/src/features/ui/components/BaseAvatar.tsx index 55b74af4a..7a2518e34 100644 --- a/fluxer_app/src/features/ui/components/BaseAvatar.tsx +++ b/fluxer_app/src/features/ui/components/BaseAvatar.tsx @@ -175,7 +175,7 @@ export const BaseAvatar = React.forwardRef( 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( wasCachedAtMount ? {url: candidateUrl, fade: 'instant'} : null, ); useEffect(() => { - setImgError(false); + setImgError(ImageCacheUtils.hasFailedImage(candidateUrl)); setImgRetryCount(0); }, [candidateUrl]); useEffect(() => { diff --git a/fluxer_app/src/features/user/state/UserSettings.ts b/fluxer_app/src/features/user/state/UserSettings.ts index bccd52dcb..ad6e64985 100644 --- a/fluxer_app/src/features/user/state/UserSettings.ts +++ b/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 { - if (error == null || typeof error !== 'object') return null; - const candidate = - ( - 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)); + private syncRetryDelayMs(error: unknown): number { + const advertisedMs = error instanceof HttpError ? resolveRetryAfterMs(error) : null; + return Math.min(60000, Math.max(advertisedMs ?? 0, this.syncBackoffMs())); } async saveSettings(settings: Partial): Promise {