From a6a1db5254df60bfa30ce0e95a525b343fa3b058 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Sun, 6 Sep 2026 19:36:36 +0200 Subject: [PATCH 1/3] fix(gallery): release grid tiles once they are far enough out of view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pre-load band made tiles arrive in time. It did nothing about them never leaving. PhotoCard latched its observer with triggerOnce, so a tile that had been scrolled past stayed mounted for the life of the page — holding its object URL, and where image protection is on a canvas sized to the image that the browser is not permitted to evict. Measured in Chrome on a seeded 546-photo grid, scrolling top to bottom: mounted tiles climb 24 → 100 → 212 → 364 → 546 and never fall. That is a monotonically growing retained set, which is the profile a memory-constrained browser discards a tab over — the reported symptom on iOS Safari 18.1 being tiles that stop appearing and a blank page after refresh. With this change the same scroll peaks at 68. PhotoCard now takes an optional outer band. The inner band, unchanged, decides when a tile starts loading; the outer one decides when it is far enough away to unmount, and unmounting is what actually frees anything, because AuthenticatedImage revokes its object URL and drops the canvas in its cleanup. The gap between the bands is the hysteresis: at three viewport heights against a one-viewport load band, a tile travels two further viewport heights after it stops loading before it is released, so ordinary scrolling never crosses both edges. Thumbnails are served private, max-age=1800, so returning costs a cache hit rather than a round trip. Opt-in per layout, and only Grid opts in. Its skeleton is aspect-square and holds the tile's box exactly, so releasing shifts nothing; the measured layouts have no such guarantee. Without the prop the observer keeps its original latch, so every other layout behaves exactly as before — pinned by a test, since that is the half most easily broken by accident. This is not presented as the fix for the iOS report. It removes the mechanism that best explains it, and it is worth having on any device; whether it is the mechanism still needs a measurement from the phone that failed. Relates to issue 1287 --- frontend/src/components/gallery/PhotoCard.tsx | 49 ++++- .../__tests__/PhotoCard.releaseBand.test.tsx | 183 ++++++++++++++++++ .../gallery/layouts/GridGalleryLayout.tsx | 20 ++ .../layouts/__tests__/gridLazyMargin.test.ts | 20 ++ 4 files changed, 266 insertions(+), 6 deletions(-) create mode 100644 frontend/src/components/gallery/__tests__/PhotoCard.releaseBand.test.tsx diff --git a/frontend/src/components/gallery/PhotoCard.tsx b/frontend/src/components/gallery/PhotoCard.tsx index 5e27a33b..62fc61f7 100644 --- a/frontend/src/components/gallery/PhotoCard.tsx +++ b/frontend/src/components/gallery/PhotoCard.tsx @@ -35,6 +35,13 @@ export interface PhotoCardProps { /** Lazy-render via IntersectionObserver with a skeleton placeholder. */ lazy?: boolean; inViewRootMargin?: string; + /** + * Outer band, in `rootMargin` form. When set, a tile that leaves it is + * unmounted again rather than kept for the life of the page (#1287). Opt-in + * per layout: only a layout whose skeleton holds the tile's box can release + * without reflowing, which today is Grid (`aspect-square`). + */ + releaseRootMargin?: string; skeletonClassName?: string; /** Keep container at opacity 0 until in view (only meaningful with `lazy`). */ fadeInWhenVisible?: boolean; @@ -85,6 +92,7 @@ export const PhotoCard: React.FC = ({ imageProps, lazy = false, inViewRootMargin, + releaseRootMargin, skeletonClassName = 'skeleton w-full h-full rounded-lg', fadeInWhenVisible = false, overlayBaseClassName, @@ -157,13 +165,40 @@ export const PhotoCard: React.FC = ({ } }, [isSelectionMode, hideOverlay]); - // Lazy loading with intersection observer - const { ref, inView: observedInView } = useInView({ - triggerOnce: true, + // Lazy loading with intersection observer. + // + // Two bands with a deliberate gap between them (#1287). The inner one, from + // `inViewRootMargin`, decides when a tile starts loading. The outer one, + // from `releaseRootMargin`, decides when it is far enough away to unmount — + // and unmounting is the part that frees anything, because AuthenticatedImage + // revokes its object URL and drops any protection canvas in its cleanup, and + // neither is reclaimable while the tile stays mounted. On a 546-photo grid + // the old latch meant every tile scrolled past was retained for the life of + // the page, which is the memory profile iOS Safari discards a tab over. + // + // The gap between the bands is the hysteresis: a tile is not released until + // it is well outside the band that would immediately reload it, so scrolling + // back and forth across one edge cannot thrash. Without a release band the + // observer keeps its original `triggerOnce` latch, so every other layout + // behaves exactly as before. + const releases = Boolean(lazy && releaseRootMargin); + const { ref: loadBandRef, inView: withinLoadBand } = useInView({ + triggerOnce: !releases, threshold: 0.1, rootMargin: inViewRootMargin, }); - const inView = !lazy || observedInView; + const { ref: keepBandRef, inView: withinKeepBand } = useInView({ + skip: !releases, + threshold: 0, + rootMargin: releaseRootMargin, + }); + const [rendered, setRendered] = useState(false); + useEffect(() => { + if (!releases) return; + if (withinLoadBand) setRendered(true); + else if (!withinKeepBand) setRendered(false); + }, [releases, withinLoadBand, withinKeepBand]); + const inView = !lazy || (releases ? rendered : withinLoadBand); // Tile width for the responsive tier (#1095), measured rather than inferred. // The observer entry only exists for `lazy` cards, and Mosaic, Masonry and @@ -183,8 +218,10 @@ export const PhotoCard: React.FC = ({ const [tile, setTile] = useState<{ width: number | null } | null>(null); const setContainerRef = useCallback((node: HTMLDivElement | null) => { containerRef.current = node; - if (lazy) ref(node); - }, [ref, lazy]); + if (!lazy) return; + loadBandRef(node); + if (releases) keepBandRef(node); + }, [loadBandRef, keepBandRef, lazy, releases]); useLayoutEffect(() => { if (!inView || tile) return; diff --git a/frontend/src/components/gallery/__tests__/PhotoCard.releaseBand.test.tsx b/frontend/src/components/gallery/__tests__/PhotoCard.releaseBand.test.tsx new file mode 100644 index 00000000..0892bb7b --- /dev/null +++ b/frontend/src/components/gallery/__tests__/PhotoCard.releaseBand.test.tsx @@ -0,0 +1,183 @@ +/** + * Releasing offscreen grid tiles (#1287). + * + * The pre-load band made tiles arrive in time; it did nothing about them never + * leaving. A tile that has been scrolled past keeps its object URL, and where + * image protection is on a full-resolution canvas the browser may not evict, + * for the life of the page — so a several-hundred-photo grid grows monotonically + * until the browser discards the tab. + * + * Unmounting is what frees those, so that is what these assert: the tile's + * subtree really goes away when it is far enough out, really comes back, and + * never flickers on ordinary scrolling. The `releaseRootMargin`-less path is + * pinned too, because every other layout still depends on the old latch. + */ +import React from 'react'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { render, screen } from '@testing-library/react'; + +import { PhotoCard } from '../PhotoCard'; +import type { Photo } from '../../../types'; + +const LOAD_BAND = '100% 0px'; +const KEEP_BAND = '300% 0px'; + +/** Which bands the tile is currently inside; the test drives these directly. */ +let bands: Record = {}; +// `triggerOnce` has to be modelled, not ignored: it is the whole difference +// between a layout that opts in to releasing and one that does not, so a mock +// that always reports live visibility would quietly turn the latch test into a +// test of the mock. +vi.mock('react-intersection-observer', () => ({ + useInView: ( + { rootMargin, skip, triggerOnce }: + { rootMargin?: string; skip?: boolean; triggerOnce?: boolean }, + ) => { + const latched = React.useRef(false); + const live = skip ? false : Boolean(bands[rootMargin ?? '']); + if (live) latched.current = true; + return { ref: () => {}, inView: triggerOnce ? latched.current : live }; + }, +})); + +// Counting mounts and unmounts is the whole point: it is the unmount that +// revokes the object URL and drops the canvas. +const lifecycle = { mounted: 0, unmounted: 0 }; +vi.mock('../../common', () => ({ + AuthenticatedImage: ({ src }: { src: string }) => { + React.useEffect(() => { + lifecycle.mounted += 1; + return () => { lifecycle.unmounted += 1; }; + }, []); + return ; + }, +})); +vi.mock('../../../contexts/GuestIdentityContext', () => ({ + useGuestIdentityOptional: () => null, +})); + +const PHOTO = { + id: 7, + filename: 'IMG_0001.jpg', + url: '/api/gallery/x/photo/7', + thumbnail_url: '/api/gallery/x/thumbnail/7', + type: 'individual', + size: 1, + uploaded_at: '2026-01-01T00:00:00Z', +} as Photo; + +function renderCard(props: Partial> = {}) { + return render( + {}} + onDownload={() => {}} + onToggleSelect={() => {}} + className="tile aspect-square" + skeletonClassName="skeleton aspect-square" + overlayBaseClassName="overlay" + imageProps={{ src: PHOTO.thumbnail_url!, alt: PHOTO.filename }} + lazy + inViewRootMargin={LOAD_BAND} + releaseRootMargin={KEEP_BAND} + {...props} + />, + ); +} + +const scrollTo = ( + rerender: (ui: React.ReactElement) => void, + next: Record, + props: Partial> = {}, +) => { + bands = next; + rerender( + {}} + onDownload={() => {}} + onToggleSelect={() => {}} + className="tile aspect-square" + skeletonClassName="skeleton aspect-square" + overlayBaseClassName="overlay" + imageProps={{ src: PHOTO.thumbnail_url!, alt: PHOTO.filename }} + lazy + inViewRootMargin={LOAD_BAND} + releaseRootMargin={KEEP_BAND} + {...props} + />, + ); +}; + +beforeEach(() => { + bands = {}; + lifecycle.mounted = 0; + lifecycle.unmounted = 0; + Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { + configurable: true, + get() { return 200; }, + }); +}); + +describe('grid tiles release when they are far enough out of view', () => { + it('does not load a tile that is only inside the outer band', () => { + bands = { [KEEP_BAND]: true }; + renderCard(); + expect(screen.queryByTestId('tile')).toBeNull(); + expect(lifecycle.mounted).toBe(0); + }); + + it('loads inside the pre-load band and releases past the outer band', () => { + bands = { [LOAD_BAND]: true, [KEEP_BAND]: true }; + const { rerender } = renderCard(); + expect(screen.getByTestId('tile')).toBeTruthy(); + expect(lifecycle.mounted).toBe(1); + + // Scrolled well past: outside both bands. + scrollTo(rerender, {}); + expect(screen.queryByTestId('tile')).toBeNull(); + // The unmount is the release — object URL revoked, canvas dropped. + expect(lifecycle.unmounted).toBe(1); + }); + + it('holds the tile in the gap between the bands, so scrolling cannot thrash', () => { + bands = { [LOAD_BAND]: true, [KEEP_BAND]: true }; + const { rerender } = renderCard(); + + // Past the load band but still within the keep band — the hysteresis gap. + scrollTo(rerender, { [KEEP_BAND]: true }); + expect(screen.getByTestId('tile')).toBeTruthy(); + expect(lifecycle.unmounted).toBe(0); + + // Back towards the viewport without ever having been released. + scrollTo(rerender, { [LOAD_BAND]: true, [KEEP_BAND]: true }); + expect(lifecycle.mounted).toBe(1); + }); + + it('brings a released tile back when it returns', () => { + bands = { [LOAD_BAND]: true, [KEEP_BAND]: true }; + const { rerender } = renderCard(); + scrollTo(rerender, {}); + expect(screen.queryByTestId('tile')).toBeNull(); + + scrollTo(rerender, { [LOAD_BAND]: true, [KEEP_BAND]: true }); + expect(screen.getByTestId('tile')).toBeTruthy(); + expect(lifecycle.mounted).toBe(2); + }); + + it('keeps the old latch for layouts that do not opt in', () => { + bands = { [LOAD_BAND]: true }; + const { rerender } = renderCard({ releaseRootMargin: undefined }); + expect(screen.getByTestId('tile')).toBeTruthy(); + + // Far outside everything. A measured layout has no skeleton that holds the + // box, so releasing there would reflow — it must stay mounted. + scrollTo(rerender, {}, { releaseRootMargin: undefined }); + expect(screen.getByTestId('tile')).toBeTruthy(); + expect(lifecycle.unmounted).toBe(0); + }); +}); diff --git a/frontend/src/components/gallery/layouts/GridGalleryLayout.tsx b/frontend/src/components/gallery/layouts/GridGalleryLayout.tsx index 66a45cd5..e10f5c6e 100644 --- a/frontend/src/components/gallery/layouts/GridGalleryLayout.tsx +++ b/frontend/src/components/gallery/layouts/GridGalleryLayout.tsx @@ -101,6 +101,26 @@ const GridPhoto: React.FC = ({ * of lead in each direction, which is what vh would have meant. */ inViewRootMargin="100% 0px" + /* + * Release band (#1287). The pre-load band above fixed tiles arriving + * late; it did nothing about tiles never leaving. Every tile scrolled + * past kept its object URL — and, where image protection is on, a + * full-resolution canvas that the browser is not allowed to evict — for + * the life of the page. On a several-hundred-photo gallery that grows + * monotonically, which is the shape a memory-constrained browser + * discards the tab over. + * + * Three viewport heights, against a one-viewport load band: a tile has + * to travel two further viewport heights after it stops loading before + * it is released, so ordinary scrolling never crosses both edges. + * Thumbnails are served `private, max-age=1800`, so coming back costs a + * cache hit rather than a round trip. + * + * Grid only, and deliberately so: the skeleton here is `aspect-square` + * and holds the tile's box exactly, so releasing shifts nothing. The + * measured layouts have no such guarantee. + */ + releaseRootMargin="300% 0px" fadeInWhenVisible={animationType === 'fade'} skeletonClassName="skeleton aspect-square w-full rounded-lg" imageProps={{ diff --git a/frontend/src/components/gallery/layouts/__tests__/gridLazyMargin.test.ts b/frontend/src/components/gallery/layouts/__tests__/gridLazyMargin.test.ts index 13fc77c2..55688a40 100644 --- a/frontend/src/components/gallery/layouts/__tests__/gridLazyMargin.test.ts +++ b/frontend/src/components/gallery/layouts/__tests__/gridLazyMargin.test.ts @@ -45,6 +45,26 @@ describe('grid lazy pre-load band', () => { } }); + it('Grid releases what it loaded, and the outer band is legal and wider', () => { + // The pre-load band fixed tiles arriving late; it did nothing about them + // never leaving. Measured in Chrome on a seeded 546-photo grid: without a + // release band the mounted count climbs 24 → 100 → 212 → 364 → 546 and + // never falls, because a tile that has been scrolled past keeps its object + // URL and any protection canvas for the life of the page. With it the peak + // is 68. + const src = read('GridGalleryLayout.tsx'); + const release = src.match(/releaseRootMargin="([^"]+)"/); + expect(release, 'Grid declares no releaseRootMargin').toBeTruthy(); + expect(release![1]).toMatch(LEGAL_ROOT_MARGIN); + + // The gap between the bands is the hysteresis. If the outer band were not + // strictly wider, a tile would be released and immediately reloaded on + // every scroll across the edge. + const load = src.match(/inViewRootMargin="([^"]+)"/); + const percent = (value: string) => Number(value.split(/\s+/)[0].replace('%', '')); + expect(percent(release![1])).toBeGreaterThan(percent(load![1])); + }); + it('every layout that lazy-renders also declares a pre-load band', () => { // The defect was Grid being lazy with no margin. Any future layout that // opts into `lazy` and forgets the margin reintroduces it. From 77ae94e649f367bb0a44166d3215ce1884c660d2 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Sun, 6 Sep 2026 19:59:53 +0200 Subject: [PATCH 2/3] fix(gallery): retry a failed image fetch once the tile is back on screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A rejected fetch in AuthenticatedImage set the error state, rendered nothing, and never asked again. The fetch effect only re-runs when its inputs change, and for a grid tile they never do — so a transient failure (a hiccup on cellular, or Safari cancelling loads when the tab goes to the background) was a permanently blank tile with no request in flight and nothing in any log. Grid passes no fallbackSrc, so there was not even a broken-image icon to point at. The retry is bounded and gated. Three attempts with a doubling delay (2 s, 4 s, 8 s), and an attempt fires only once the placeholder intersects the viewport and the document is visible, so a tile that failed while the user was away retries when they come back rather than while they are still gone. A new src gets a fresh budget. The fallbackSrc path is untouched: it already renders a plain and should not loop. Two refinements from review. A final 4xx (anything but 408 and 429) exhausts the budget at once: an expired gallery token or a missing photo cannot be retried into existence, and on a 68-tile viewport three retries each would be ~200 requests that cannot succeed. And a 429's Retry-After is honoured as the minimum delay, because the backoff alone would spend every retry inside a 15-minute rate-limit window and leave the tile blank after the limit had lifted. Retry-After is not CORS-safelisted, so server.js now exposes it for split-origin deployments alongside Content-Disposition. The error branch now renders the same grey box as the loading state instead of null. That is what the retry effect observes, and it is also something the user can see. The empty-src branch now clears the error flag too, so a tile whose src is removed after a failure does not keep showing the failure box. Nine tests in AuthenticatedImage.retry.test.tsx; the retry cases fail against the previous version. Not presented as the fix for the iOS report. It closes the one gap that turns a transient failure into a permanent one, which the reporter asked for in the original issue, and it is worth having on any device. Relates to issue 1287 --- backend/server.js | 7 +- .../components/common/AuthenticatedImage.tsx | 137 ++++++++++- .../AuthenticatedImage.retry.test.tsx | 216 ++++++++++++++++++ 3 files changed, 357 insertions(+), 3 deletions(-) create mode 100644 frontend/src/components/common/__tests__/AuthenticatedImage.retry.test.tsx diff --git a/backend/server.js b/backend/server.js index 33c3aa7a..02a7c715 100644 --- a/backend/server.js +++ b/backend/server.js @@ -239,7 +239,12 @@ const corsOptions = { // deployments can read the server's chosen download filename. Used // by the gallery/admin download flows to honour the #493 "original // camera filename" toggle on individual photo downloads (#507). - exposedHeaders: ['Content-Disposition'], + // + // Retry-After is not CORS-safelisted either. AuthenticatedImage reads it + // off a 429 to wait out the rate-limit window before retrying a thumbnail + // fetch; without it a split-origin deployment would spend its retry budget + // inside the window and leave the tile blank after the limit had lifted. + exposedHeaders: ['Content-Disposition', 'Retry-After'], }; // Only attach CORS to API endpoints, not static assets diff --git a/frontend/src/components/common/AuthenticatedImage.tsx b/frontend/src/components/common/AuthenticatedImage.tsx index 0437413a..0db0b7ad 100644 --- a/frontend/src/components/common/AuthenticatedImage.tsx +++ b/frontend/src/components/common/AuthenticatedImage.tsx @@ -30,6 +30,44 @@ interface AuthenticatedImageProps extends Omit + status >= 400 && status < 500 && status !== 408 && status !== 429; + /** * Fetches an image with the gallery's bearer token and renders it. * @@ -76,6 +114,17 @@ export const AuthenticatedImage: React.FC = ({ const [canvasFailed, setCanvasFailed] = useState(false); const canvasRef = useRef(null); const imageRef = useRef(null); + // Retry state (#1287). The nonce is a dependency of the fetch effect, so + // bumping it is the retry; the counter is per src, so a new image gets a + // fresh budget without an extra effect run to reset it. + const [retryNonce, setRetryNonce] = useState(0); + const attemptsRef = useRef(0); + // Cooldown the server asked for on the last failure (#1287). The backoff + // alone would spend all three retries inside a 15-minute rate-limit window + // and leave the tile blank after the limit had actually lifted. + const retryAfterRef = useRef(0); + const lastSrcRef = useRef(undefined); + const retryRef = useRef(null); // Draw image to canvas when canvas rendering is enabled // Returns whether the pixels actually made it onto the canvas, so the @@ -108,9 +157,15 @@ export const AuthenticatedImage: React.FC = ({ // screen, which on a several-hundred-photo gallery is most of them. const controller = new AbortController(); + if (lastSrcRef.current !== src) { + lastSrcRef.current = src; + attemptsRef.current = 0; + } + // Determine which token to use based on context if (!src) { setImageSrc(fallbackSrc || ''); + setError(false); setIsLoading(false); return; } @@ -175,7 +230,11 @@ export const AuthenticatedImage: React.FC = ({ }); if (!response.ok) { - throw new Error(`Failed to fetch image: ${response.status} ${response.statusText}`); + throw new HttpError( + response.status, + response.statusText, + response.headers?.get?.('Retry-After') ?? null, + ); } return await response.blob(); @@ -203,6 +262,13 @@ export const AuthenticatedImage: React.FC = ({ // Returning here also stops the fallback below from firing a second // request against an already-aborted signal. if (aborted) return; + // A final 4xx exhausts the retry budget: on a 68-tile viewport, three + // retries per tile against an expired token would be ~200 requests + // that cannot succeed. + if (err instanceof HttpError && isFinalStatus(err.status)) { + attemptsRef.current = MAX_RETRIES; + } + retryAfterRef.current = err instanceof HttpError ? err.retryAfterMs : 0; setIsLoading(false); if (fallbackSrc && fallbackSrc !== src) { try { @@ -238,7 +304,65 @@ export const AuthenticatedImage: React.FC = ({ objectUrls.forEach((url) => URL.revokeObjectURL(url)); }; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [src, fallbackSrc, slug, queuePriority]); + }, [src, fallbackSrc, slug, queuePriority, retryNonce]); + + // Retry a failed fetch once the tile is back on screen (#1287). + // + // Before this, a rejected fetch set `error` and nothing ever asked again: + // the effect above only re-runs when its inputs change, and for a grid + // tile they never do. On the original reporter's install that was the + // difference between a transient failure and a permanently blank tile — + // a hiccup on cellular, or Safari cancelling loads when the tab goes to + // the background, left a tile with no image, no request in flight and + // nothing in any log, for as long as the gallery stayed open. That is the + // retry-on-scrolling-back-into-view the reporter asked for in the issue. + // + // Bounded, and gated on visibility. The delay doubles per attempt so a + // server that is actually down is not hammered, and an attempt does not + // fire until the placeholder intersects the viewport and the document is + // visible — a tile that failed while backgrounded retries when the user + // comes back, not while they are still away. Without IntersectionObserver + // the placeholder counts as visible. + const retryable = error && !fallbackSrc && attemptsRef.current < MAX_RETRIES; + useEffect(() => { + if (!retryable) return; + const el = retryRef.current; + if (!el) return; + + let cancelled = false; + let delayElapsed = false; + let onScreen = typeof IntersectionObserver === 'undefined'; + + const retry = () => { + if (cancelled || !delayElapsed || !onScreen) return; + if (typeof document !== 'undefined' && document.visibilityState === 'hidden') return; + cancelled = true; + attemptsRef.current += 1; + setRetryNonce((n) => n + 1); + }; + + const timer = setTimeout(() => { + delayElapsed = true; + retry(); + }, Math.max(RETRY_BASE_DELAY_MS * 2 ** attemptsRef.current, retryAfterRef.current)); + + let observer: IntersectionObserver | null = null; + if (typeof IntersectionObserver !== 'undefined') { + observer = new IntersectionObserver((entries) => { + onScreen = entries.some((entry) => entry.isIntersecting); + retry(); + }); + observer.observe(el); + } + document.addEventListener('visibilitychange', retry); + + return () => { + cancelled = true; + clearTimeout(timer); + observer?.disconnect(); + document.removeEventListener('visibilitychange', retry); + }; + }, [retryable, retryNonce]); // Effect to draw to canvas when image is loaded and canvas rendering is enabled useEffect(() => { @@ -314,6 +438,15 @@ export const AuthenticatedImage: React.FC = ({ return {alt}; } + if (error) { + // Same box as the loading placeholder, and the element the retry effect + // observes. Returning null here (as this used to) left nothing to watch + // and nothing for the user to see either. + return ( +
+ ); + } + if (!imageSrc) { return null; } diff --git a/frontend/src/components/common/__tests__/AuthenticatedImage.retry.test.tsx b/frontend/src/components/common/__tests__/AuthenticatedImage.retry.test.tsx new file mode 100644 index 00000000..fb3c82ef --- /dev/null +++ b/frontend/src/components/common/__tests__/AuthenticatedImage.retry.test.tsx @@ -0,0 +1,216 @@ +/** + * Retry after a failed fetch (#1287). + * + * A rejected fetch used to set `error`, render nothing, and never ask again: + * the fetch effect only re-runs when its inputs change, and for a grid tile + * they never do. A transient failure — a hiccup on cellular, Safari + * cancelling loads when the tab is backgrounded — was therefore a + * permanently blank tile with nothing in any log. + * + * The retry is bounded (three attempts, doubling delay) and gated on the + * placeholder being on screen and the document being visible. + */ +import { act, render } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +vi.mock('../../../utils/galleryAuthStorage', () => ({ + getActiveGallerySlug: () => 'demo', + getGalleryToken: () => 'token', + inferGallerySlugFromLocation: () => 'demo', + resolveSlugFromRequestUrl: () => 'demo', +})); +vi.mock('../../../utils/url', () => ({ buildResourceUrl: (u: string) => `http://localhost${u}` })); + +import { AuthenticatedImage } from '../AuthenticatedImage'; + +const ok = () => Promise.resolve({ + ok: true, + blob: async () => new Blob(['x'], { type: 'image/png' }), +}); +const loadFailed = () => Promise.reject(new TypeError('Load failed')); +const status = (code: number, headers: Record = {}) => () => Promise.resolve({ + ok: false, + status: code, + statusText: 'x', + headers: { get: (name: string) => headers[name] ?? null }, +}); + +/** The observer callbacks registered by the component, so a test can drive them. */ +type IOCallback = (entries: Array<{ isIntersecting: boolean }>) => void; +let observers: IOCallback[]; + +beforeEach(() => { + vi.useFakeTimers(); + observers = []; + URL.createObjectURL = vi.fn(() => 'blob:mock') as unknown as typeof URL.createObjectURL; + URL.revokeObjectURL = vi.fn() as unknown as typeof URL.revokeObjectURL; +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.useRealTimers(); +}); + +/** Let every pending promise in the fetch/queue chain settle. */ +const flush = () => act(async () => { await vi.advanceTimersByTimeAsync(0); }); +const advance = (ms: number) => act(async () => { await vi.advanceTimersByTimeAsync(ms); }); + +/** IntersectionObserver stub that never fires on its own; tests drive it. */ +function stubIntersectionObserver() { + vi.stubGlobal('IntersectionObserver', class { + constructor(cb: IOCallback) { observers.push(cb); } + observe() {} + disconnect() {} + unobserve() {} + }); +} + +describe('AuthenticatedImage retry', () => { + it('retries a failed fetch after the delay and renders the image', async () => { + const fetchMock = vi.fn().mockImplementationOnce(loadFailed).mockImplementation(ok); + vi.stubGlobal('fetch', fetchMock); + + const { container } = render(); + await flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(container.querySelector('img')).toBeNull(); + + await advance(1999); + expect(fetchMock).toHaveBeenCalledTimes(1); + + await advance(1); + await flush(); + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(container.querySelector('img')?.getAttribute('src')).toBe('blob:mock'); + }); + + it('gives up after three retries with a doubling delay', async () => { + const fetchMock = vi.fn().mockImplementation(loadFailed); + vi.stubGlobal('fetch', fetchMock); + + render(); + await flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + + await advance(2000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(2); + await advance(4000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(3); + await advance(8000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(4); + + await advance(60_000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(4); + }); + + it('waits until the placeholder is on screen before retrying', async () => { + stubIntersectionObserver(); + const fetchMock = vi.fn().mockImplementationOnce(loadFailed).mockImplementation(ok); + vi.stubGlobal('fetch', fetchMock); + + const { container } = render(); + await flush(); + expect(observers).toHaveLength(1); + + // Delay elapsed, but the tile is not on screen: no retry. + await advance(2000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + + // Scrolled back into view: retry fires now, without another delay. + act(() => observers[0]([{ isIntersecting: true }])); + await flush(); + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(container.querySelector('img')).not.toBeNull(); + }); + + it('does not retry while the document is hidden, and does when it comes back', async () => { + const fetchMock = vi.fn().mockImplementationOnce(loadFailed).mockImplementation(ok); + vi.stubGlobal('fetch', fetchMock); + const visibility = vi.spyOn(document, 'visibilityState', 'get').mockReturnValue('hidden'); + + render(); + await flush(); + await advance(2000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + + visibility.mockReturnValue('visible'); + act(() => { document.dispatchEvent(new Event('visibilitychange')); }); + await flush(); + expect(fetchMock).toHaveBeenCalledTimes(2); + visibility.mockRestore(); + }); + + it('does not retry a final 4xx such as an expired token', async () => { + const fetchMock = vi.fn().mockImplementation(status(401)); + vi.stubGlobal('fetch', fetchMock); + + render(); + await flush(); + await advance(60_000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('retries a 5xx and a 429', async () => { + const fetchMock = vi.fn() + .mockImplementationOnce(status(503)) + .mockImplementationOnce(status(429)) + .mockImplementation(ok); + vi.stubGlobal('fetch', fetchMock); + + const { container } = render(); + await flush(); + await advance(2000); await flush(); + await advance(4000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(3); + expect(container.querySelector('img')?.getAttribute('src')).toBe('blob:mock'); + }); + + it('waits out a Retry-After longer than the backoff before spending a retry', async () => { + const fetchMock = vi.fn() + .mockImplementationOnce(status(429, { 'Retry-After': '30' })) + .mockImplementation(ok); + vi.stubGlobal('fetch', fetchMock); + + const { container } = render(); + await flush(); + // The 2 s backoff alone would have fired here. + await advance(29_999); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(1); + await advance(1); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(container.querySelector('img')).not.toBeNull(); + }); + + it('leaves the fallbackSrc path alone', async () => { + const fetchMock = vi.fn().mockImplementation(loadFailed); + vi.stubGlobal('fetch', fetchMock); + + const { container } = render( + , + ); + await flush(); + // primary + fallback, then the plain takes over — no retry loop. + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(container.querySelector('img')?.getAttribute('src')).toBe('/static/fallback.png'); + await advance(60_000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it('a new src gets a fresh retry budget', async () => { + const fetchMock = vi.fn().mockImplementation(loadFailed); + vi.stubGlobal('fetch', fetchMock); + + const { rerender } = render(); + await flush(); + await advance(2000); await flush(); + await advance(4000); await flush(); + await advance(8000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(4); + + rerender(); + await flush(); + expect(fetchMock).toHaveBeenCalledTimes(5); + await advance(2000); await flush(); + expect(fetchMock).toHaveBeenCalledTimes(6); + }); +}); From 6459bff50eab924bdcdc687e359e38785a6cf297 Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Sun, 6 Sep 2026 21:13:42 +0200 Subject: [PATCH 3/3] chore(main): release 3.126.2-beta.0 (#1319) --- .release-please-manifest-beta.json | 2 +- CHANGELOG.md | 9 +++++++++ backend/package.json | 2 +- frontend/package.json | 2 +- 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/.release-please-manifest-beta.json b/.release-please-manifest-beta.json index 9559ddcb..444c772f 100644 --- a/.release-please-manifest-beta.json +++ b/.release-please-manifest-beta.json @@ -1,3 +1,3 @@ { - ".": "3.126.1-beta.0" + ".": "3.126.2-beta.0" } diff --git a/CHANGELOG.md b/CHANGELOG.md index 7eb9b288..c53a0928 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,15 @@ All notable changes to PicPeak will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [3.126.2-beta.0](https://github.com/PicPeak/picpeak/compare/v3.126.1-beta.0...v3.126.2-beta.0) (2026-09-06) + + +### Bug Fixes + +* **gallery:** release grid tiles once they are far enough out of view ([b3937d0](https://github.com/PicPeak/picpeak/commit/b3937d0b8c54c8ddf8a428be88aa444ab2b4f2d2)) +* **gallery:** retry a failed image fetch once the tile is back on screen ([c4b03a8](https://github.com/PicPeak/picpeak/commit/c4b03a831f843ec447a54d712aefa89c9761e8e8)) +* **gallery:** retry a failed image fetch once the tile is back on screen ([77ae94e](https://github.com/PicPeak/picpeak/commit/77ae94e649f367bb0a44166d3215ce1884c660d2)) + ## [3.126.1-beta.0](https://github.com/PicPeak/picpeak/compare/v3.126.0-beta.0...v3.126.1-beta.0) (2026-09-06) diff --git a/backend/package.json b/backend/package.json index 2ca11a3a..2b51302b 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "picpeak-backend", - "version": "3.126.1-beta.0", + "version": "3.126.2-beta.0", "description": "Backend for PicPeak event photo sharing platform", "main": "server.js", "engines": { diff --git a/frontend/package.json b/frontend/package.json index c9bbdaa1..8e030c4f 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "picpeak-frontend", "private": true, - "version": "3.126.1-beta.0", + "version": "3.126.2-beta.0", "type": "module", "scripts": { "dev": "vite",