fix: retry avatar thumbnail loads after a failed attempt instead of rethrowing forever

Explorer/Assets/DCL/AvatarRendering/AvatarShape/Tests/EditMode/ECSThumbnailProviderShould.cs

@@ -0,0 +1,124 @@
+using Arch.Core;
+using Cysharp.Threading.Tasks;
+using DCL.AvatarRendering.Loading.Components;
+using DCL.AvatarRendering.Loading.DTO;
+using DCL.AvatarRendering.Loading.Exceptions;
+using DCL.AvatarRendering.Wearables;
+using DCL.AvatarRendering.Wearables.Helpers;
+using DCL.Multiplayer.Connections.DecentralandUrls;
+using ECS.StreamableLoading.Common.Components;
+using ECS.StreamableLoading.Textures;
+using NSubstitute;
+using NUnit.Framework;
+using System.Threading;
+using System.Threading.Tasks;
+using UnityEngine;
+using Promise = ECS.StreamableLoading.Common.AssetPromise<ECS.StreamableLoading.Textures.TextureData, ECS.StreamableLoading.Textures.GetTextureIntention>;
+
+namespace DCL.AvatarRendering.AvatarShape.Tests
+{
+    [TestFixture]
+    public class ECSThumbnailProviderShould
+    {
+        private static readonly QueryDescription THUMBNAIL_PROMISES = new QueryDescription().WithAll<IThumbnailAttachment, Promise>();
+
+        private World world = null!;
+        private ECSThumbnailProvider provider = null!;
+
+        [SetUp]
+        public void SetUp()
+        {
+            world = World.Create();
+
+            IDecentralandUrlsSource urlsSource = Substitute.For<IDecentralandUrlsSource>();
+            urlsSource.Url(Arg.Any<DecentralandUrl>()).Returns("https://peer.decentraland.test/content");
+
+            provider = new ECSThumbnailProvider(urlsSource, world);
+        }
+
+        [TearDown]
+        public void TearDown()
+        {
+            world.Dispose();
+        }
+
+        [Test]
+        public async Task RetryAfterFailedSlotInsteadOfRethrowing()
+        {
+            FakeWearable wearable = NewWearable();
+            wearable.ThumbnailAssetResult = StreamableLoadingResult<SpriteData>.WithFallback.Failed();
+
+            UniTask<Sprite> getTask = provider.GetAsync(wearable, CancellationToken.None);
+            int promisesSpawned = world.CountEntities(in THUMBNAIL_PROMISES);
+
+            // Resolve the attachment the way ResolveAvatarAttachmentThumbnailSystem does on success.
+            SpriteData spriteData = NewSpriteData();
+            wearable.ThumbnailAssetResult = new StreamableLoadingResult<SpriteData>.WithFallback(spriteData);
+
+            Sprite? sprite;
+
+            try { sprite = await getTask; }
+            catch (ThumbnailLoadFailedException) { sprite = null; }
+
+            Assert.That(promisesSpawned, Is.EqualTo(1), "GetAsync must clear a Failed slot and spawn a fresh promise instead of rethrowing the cached failure");
+            Assert.That(sprite, Is.SameAs(spriteData.Sprite));
+        }
+
+        [Test]
+        public async Task ReturnCachedSuccessWithoutSpawningPromise()
+        {
+            FakeWearable wearable = NewWearable();
+            SpriteData spriteData = NewSpriteData();
+            wearable.ThumbnailAssetResult = new StreamableLoadingResult<SpriteData>.WithFallback(spriteData);
+
+            Sprite sprite = await provider.GetAsync(wearable, CancellationToken.None);
+
+            Assert.That(sprite, Is.SameAs(spriteData.Sprite));
+            Assert.That(world.CountEntities(in THUMBNAIL_PROMISES), Is.Zero);
+        }
+
+        [Test]
+        public async Task MarkFailedOnTimeoutAndRetryOnNextCall()
+        {
+            FakeWearable wearable = NewWearable();
+
+            try
+            {
+                await provider.GetAsync(wearable, CancellationToken.None, timeoutMs: 1);
+                Assert.Fail("Expected the first never-resolving load to throw after the timeout");
+            }
+            catch (ThumbnailLoadFailedException) { }
+
+            Assert.That(wearable.ThumbnailAssetResult, Is.Not.Null);
+            Assert.That(wearable.ThumbnailAssetResult!.Value.IsInitialized, Is.True);
+            Assert.That(wearable.ThumbnailAssetResult!.Value.Succeeded, Is.False);
+
+            int promisesAfterTimeout = world.CountEntities(in THUMBNAIL_PROMISES);
+
+            UniTask<Sprite> retryTask = provider.GetAsync(wearable, CancellationToken.None, timeoutMs: 1);
+            int promisesAfterRetry = world.CountEntities(in THUMBNAIL_PROMISES);
+
+            try { await retryTask; }
+            catch (ThumbnailLoadFailedException) { }
+
+            Assert.That(promisesAfterRetry, Is.EqualTo(promisesAfterTimeout + 1), "A call after a timed-out attempt must spawn a fresh promise instead of rethrowing the cached failure");
+        }
+
+        private static FakeWearable NewWearable() =>
+            new (new WearableDTO
+            {
+                metadata = new WearableDTO.WearableMetadataDto
+                {
+                    id = "urn:decentraland:off-chain:base-avatars:red_hoodie",
+                    thumbnail = "bafybeie7lzqakerm4n4x7557g3va4sv7aeoniexlomdgjskuoubo6s3mku",
+                    data =
+                    {
+                        representations = new AvatarAttachmentDTO.Representation[] { new () },
+                    },
+                },
+            });
+
+        private static SpriteData NewSpriteData() =>
+            new (new TextureData(Texture2D.whiteTexture), Sprite.Create(Texture2D.whiteTexture!, new Rect(0, 0, 1, 1), new Vector2()));
+    }
+}

