fix(web): stop a single image error from permanently blanking an avatar

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<Nullable<string>>(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<HTMLImageElement>) => {
+      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 (
       <HarmonyAvatar
-        key={hasError ? 'error' : 'no-error'}
+        key={finalImageSrc}
         src={finalImageSrc}
         onError={handleError}
         {...other}
@@ -80,7 +94,7 @@ export const AvatarContent = (props: AvatarProps) => {
   if (onClick) {
     return (
       <HarmonyAvatar
-        key={hasError ? 'error' : 'no-error'}
+        key={finalImageSrc}
         role='button'
         tabIndex={0}
         aria-label={label}
@@ -103,7 +117,7 @@ export const AvatarContent = (props: AvatarProps) => {
         noOverflow={popover}
       >
         <HarmonyAvatar
-          key={hasError ? 'error' : 'no-error'}
+          key={finalImageSrc}
           data-testid='avatar-test'
           src={finalImageSrc}
           onError={handleError}
@@ -115,7 +129,7 @@ export const AvatarContent = (props: AvatarProps) => {
 
   return (
     <HarmonyAvatar
-      key={hasError ? 'error' : 'no-error'}
+      key={finalImageSrc}
       src={finalImageSrc}
       onError={handleError}
       {...other}

packages/web/src/components/user-card/UserCard.test.tsx

@@ -1,11 +1,20 @@
+import { SquareSizes } from '@audius/common/models'
 import { PROFILE_PAGE } from '@audius/common/src/utils/route'
 import { Text } from '@audius/harmony'
 import { MemoryRouter, Route, Routes } from 'react-router'
 import { describe, expect, beforeAll, afterEach, afterAll } from 'vitest'
 
 import { artistUser } from 'test/mocks/fixtures/users'
 import { mockUsers } from 'test/msw/mswMocks'
-import { RenderOptions, mswServer, render, screen, it } from 'test/test-utils'
+import {
+  RenderOptions,
+  mswServer,
+  render,
+  screen,
+  it,
+  fireEvent,
+  waitFor
+} from 'test/test-utils'
 
 import { UserCard } from './UserCard'
 
@@ -69,6 +78,31 @@ describe('UserCard', () => {
     )
   })
 
+  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 <img>
+    // 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()

packages/web/src/hooks/useProfilePicture.ts

@@ -6,23 +6,32 @@ 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 `<img>` 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')
   })
   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