From e283fa32d72d26efe4102c1a68a2944971a03203 Mon Sep 17 00:00:00 2001 From: Dylan Audius Date: Fri, 4 Sep 2026 12:53:19 -0700 Subject: [PATCH] fix(web): stop a single image error from permanently blanking an avatar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avatar tracked image failures in a boolean `hasError` that only reset when `userId` changed, so the first error swapped in the empty-avatar placeholder for the life of the mount. useImageSize does resolve a working mirror after a failure, but Avatar discarded it — the placeholder had already won — which is why verified artists with perfectly good profile pictures render a gray silhouette in search. The trigger is the API handing out a content node that 502s for the blob (~5% of profile-picture primaries at the moment; the shuffle in rendezvous.Select re-rolls the primary per request, so the same user shows up broken for some sessions and fine for others). A transient failure under a burst of cards does it too. Track the failing src instead of a boolean, so a later mirror url renders normally, and forward the error to useImageSize.onError so it advances to the next mirror rather than leaving the image stranded on a dead host. Once every mirror has failed the placeholder sticks, and since it is a data uri it cannot error and re-enter the retry path. Co-Authored-By: Claude Opus 5 --- packages/web/src/components/avatar/Avatar.tsx | 48 ++++++++++++------- .../components/user-card/UserCard.test.tsx | 36 +++++++++++++- packages/web/src/hooks/useProfilePicture.ts | 32 +++++++++---- 3 files changed, 88 insertions(+), 28 deletions(-) diff --git a/packages/web/src/components/avatar/Avatar.tsx b/packages/web/src/components/avatar/Avatar.tsx index 0fa1250874c..634a1ecfbc4 100644 --- a/packages/web/src/components/avatar/Avatar.tsx +++ b/packages/web/src/components/avatar/Avatar.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect } from 'react' +import { useState, useEffect, useCallback, SyntheticEvent } from 'react' import { useCurrentUserId, useUser } from '@audius/common/api' import { imageProfilePicEmptyNew } from '@audius/common/assets' @@ -11,7 +11,7 @@ import { import { componentWithErrorBoundary } from 'components/error-wrapper/componentWithErrorBoundary' import { UserLink } from 'components/link' -import { useProfilePicture } from 'hooks/useProfilePicture' +import { useProfilePictureSource } from 'hooks/useProfilePicture' const messages = { goTo: 'Go to', @@ -39,24 +39,38 @@ export const AvatarContent = (props: AvatarProps) => { ...other } = props - const [hasError, setHasError] = useState(false) + // Tracks the src that most recently failed to render. This must be the + // failing url rather than a boolean: a boolean latches, so once one host + // 502s the placeholder would win forever, even after `useImageSize` + // resolves a working mirror into `imageUrl`. + const [failedSrc, setFailedSrc] = useState>(null) useEffect(() => { - setHasError(false) + setFailedSrc(null) }, [userId]) - const handleError = () => { - setHasError(true) - } - - const profileImage = useProfilePicture({ - userId: userId ?? undefined, - size: imageSize - }) + const { imageUrl: profileImage, onError: onImageError } = + useProfilePictureSource({ + userId: userId ?? undefined, + size: imageSize + }) + + const handleError = useCallback( + (event: SyntheticEvent) => { + const src = event.currentTarget.src + setFailedSrc(src) + // Let useImageSize advance to the next mirror. Once every mirror has + // failed it stops handing back new urls, and the placeholder below + // sticks (a data uri, so it cannot error and re-enter this path). + onImageError?.(src) + }, + [onImageError] + ) const image = userId ? profileImage : imageProfilePicEmptyNew - const finalImageSrc = hasError ? imageProfilePicEmptyNew : image + const finalImageSrc = + image && image === failedSrc ? imageProfilePicEmptyNew : image const { data: currentUserId } = useCurrentUserId() const { data: userName } = useUser(userId, { @@ -69,7 +83,7 @@ export const AvatarContent = (props: AvatarProps) => { if (ariaHidden) { return ( { if (onClick) { return ( { noOverflow={popover} > { return ( { ) }) + it('retries a mirror when the primary host fails to render', async () => { + // A content node that answers /health_check but 502s on the blob still + // gets handed out as the primary, so the avatar has to survive an + // error by moving to a mirror rather than latching the empty placeholder. + const deadUrl = + 'https://dead-node.test/artist-user-image-profile-medium.jpg' + renderUserCard({ + ...artistUser, + profile_picture: { + ...artistUser.profile_picture, + [SquareSizes.SIZE_480_BY_480]: deadUrl + } + }) + + fireEvent.error(await screen.findByRole('img')) + + await waitFor(async () => { + const src = (await screen.findByRole('img')).getAttribute('src') + expect(src).not.toMatch(/^data:/) + expect(new URL(src!).hostname).toBe( + new URL(artistUser.profile_picture.mirrors[0]).hostname + ) + }) + }) + it('handles users with large follow counts correctly', async () => { renderUserCard({ ...artistUser, follower_count: 1000 }) expect(await screen.findByText('1K Followers')).toBeInTheDocument() diff --git a/packages/web/src/hooks/useProfilePicture.ts b/packages/web/src/hooks/useProfilePicture.ts index 326460dcb3a..3a00872c1fe 100644 --- a/packages/web/src/hooks/useProfilePicture.ts +++ b/packages/web/src/hooks/useProfilePicture.ts @@ -6,15 +6,24 @@ import { pick } from 'lodash' import { preload } from 'utils/image' -export const useProfilePicture = ({ - userId, - size, - defaultImage -}: { +type UseProfilePictureArgs = { userId?: ID size: SquareSizes defaultImage?: string -}) => { +} + +/** + * Like `useProfilePicture`, but also returns the `onError` callback from + * `useImageSize`. Callers that render the url in an `` should pass it + * through, so that a render-time failure (which `preload` can miss — the two + * requests are separate and a node can fail one and serve the other) advances + * to the next mirror instead of stranding the image on a dead host. + */ +export const useProfilePictureSource = ({ + userId, + size, + defaultImage +}: UseProfilePictureArgs) => { const { data: partialUser } = useUser(userId, { select: (user) => pick(user, 'profile_picture', 'updatedProfilePicture', 'is_deactivated') @@ -22,7 +31,7 @@ export const useProfilePicture = ({ const { profile_picture, updatedProfilePicture, is_deactivated } = partialUser ?? {} - const { imageUrl } = useImageSize({ + const { imageUrl, onError } = useImageSize({ // Deactivated/deleted accounts must not expose their profile picture // (privacy/GDPR) — force the default placeholder instead. artwork: is_deactivated ? undefined : profile_picture, @@ -32,10 +41,13 @@ export const useProfilePicture = ({ }) if (is_deactivated) { - return defaultImage ?? profilePicEmpty + return { imageUrl: defaultImage ?? profilePicEmpty, onError: undefined } } if (updatedProfilePicture) { - return updatedProfilePicture.url + return { imageUrl: updatedProfilePicture.url, onError: undefined } } - return imageUrl + return { imageUrl, onError } } + +export const useProfilePicture = (args: UseProfilePictureArgs) => + useProfilePictureSource(args).imageUrl