fix: Give media that is still loading a grace period before rebuilding (#2739)

- Related: #2718

src/card-controller/issues/issue-manager.ts

@@ -203,16 +203,6 @@ export class IssueManager {
       this._retryTimer.reset();
       return;
     }
-    if (!this._stateManager.canRetryNow()) {
-      // An issue still has a failing problem but cannot retry right now. Cancel
-      // the pending timer so it does not fire a retry the issue cannot take --
-      // but do not reset(), which is reserved for the resolved case above.
-      // Consequently no retry is scheduled here; scheduling resumes on a later
-      // evaluate() (any condition-state change re-runs this), once the issue
-      // reports it can retry again or has resolved.
-      this._retryTimer.cancel();
-      return;
-    }
     if (this._retryTimer.isRunning()) {
       return;
     }
@@ -238,15 +228,19 @@ export class IssueManager {
         : retryConfig,
     );
 
-    // Schedule without advancing: the backoff only escalates if the retry
-    // actually runs (via the explicit advance() below), not when it's gated.
+    // Scheduled whether or not a retry can run right now: a hold can end with
+    // nothing else happening on the card (e.g. media given time to finish
+    // loading), so the timer has to come back and look rather than waiting to
+    // be woken. Scheduled without advancing: the backoff only escalates if the
+    // retry actually runs (via the explicit advance() below), not when it's
+    // held.
     this._retryTimer.schedule(
       () => {
         if (!this._stateManager.needsRetry()) {
           this._retryTimer.reset();
           return;
         }
-        if (this._isScheduledRetryAllowed()) {
+        if (this._isScheduledRetryAllowed() && this._stateManager.canRetryNow()) {
           this._stateManager.retry();
 
           // This attempt counts: advance the backoff so the next schedule
@@ -256,8 +250,11 @@ export class IssueManager {
           this._retryTimer.advance();
           this.evaluate();
         } else {
-          // Retry was gated (e.g. user interaction). Not a failed attempt; the
-          // backoff stays put and we re-arm at the same delay.
+          // Retry was gated: the user is interacting, or every issue that needs
+          // one is holding off (e.g. media still loading). Nothing was
+          // attempted, so the backoff stays put and we re-arm at the same delay
+          // rather than escalating towards the ten minute cap for retries that
+          // never ran.
           this._scheduleRetryIfNeeded();
         }
       },

src/card-controller/issues/issues/media-unavailable.ts

@@ -1,3 +1,4 @@
+import { add } from 'date-fns';
 import type { IssueResolveContext, IssueTriggerContext } from 'issue';
 
 import type {
@@ -58,25 +59,43 @@ declare module 'issue' {
 interface TargetError {
   reason: MediaUnavailableIssueReason;
   description?: string;
+
+  // The earliest the card should rebuild this target's media. Carried forward
+  // across repeat reports of the same failure, and refreshed when a rebuild
+  // starts a new attempt.
+  rebuildNotBefore: Date;
 }
 
-// The per-cause presentation (localization key + icon), shared by the
-// notification metadata and the reconnecting placeholder so each cause is
+// The per-cause presentation (localization key + icon) and handling, shared by
+// the notification metadata and the reconnecting placeholder so each cause is
 // described in exactly one place. `resetOnLoad` means a media load will reset
-// this issue reason.
+// this issue reason. `rebuildGraceSeconds` is how long the media is left alone
+// before the card rebuilds it: media that has failed has nothing to protect and
+// is rebuilt at once, so only a load still in progress asks for any.
 export const MEDIA_UNAVAILABLE_REASONS: Record<
   MediaUnavailableIssueReason,
-  { localizationKey: string; icon: string; resetOnLoad: boolean }
+  {
+    localizationKey: string;
+    icon: string;
+    resetOnLoad: boolean;
+    rebuildGraceSeconds: number;
+  }
 > = {
   entity_unavailable: {
     localizationKey: 'issues.media_unavailable.reasons.entity_unavailable',
     icon: 'mdi:cctv-off',
     resetOnLoad: false,
+    rebuildGraceSeconds: 0,
   },
   not_loading: {
     localizationKey: 'issues.media_unavailable.reasons.not_loading',
     icon: 'mdi:progress-helper',
     resetOnLoad: true,
+    // A load that has not arrived yet has not necessarily failed: the provider is
+    // mounted and still trying, so rebuilding it destroys an attempt that may be
+    // about to succeed. Hold a rebuild off for at least this long, three load
+    // windows, to give that attempt time to finish.
+    rebuildGraceSeconds: 30,
   },
   playback_error: {
     localizationKey: 'issues.media_unavailable.reasons.playback_error',
@@ -84,16 +103,19 @@ export const MEDIA_UNAVAILABLE_REASONS: Record<
 
     // A player can load media and still fail to play it.
     resetOnLoad: false,
+    rebuildGraceSeconds: 0,
   },
   server_error: {
     localizationKey: 'issues.media_unavailable.reasons.server_error',
     icon: 'mdi:server-network-off',
     resetOnLoad: true,
+    rebuildGraceSeconds: 0,
   },
   stalled: {
     localizationKey: 'issues.media_unavailable.reasons.stalled',
     icon: 'mdi:motion-pause',
     resetOnLoad: false,
+    rebuildGraceSeconds: 0,
   },
   unsupported: {
     localizationKey: 'issues.media_unavailable.reasons.unsupported',
@@ -102,6 +124,7 @@ export const MEDIA_UNAVAILABLE_REASONS: Record<
     // Substitute pictures are never announced as loaded media, so a load means
     // the requested media was delivered in some supported way after all.
     resetOnLoad: true,
+    rebuildGraceSeconds: 0,
   },
 };
 
@@ -127,12 +150,30 @@ export class MediaUnavailableIssue implements Issue {
   // =========================================================================
 
   public trigger(context: IssueTriggerContext['media_unavailable']): void {
+    // A target already failing this same way keeps the deadline it was given,
+    // so repeat reports of one failure cannot push a rebuild out indefinitely.
+    const existing = this._erroredTargets.get(context.targetID);
     this._erroredTargets.set(context.targetID, {
       reason: context.reason,
       description: context.description,
+      rebuildNotBefore:
+        existing?.reason === context.reason
+          ? existing.rebuildNotBefore
+          : this._getRebuildDeadline(context.reason),
     });
   }
 
+  private _getRebuildDeadline(reason: MediaUnavailableIssueReason): Date {
+    return add(new Date(), {
+      seconds: MEDIA_UNAVAILABLE_REASONS[reason].rebuildGraceSeconds,
+    });
+  }
+
+  // Whether this target's media has waited long enough to be rebuilt.
+  private _isRebuildDue(error: TargetError): boolean {
+    return new Date() >= error.rebuildNotBefore;
+  }
+
   public resolve(context: IssueResolveContext['media_unavailable']): void {
     const error = this._erroredTargets.get(context.targetID);
     if (!error) {
@@ -236,8 +277,21 @@ export class MediaUnavailableIssue implements Issue {
     return this.hasIssue();
   }
 
-  public retry(): boolean {
-    const retryTargets = this._getDisplayedErrors();
+  // False while everything on screen is still within its grace period, so the
+  // manager treats the moment as one where nothing was attempted rather than as
+  // a failed attempt that should lengthen the wait for the next one.
+  public canRetryNow(): boolean {
+    return [...this._getDisplayedErrors().values()].some((error) =>
+      this._isRebuildDue(error),
+    );
+  }
+
+  public retry(force?: boolean): boolean {
+    const retryTargets = new Map(
+      [...this._getDisplayedErrors()].filter(
+        ([, error]) => force || this._isRebuildDue(error),
+      ),
+    );
     if (!retryTargets.size) {
       return false;
     }
@@ -250,6 +304,10 @@ export class MediaUnavailableIssue implements Issue {
       mediaEpoch[id] = (mediaEpoch[id] ?? 0) + 1;
     }
 
+    for (const error of retryTargets.values()) {
+      error.rebuildNotBefore = this._getRebuildDeadline(error.reason);
+    }
+
     // Intentionally keep _erroredTargets in place. The issue stays visible
     // while the provider re-attempts loading underneath. If the retry succeeds,
     // the fresh load clears a not-loading error and the rebuilt provider's

src/card-controller/issues/state-manager.ts

@@ -141,7 +141,7 @@ export class IssueStateManager implements IssueReadOnlyState {
       if (!force && !this._canRetryNow(issue)) {
         continue;
       }
-      if (issue.retry?.()) {
+      if (issue.retry?.(force)) {
         return;
       }
     }

src/card-controller/issues/types.ts

@@ -92,9 +92,10 @@ export interface Issue {
   // retries bypass it. Defaults to needsRetry() when not implemented.
   canRetryNow?(): boolean;
 
-  // Called by the manager when a retry is due. Returns true to stop the retry
-  // loop (exclusive), false to allow subsequent issues to also retry.
-  retry?(): boolean;
+  // Called by the manager when a retry is due. `force` marks a retry the user
+  // asked for by hand. Returns true to stop the retry loop (exclusive), false
+  // to allow subsequent issues to also retry.
+  retry?(force?: boolean): boolean;
 
   // Optional user-initiated fix. Not called by the issue infrastructure --
   // callers (e.g. notification control actions) invoke this directly.