Explorer/Assets/DCL/AvatarRendering/Thumbnails/ECSThumbnailProvider.cs

@@ -31,12 +31,10 @@ public async UniTask<Sprite> GetAsync(IThumbnailAttachment avatarAttachment, Can
                 if (existing.Succeeded)
                     return existing.Asset;
 
-                // A previous attempt was cancelled (e.g. page change). Clear so a fresh promise
-                // can spawn below. Sticky failures keep the slot and fall through to throw.
-                if (existing.Cancelled)
-                    avatarAttachment.ThumbnailAssetResult = null;
-                else
-                    throw new ThumbnailLoadFailedException();
+                // A previous attempt ended without a sprite (timeout or cancellation). Terminal
+                // non-success states are per-attempt, never sticky across explicit requests:
+                // clear the slot so a fresh promise can spawn below.
+                avatarAttachment.ThumbnailAssetResult = null;
             }
 
             CancellationTokenSource promiseCts = CancellationTokenSource.CreateLinkedTokenSource(ct);
@@ -59,10 +57,9 @@ public async UniTask<Sprite> GetAsync(IThumbnailAttachment avatarAttachment, Can
             }
             catch (OperationCanceledException) when (!ct.IsCancellationRequested)
             {
-                // Timed out: cancel the underlying promise so the resolver cleans it up, and record
-                // a sticky Failed on the attachment so subsequent calls don't immediately re-attempt
-                // a load that we already gave up on. (The resolver will see IsCancellationRequested
-                // but skip overwriting because the slot is now Failed, not Cancelled.)
+                // Timed out: cancel the underlying promise so the resolver cleans it up. Failed
+                // releases concurrent waiters on this attachment and keeps the resolver from
+                // stamping Cancelled over the slot; the next explicit GetAsync clears it and retries.
                 promiseCts.Cancel();
                 avatarAttachment.ThumbnailAssetResult = StreamableLoadingResult<SpriteData>.WithFallback.Failed();
 

Explorer/Assets/DCL/AvatarRendering/Thumbnails/Systems/ResolveAvatarAttachmentThumbnailSystem.cs

@@ -4,7 +4,6 @@
 using Arch.SystemGroups.DefaultSystemGroups;
 using DCL.AvatarRendering.Loading.Components;
 using DCL.AvatarRendering.Thumbnails.Utils;
-using DCL.AvatarRendering.Wearables;
 using DCL.Diagnostics;
 using ECS.Abstract;
 using ECS.StreamableLoading.Common.Components;
@@ -34,9 +33,8 @@ private void CompleteWearableABThumbnailDownload(Entity entity, ref IThumbnailAt
         {
             if (promise.IsCancellationRequested(World))
             {
-                // Mark as Cancelled so the next GetAsync call clears the slot and retries.
-                // Don't overwrite a sticky Failed (e.g. from a consumer timeout); only reset to
-                // Cancelled when the slot is clear.
+                // Release waiters with Cancelled only while the slot still signals in-flight; an
+                // initialized slot (e.g. Failed from a consumer timeout) already carries this attempt's terminal state.
                 if (wearable.ThumbnailAssetResult is not { IsInitialized: true })
                     wearable.ThumbnailAssetResult = StreamableLoadingResult<SpriteData>.WithFallback.CancelledResult();
                 World.Destroy(entity);
@@ -55,8 +53,8 @@ private void CompleteWearableThumbnailDownload(Entity entity, ref IThumbnailAtta
         {
             if (promise.IsCancellationRequested(World))
             {
-                // Mark as Cancelled so the next GetAsync call clears the slot and retries.
-                // Don't overwrite a sticky Failed (e.g. from a consumer timeout).
+                // Release waiters with Cancelled only while the slot still signals in-flight; an
+                // initialized slot already carries this attempt's terminal state.
                 if (wearable.ThumbnailAssetResult is not { IsInitialized: true })
                     wearable.ThumbnailAssetResult = StreamableLoadingResult<SpriteData>.WithFallback.CancelledResult();
                 World.Destroy(entity);

Explorer/Assets/DCL/EmotesWheel/EmotesWheelController.cs

@@ -2,7 +2,7 @@
 using CommunicationData.URLHelpers;
 using Cysharp.Threading.Tasks;
 using DCL.AvatarRendering.Emotes;
-using DCL.AvatarRendering.Loading.Components;
+using DCL.AvatarRendering.Thumbnails.Utils;
 using DCL.AvatarRendering.Wearables;
 using DCL.Backpack;
 using DCL.Diagnostics;
@@ -13,6 +13,7 @@
 using DCL.Profiles.Self;
 using DCL.UI;
 using MVC;
+using System;
 using System.Threading;
 using UnityEngine;
 using UnityEngine.InputSystem;
@@ -181,9 +182,24 @@ private async UniTask WaitForThumbnailAsync(IEmote emote, EmoteWheelSlotView vie
             view.Thumbnail.gameObject.SetActive(false);
             view.LoadingSpinner.SetActive(true);
 
-            Sprite sprite = await thumbnailProvider.GetAsync(emote, ct);
+            try
+            {
+                Sprite sprite = await thumbnailProvider.GetAsync(emote, ct);
+
+                view.Thumbnail.sprite = sprite;
+            }
+            catch (OperationCanceledException) { return; }
+            catch (Exception e)
+            {
+                ReportHub.LogException(e, new ReportData(ReportCategory.THUMBNAILS));
+
+                if (ct.IsCancellationRequested) return;
+
+                // The failure path must still surface a sprite and release the spinner, otherwise
+                // the slot stays stuck on a load that already gave up.
+                view.Thumbnail.sprite = LoadThumbnailsUtils.DEFAULT_THUMBNAIL.Sprite;
+            }
 
-            view.Thumbnail.sprite = sprite;
             view.Thumbnail.gameObject.SetActive(true);
             view.LoadingSpinner.SetActive(false);
         }

Explorer/Assets/DCL/Infrastructure/ECS/Unity/StreamableLoading/Common/Components/StreamableLoadingResult.cs

@@ -1,8 +1,6 @@
-using AssetManagement;
-using DCL.Diagnostics;
+using DCL.Diagnostics;
 using System;
 using UnityEngine;
-using System.Runtime.CompilerServices;
 
 namespace ECS.StreamableLoading.Common.Components
 {
@@ -25,14 +23,14 @@ public readonly struct WithFallback
 
             /// <summary>
             ///     True when the request reached a terminal successful (or fallback-acceptable) state.
-            ///     False when the request failed or was cancelled; check <see cref="Cancelled"/>
-            ///     to distinguish a transient cancellation from a sticky failure.
+            ///     False when the request failed or was cancelled; the value describes that single
+            ///     attempt only.
             /// </summary>
             public readonly bool Succeeded;
 
             /// <summary>
-            ///     True when the request was cancelled mid-flight (transient state — consumers can
-            ///     clear the slot and retry). False for both genuine successes and sticky failures.
+            ///     True when the request was cancelled mid-flight. False for both genuine
+            ///     successes and failures (e.g. a consumer timeout).
             /// </summary>
             public readonly bool Cancelled;