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

[alejandro-jimenez-dcl]

Problem

One transient thumbnail failure poisons that item's thumbnail for the whole session:
ECSThumbnailProvider.GetAsync rethrows ThumbnailLoadFailedException from cache on every
subsequent request. The emote wheel's spinner sticks forever (unobserved UniTaskVoid
chain), and backpack grids replay-log the exception per page render. Sentry family
PAD/P9R/PD5 ≈ 260 events/week of pure cached replays (~396 events / ~33 users with the
one-shot timeout signals PA1/P9Y included). Introduced by #8824.

Root cause

An initialized non-Succeeded ThumbnailAssetResult slot was treated as a permanent terminal
state: the only producer of the sticky Failed state is the provider's own 30 s
consumer-timeout catch, and there was no recovery path until restart. Separately,
EmotesWheelController.WaitForThumbnailAsync had no catch, so the exception vanished into a
.Forget() chain with the loading spinner still active.

Fix

1. ECSThumbnailProvider: any initialized non-Succeeded slot is now clear-and-retry -
   reset the slot and spawn a fresh promise. The timeout path still writes Failed()
   (it remains the release signal for concurrent waiters); it just stops being permanent.
2. EmotesWheelController.WaitForThumbnailAsync: backpack-style catch - OCE returns; other
   exceptions log via ReportHub (THUMBNAILS), fall back to the default thumbnail, and
   release the spinner (mirrors BackpackEmoteGridController).
3. With the retry in place WithFallback.Cancelled lost its only in-lane reader, but the
   cancelled-vs-failed distinction is retained: the API pre-exists on dev and an in-flight
   sibling PR's disposal tests consume CancelledResult(); the resolver keeps stamping
   Cancelled for in-flight cancellation and Failed for terminal failures (see the
   Cross-PR note below).
4. Comment/doc updates stating the new per-attempt invariant (no behavior change).

Contract change: GetAsync moves from "throws instantly forever after first failure" to
"retries per explicit call". All 11 call sites are bounded per explicit call and concurrent
waiters are deduped per attachment while an attempt is in flight (the signal is
per-attachment, not per-attempt - a same-frame stale-entity overlap window exists and is
tracked as a follow-up); worst case a persistently-failing item costs one 30 s promise per
grid/wheel open.

Test

New EditMode ECSThumbnailProviderShould: RetryAfterFailedSlotInsteadOfRethrowing (pin:
instant rethrow, 0 promises spawned), MarkFailedOnTimeoutAndRetryOnNextCall (pin: cached
rethrow without spawning), ReturnCachedSuccessWithoutSpawningPromise (guards against an
always-retry regression).

Validation

Windows Unity 6000.4 EditMode lane at the pin: RED FAIL 2/3 as intended (retry test expected
1 promise got 0; timeout test expected 2 got 1; success guard passed) / GREEN PASS 3/3.

Fixes #8902
Fixes #8891

Cross-PR note

WithFallback.Cancelled / CancelledResult() are retained - an earlier revision of
this branch deleted them (Fix item 3 above describes that revision and is superseded;
2b1374f8d3 reverted the deletion). The pair pre-exists this PR (present at the base
commit, StreamableLoadingResult.cs:58), and while this branch no longer reads the
Cancelled discriminator - GetAsync now treats every initialized non-Succeeded slot,
cancelled or failed, as clear-and-retry - it is not dead API: the in-flight sibling PR
bugsweep/unload-thumbnail-nre-blocks-memory-release consumes
WithFallback.CancelledResult() in its thumbnail-disposal test
(StorageThumbnailDisposalShould.cs), and the resolver's cancellation stamps remain the
waiter-release signal via IsInitialized. Removing a pre-existing shared-struct member
out from under a sibling in-flight PR is out of scope for this bugfix; if the
discriminator is still unread once both PRs land, collapsing Cancelled into Failed
can ship as its own cleanup.

Includes inspection-warning cleanup in all touched files.

Fixes #8891
Fixes #8902

[github-actions]

🚦 CI Status

Windows and Mac build successful in Unity Cloud! You can find a link to the downloadable artifact below.

Name	Link
Commit	90c3c76
Logs	https://github.com/decentraland/unity-explorer/actions/runs/32252105710
Download Windows	https://github.com/decentraland/unity-explorer/suites/87428243076/artifacts/
Download Windows S3	https://explorer-artifacts.decentraland.org/@dcl/unity-explorer/branch/bugsweep/thumbnail-sticky-failure-rethrow/pr-25304-90c3c76/Decentraland_windows64.zip
Download Mac	https://github.com/decentraland/unity-explorer/suites/87428243076/artifacts/
Download Mac S3	https://explorer-artifacts.decentraland.org/@dcl/unity-explorer/branch/bugsweep/thumbnail-sticky-failure-rethrow/pr-25304-90c3c76/Decentraland_macos.zip
Built on	2026-08-19T13:12:48Z

Warnings not reduced: 12724 => 13128 — remove at least 405 warnings to merge.

Warnings/errors in files changed by this PR (1)

