Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
| 2026-08-20 | codex/image-auto-retry | 1fb62c61774c109d4f806b04a70f9400f35cd1f6 | signed image lazy loading and automatic recovery | P2 findings: classify automatic retries by failure status and preserve native lazy loading for cache-prefetched images before PR. | Working-tree diff and call-site inspection; focused tests passed (tests/signed-image.test.ts + tests/signed-image.dom.test.tsx: 16 passed); typecheck, lint, browser, and provider-backed checks not run. |
49 changes: 45 additions & 4 deletions src/components/clinical-dashboard/signed-image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,21 @@ import { CircleAlert, Maximize2 } from "lucide-react";
import { cn, Skeleton } from "@/components/ui-primitives";
import { Button } from "@/components/ui/button";
import { getCachedSignedUrl } from "@/lib/signed-url-cache";
import { useSignedImageUrl } from "@/components/clinical-dashboard/use-signed-image-url";
import { type SignedImageFailure, useSignedImageUrl } from "@/components/clinical-dashboard/use-signed-image-url";
import { ImageLightbox } from "@/components/clinical-dashboard/image-lightbox";

const AUTOMATIC_RETRY_DELAYS_MS = [250, 1_000] as const;

function automaticRetryDelay(failure: SignedImageFailure | null, attempt: number) {
const baseDelay = AUTOMATIC_RETRY_DELAYS_MS[attempt];
if (!failure?.retryable || baseDelay === undefined) return null;
// A 429 without a server cooldown is unsafe to guess at: another immediate
// request can only deepen the rate limit. When supplied, Retry-After is the
// earliest permissible retry, never an optional hint.
if (failure.status === 429 && failure.retryAfterMs === null) return null;
return Math.max(baseDelay, failure.retryAfterMs ?? 0);
}