Assets/DCL/Infrastructure/ECS/Unity/StreamableLoading/Common/Components/StreamableLoadingResult.cs:10  DefaultStructEqualityIsUsed.Global  Struct 'StreamableLoadingResult' is checked for equality using an inefficient runtime-provided implementation

All Unity tests passed ✅

TESTS SUITE	Result	Passed	Failed	Skipped
EditMode	✅ Passed	25057	0	13
PlayMode	✅ Passed	236	0	37

[decentraland-bot]

PR Review — #9790 fix: retry avatar thumbnail loads after a failed attempt instead of rethrowing forever

STEP 2 — Root-cause check: PASS ✅

Problem: ECSThumbnailProvider.GetAsync treated a Failed thumbnail slot as a permanent terminal state — every subsequent call rethrew ThumbnailLoadFailedException from cache, poisoning that item's thumbnail for the entire session. The emote wheel's spinner stuck forever (unobserved UniTaskVoid chain) and backpack grids replayed the exception per page render.

Does the diff fix the cause? Yes. The root cause was the else throw new ThumbnailLoadFailedException() branch in GetAsync that turned a transient timeout failure into a permanent sticky state. The fix removes the conditional (if Cancelled→clear, else→throw) and replaces it with unconditional clear-and-retry for any initialized non-succeeded slot. This directly eliminates the sticky failure mechanism.

STEP 3 — Design & integration: PASS ✅

No new long-lived units introduced. The changes modify existing behavior within existing classes — no new systems, managers, or controllers.

Lifecycle ownership: The retry logic lives in ECSThumbnailProvider, which is the consumer-facing provider and the correct home for the retry-vs-cache decision. The resolver system (ResolveAvatarAttachmentThumbnailSystem) continues to own promise resolution. The error handling in EmotesWheelController mirrors the established pattern in BackpackEmoteGridController (lines 380–401).

Teardown trace:

• timeoutCts — properly disposed via using declaration (line 49).
• promiseCts — created at line 40, cancelled on timeout (line 63), but never disposed. This is pre-existing (not introduced by this PR) and is a known design trade-off: the CTS outlives GetAsync because it is bound to the ECS promise entity that the resolver system checks via IsCancellationRequested. Not flagged as a PR issue.
• EmotesWheelController.slotSetUpCts — properly cancelled and disposed in OnViewClose (line 124).

Contract change is bounded: GetAsync moves from "throws instantly forever after first failure" to "retries per explicit call". All call sites are bounded per explicit invocation. Concurrent waiters are deduped per attachment while an attempt is in-flight. Worst case for a persistently-failing item: one 30 s promise per grid/wheel open — acceptable.

Resolver guard logic is correct: The guard if (wearable.ThumbnailAssetResult is not { IsInitialized: true }) in both resolver query methods prevents the resolver from overwriting a Failed state (set by the consumer timeout) with Cancelled, ensuring the timeout's terminal state takes priority.

STEP 4 — Member audit: PASS ✅

No new public properties or accessors introduced. The Cancelled field and CancelledResult() factory on WithFallback are retained (pre-existing), not new — justified by the sibling PR bugsweep/unload-thumbnail-nre-blocks-memory-release which consumes CancelledResult() in its disposal tests.

STEP 5 — Line-level review: PASS ✅

I performed two passes over all changed lines checking for:

• Bugs, logic errors, race conditions
• Missing error handling
• Resource/subscription leaks
• Naming conventions (PascalCase types/methods, camelCase locals/params)
• CLAUDE.md anti-patterns
• Comment quality
• Async patterns (§9)
• Nullable reference types
• Test patterns (AAA, NUnit + NSubstitute)

No issues found. Specific verification:

1. ECSThumbnailProvider.cs — The unconditional slot clear (line 37) is correct: both Failed and Cancelled states are per-attempt, and the next explicit GetAsync should retry. The timeout catch (lines 58–67) correctly cancels promiseCts, stamps Failed() to release concurrent waiters, and throws ThumbnailLoadFailedException.

2. EmotesWheelController.cs — The new try/catch in WaitForThumbnailAsync (lines 185–201) correctly mirrors BackpackEmoteGridController (lines 380–401):

   • OperationCanceledException → return (view is closing)
   • Other exceptions → ReportHub.LogException with ReportCategory.THUMBNAILS, cancellation check, fallback to LoadThumbnailsUtils.DEFAULT_THUMBNAIL.Sprite
   • Spinner reset (lines 203–204) executes on both success and fallback paths; only skipped on cancellation (correct — view is being torn down)

3. ResolveAvatarAttachmentThumbnailSystem.cs — Comment-only changes. Guard logic unchanged. Removed unused using DCL.AvatarRendering.Wearables import — correct cleanup.

4. StreamableLoadingResult.cs — Doc-only changes. Removed unused using AssetManagement and using System.Runtime.CompilerServices imports — correct cleanup.