/**
* Shared renderer for a private image served through a signed-URL endpoint.
*
Expand Down Expand Up @@ -81,7 +93,16 @@ export const SignedImage = memo(function SignedImage({
const frameRef = useRef<HTMLDivElement | null>(null);
const triggerRef = useRef<HTMLButtonElement>(null);
const [retryDisabled, setRetryDisabled] = useState(false);
const { url, failed, retry, markFailed } = useSignedImageUrl(endpoint, shouldLoad);
const [automaticRetryCount, setAutomaticRetryCount] = useState(0);
const [seenEndpoint, setSeenEndpoint] = useState(endpoint);
if (endpoint !== seenEndpoint) {
setSeenEndpoint(endpoint);
setAutomaticRetryCount(0);
setLoaded(false);
}
const { url, failed, failure, retry, markFailed } = useSignedImageUrl(endpoint, shouldLoad);
const nextAutomaticRetryDelay = automaticRetryDelay(failure, automaticRetryCount);
const automaticRetryPending = nextAutomaticRetryDelay !== null;

// Defer the request until the frame is near the viewport. A cached URL seeds
// `shouldLoad` synchronously, so already-fetched images skip the observer.
Expand All @@ -107,9 +128,26 @@ export const SignedImage = memo(function SignedImage({
return () => observer.disconnect();
}, [rootMargin, shouldLoad]);

// A signed-URL request or the image download can fail transiently while a
// reader scrolls through a long evidence rail. Recover nearby images without
// requiring a click, but cap the retries so a missing or unsupported asset
// still settles on the existing explicit failure action.
useEffect(() => {
if (nextAutomaticRetryDelay === null) return () => undefined;

const timer = window.setTimeout(() => {
setLoaded(false);
setShouldLoad(true);
setAutomaticRetryCount((current) => current + 1);
retry();
Comment thread
BigSimmo marked this conversation as resolved.
}, nextAutomaticRetryDelay);
return () => window.clearTimeout(timer);
}, [nextAutomaticRetryDelay, retry]);

function retryImage() {
if (retryDisabled) return;
setRetryDisabled(true);
setAutomaticRetryCount(0);
setLoaded(false);
setShouldLoad(true);
retry();
Expand All @@ -121,7 +159,7 @@ export const SignedImage = memo(function SignedImage({
markFailed();
}

if (failed) {
if (failed && !automaticRetryPending) {
return (
<div
ref={frameRef}
Expand Down Expand Up @@ -188,7 +226,10 @@ export const SignedImage = memo(function SignedImage({
// reader is actually looking at. next/image already emits
// `decoding="async"`, so this is the missing half of that pair.
fetchPriority={priority ? "high" : "low"}
onLoad={() => setLoaded(true)}
onLoad={() => {
setLoaded(true);
setAutomaticRetryCount(0);
}}
onError={handleImageError}
className={cn(
"rounded-lg object-contain transition-opacity duration-[var(--duration-deliberate)] motion-reduce:transition-none",
Expand Down
57 changes: 37 additions & 20 deletions src/components/clinical-dashboard/use-signed-image-url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,18 @@
import { useCallback, useEffect, useState } from "react";

import { authorizationIdentity } from "@/lib/authorization-header";
import { isRetryableApiStatus, parseRetryAfterMs } from "@/lib/api-client-error";
import { clearCachedSignedUrl, getCachedSignedUrl, setCachedSignedUrl } from "@/lib/signed-url-cache";
import { useAuthSession } from "@/lib/supabase/client";

type SignedUrlResponse = { status: number; data: { url?: string } | null };
type SignedUrlResponse = { status: number; data: { url?: string } | null; retryAfterMs: number | null };

export type SignedImageFailure = {
source: "response" | "network" | "image";
status: number | null;
retryable: boolean;
retryAfterMs: number | null;
};

/**
* One in-flight request per endpoint *and identity*, shared by every consumer
Expand All @@ -32,7 +40,7 @@ function beginSignedUrlRequest(key: string, endpoint: string, headers: Record<st
const request = fetch(endpoint, { headers })
.then(async (response): Promise<SignedUrlResponse> => {
const data = response.ok ? await response.json() : null;
return { status: response.status, data };
return { status: response.status, data, retryAfterMs: parseRetryAfterMs(response) };
})
.finally(() => {
inFlightSignedUrlRequests.delete(key);
Expand All @@ -41,12 +49,8 @@ function beginSignedUrlRequest(key: string, endpoint: string, headers: Record<st
return request;
}

/** Drop any shared request for this endpoint so a retry genuinely refetches. */
function dropInFlightSignedUrlRequests(endpoint: string) {
for (const key of inFlightSignedUrlRequests.keys()) {
if (key.startsWith(`${endpoint}\u0000`)) inFlightSignedUrlRequests.delete(key);
}
}
// In-flight requests are removed automatically in .finally when they settle.
// Active in-flight requests are preserved during retries so sibling consumers share them.

/**
* Resolve a private image's signed URL through its `/signed-url` endpoint, with
Expand All @@ -58,9 +62,17 @@ function dropInFlightSignedUrlRequests(endpoint: string) {
*/
export function useSignedImageUrl(endpoint: string, enabled: boolean) {
const [url, setUrl] = useState(() => getCachedSignedUrl(endpoint)?.url ?? null);
const [failed, setFailed] = useState(false);
const [failure, setFailure] = useState<SignedImageFailure | null>(null);
Comment thread
BigSimmo marked this conversation as resolved.
const [attempt, setAttempt] = useState(0);
const { authorizationHeader, session, markSessionExpired } = useAuthSession();

const [seenEndpoint, setSeenEndpoint] = useState(endpoint);
if (endpoint !== seenEndpoint) {
setSeenEndpoint(endpoint);
setUrl(getCachedSignedUrl(endpoint)?.url ?? null);
setFailure(null);
}

// Drop painted URLs during render when the auth *identity* changes (sign-out /
// expiry / account switch). Auth also clears the module LRU; without this,
// mounted consumers keep showing the prior user's URL until refetch settles.
Expand All @@ -75,7 +87,7 @@ export function useSignedImageUrl(endpoint: string, enabled: boolean) {
if (authIdentity !== seenAuthIdentity) {
setSeenAuthIdentity(authIdentity);
setUrl(null);
setFailed(false);
setFailure(null);
}

useEffect(() => {
Expand All @@ -87,7 +99,7 @@ export function useSignedImageUrl(endpoint: string, enabled: boolean) {
window.requestAnimationFrame(() => {
if (!active) return;
setUrl(cached.url);
setFailed(false);
setFailure(null);
});
return () => {
active = false;
Expand All @@ -102,7 +114,7 @@ export function useSignedImageUrl(endpoint: string, enabled: boolean) {
const key = signedUrlRequestKey(endpoint, authorizationHeader);
const request = inFlightSignedUrlRequests.get(key) ?? beginSignedUrlRequest(key, endpoint, authorizationHeader);
request
.then(({ status, data }) => {
.then(({ status, data, retryAfterMs }) => {
if (!active) return;
// A request restarted after sign-out has no identity and is expected to
// receive 401. Likewise, a request from an old identity can settle
Expand All @@ -115,13 +127,20 @@ export function useSignedImageUrl(endpoint: string, enabled: boolean) {
// account switch can hand the next user the prior bearer URL.
setCachedSignedUrl(endpoint, { ...data, url: data.url });
setUrl(data.url);
setFailed(false);
setFailure(null);
} else {
setFailed(true);
setFailure({
source: "response",
status,
retryable: isRetryableApiStatus(status),
retryAfterMs,
});
}
})
.catch(() => {
if (active) setFailed(true);
if (active) {
setFailure({ source: "network", status: null, retryable: true, retryAfterMs: null });
}
});
return () => {
active = false;
Expand All @@ -131,19 +150,17 @@ export function useSignedImageUrl(endpoint: string, enabled: boolean) {
// Drop the cached URL and refetch (e.g. after a 403 on an expired URL).
const retry = useCallback(() => {
clearCachedSignedUrl(endpoint);
dropInFlightSignedUrlRequests(endpoint);
setUrl(null);
setFailed(false);
setFailure(null);
setAttempt((current) => current + 1);
}, [endpoint]);

// Mark the current URL dead (e.g. <img> onError) so the frame shows its failure state.
const markFailed = useCallback(() => {
clearCachedSignedUrl(endpoint);
dropInFlightSignedUrlRequests(endpoint);
setUrl(null);
setFailed(true);
setFailure({ source: "image", status: null, retryable: true, retryAfterMs: null });
}, [endpoint]);

return { url, failed, retry, markFailed };
return { url, failed: failure !== null, failure, retry, markFailed };
}
10 changes: 5 additions & 5 deletions src/lib/api-client-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,16 @@ function parseJsonPayload(raw: string): ApiErrorPayload | null {
return payload;
}

function retryAfterMs(response: Response, now: number) {
const raw = response.headers.get("retry-after")?.trim();
export function parseRetryAfterMs(response: { headers?: { get(name: string): string | null } }, now = Date.now()) {
const raw = response.headers?.get("retry-after")?.trim();
if (!raw) return null;
const seconds = Number(raw);
if (Number.isFinite(seconds) && seconds >= 0) return Math.ceil(seconds * 1000);
const date = Date.parse(raw);
return Number.isFinite(date) ? Math.max(0, date - now) : null;
}

function retryableStatus(status: number) {
export function isRetryableApiStatus(status: number) {
return status === 408 || status === 429 || status === 500 || status === 502 || status === 503 || status === 504;
}

Expand Down Expand Up @@ -108,14 +108,14 @@ export async function parseApiErrorResponse(response: Response, now = Date.now()
(typeof payload?.code === "string" && payload.code) ||
(typeof details?.code === "string" && details.code) ||
`http_${response.status}`;
const headerDelay = retryAfterMs(response, now);
const headerDelay = parseRetryAfterMs(response, now);
const detailsDelay =
typeof details?.retryAfterSeconds === "number" ? Math.max(0, details.retryAfterSeconds * 1000) : null;
return new ApiClientError(
message,
response.status,
code,
retryableStatus(response.status),
isRetryableApiStatus(response.status),
headerDelay ?? detailsDelay,
);
}
Loading
Loading