5. ECSThumbnailProviderShould.cs — Well-structured tests:

   • RetryAfterFailedSlotInsteadOfRethrowing: Pins the retry behavior — sets Failed slot, verifies GetAsync spawns a new promise (count == 1) and returns the resolved sprite. AAA pattern. ✓
   • ReturnCachedSuccessWithoutSpawningPromise: Guards against always-retry regression — pre-sets success, verifies cached return with zero promises. ✓
   • MarkFailedOnTimeoutAndRetryOnNextCall: Pins timeout→retry — first call times out (1ms), verifies Failed state, second call spawns a fresh promise (count +1). ✓
   • Test setup uses NSubstitute for IDecentralandUrlsSource. FakeWearable (pre-existing test helper) implements IThumbnailAttachment. ✓

Known limitation acknowledged by the author: A same-frame stale-entity overlap window exists when concurrent GetAsync calls target the same attachment. This is tracked as a follow-up and is not a regression — the old code had the same concurrency model.

STEP 6 — Complexity: COMPLEX

Touches async flow (UniTask cancellation, CancellationTokenSource lifecycle), Result types (StreamableLoadingResult.WithFallback), and the ECS thumbnail resolution pipeline.

STEP 7 — QA: YES

Changes affect runtime code in the emote wheel and thumbnail loading pipeline — user-visible behavior (thumbnail loading, spinner state, retry on failure).

STEP 8 — Non-blocking warnings

None. Main.unity not modified.

Security review: No security issues found

No secrets, credentials, user input handling, auth changes, or sensitive data exposure. Pure client-side bugfix.

---

REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Touches async/UniTask cancellation flows, StreamableLoadingResult types, and the ECS thumbnail resolution pipeline
QA_REQUIRED: YES

---

Reviewed by Jarvis 🤖 · Requested by decentraland-bot via GitHub

[alejandro-jimenez-dcl]

approved

[github-actions]

🔍 Jarvis reviewed this PR and found no blocking issues, but assessed it as complex — human DEV review is still required before merging.

[decentraland-bot]

PR #9790, run #32259898332

Builds: Windows change, Windows baseline, macOS change, macOS baseline

How to read this table

• Each build is measured 3 times. The values are the median, and (min–max) is the lowest and highest of those runs — a wide range means the metric is noisy and small differences are not trustworthy.
• Δ is Change minus Baseline (a negative Δ means Change is faster).
• 🟢 faster / 🔴 slower — a real difference: larger than both 3% and the run-to-run range.
• ⚪ within noise — the difference is smaller than how much the build varies between its own runs, so it cannot be told apart from random variation. Treat it as no change.
• Exceptions per run — the average number of exceptions in a run's log; more than the baseline is flagged 🔴 even when frame times look fine. The Exception breakdown under each table groups them by the explorer's report category and exception type (as totals across the runs).
• A run that logged unusually many exceptions (at least 10 and 5× the median of its build's runs — e.g. a service was down during it) is excluded from all numbers and called out under the table.

Intel Core i5

Metric	Baseline	Change	Δ	Result
Samples	2313 (×3)	2202 (×3)
CPU average	38.6 ms (33.4–38.8)	40.7 ms (39.6–41.6)	2.1 ms	⚪ within noise
CPU 1% worst	322.6 ms (57.0–343.5)	394.5 ms (350.4–474.9)	71.9 ms	⚪ within noise
CPU 0.1% worst	344.1 ms (341.1–360.3)	421.1 ms (361.5–496.0)	76.9 ms	⚪ within noise
GPU average	9.5 ms (9.2–9.6)	9.6 ms (9.4–9.7)	0.0 ms	⚪ within noise
GPU 1% worst	35.6 ms (23.5–37.7)	42.8 ms (36.9–50.7)	7.3 ms	⚪ within noise
GPU 0.1% worst	44.4 ms (39.8–45.0)	50.6 ms (42.3–60.5)	6.2 ms	⚪ within noise
Exceptions per run	66	66	0	⚪ none new

Exception breakdown

Exception	Baseline (3 runs)	Change (3 runs)
[UI] DllNotFoundException	192	192
[ENGINE] NullReferenceException	3	3
[ENGINE] ObjectDisposedException	3	3

Apple M1

Metric	Baseline	Change	Δ	Result
Samples	4105 (×3)	4033 (×3)
CPU average	21.8 ms (21.8–22.9)	22.2 ms (22.2–22.6)	0.4 ms	⚪ within noise
CPU 1% worst	215.9 ms (215.7–217.7)	233.6 ms (232.3–234.6)	17.8 ms	🔴 8% slower
CPU 0.1% worst	226.3 ms (222.9–228.8)	238.5 ms (236.8–239.6)	12.3 ms	🔴 5% slower
GPU average	2.5 ms (2.0–3.2)	2.9 ms (2.8–6.0)	0.4 ms	⚪ within noise
GPU 1% worst	34.3 ms (34.2–36.2)	34.4 ms (34.2–34.9)	0.1 ms	⚪ within noise
GPU 0.1% worst	36.3 ms (34.8–37.5)	35.7 ms (35.6–35.8)	-0.6 ms	⚪ within noise
Exceptions per run	0	0	0	⚪ none new