fix: Report media failures once per failure (#2666)

tests/browser/test-media.ts

@@ -75,6 +75,15 @@ export const createUnansweredMediaURL = (): string => createTestMediaURL([]);
 export const createStallingMediaURL = (filename?: string): string =>
   createTestMediaURL([HTTP_OK], false, filename);
 
+/**
+ * How many requests a media URL has been asked for, so a test can count what
+ * the card actually fetched rather than only what it displayed.
+ */
+export const getTestMediaRequestCount = (url: string): number => {
+  const token = new URL(url, window.location.href).searchParams.get('token');
+  return (token ? requestCounts.get(token) : null) ?? 0;
+};
+
 /**
  * Serves a fixture at `/test-media/<file>`, behaving as the query asks:
  *

tests/card-controller/issues/issues/media-unavailable.browser.test.ts

@@ -16,6 +16,7 @@ import {
   createStallingMediaURL,
   createTemporarilyFailingMediaURL,
   createUnansweredMediaURL,
+  getTestMediaRequestCount,
   useTestMedia,
 } from '../../../browser/test-media';
 import {
@@ -375,6 +376,21 @@ describe('MediaUnavailableIssue', () => {
 
     expect(getBlockNotificationText(card.card)).toContain('Could not load image');
     expect(getBlockNotificationText(card.card)).toContain(CAMERA_ENTITY);
+
+    expect(card.events.getEntries('advanced-camera-card:issue:trigger')).toHaveLength(1);
+  });
+
+  it('should trigger an issue for an image view whose media fails to load', async () => {
+    const card = await mountCard({
+      view: { default: 'image' },
+      image: { mode: 'url', url: createFailingMediaURL() },
+    });
+
+    await card.events.waitForFirst('advanced-camera-card:issue:trigger');
+    await waitForIssueReported(card);
+
+    expect(isIssueReported(card)).toBe(true);
+    expect(card.events.getEntries('advanced-camera-card:issue:trigger')).toHaveLength(1);
   });
 
   it('should clear the issue report once the camera delivers media again', async () => {
@@ -463,12 +479,11 @@ describe('MediaUnavailableIssue', () => {
   });
 
   it('should re-attempt when the retry control is used', async () => {
+    const mediaURL = createTemporarilyFailingMediaURL(1);
     const card = await mountCard({
       // Automatic retries switched off.
       view: { issues: { retry_seconds: 0 } },
-      cameras: [
-        createStillImageCameraConfig(CAMERA_ENTITY, createTemporarilyFailingMediaURL(1)),
-      ],
+      cameras: [createStillImageCameraConfig(CAMERA_ENTITY, mediaURL)],
     });
 
     await card.events.waitForFirst('advanced-camera-card:issue:trigger');
@@ -483,6 +498,8 @@ describe('MediaUnavailableIssue', () => {
 
     expect(isIssueReported(card)).toBe(false);
     expect(isLiveMediaShowing(card.card)).toBe(true);
+
+    expect(getTestMediaRequestCount(mediaURL)).toBe(2);
   });
 
   it('should not report while a non-media view is showing', async () => {
@@ -551,7 +568,7 @@ describe('MediaUnavailableIssue', () => {
     expect(getBlockNotificationText(card.card)).toContain('Stream stalled');
   });
 
-  it('should trigger an issue for a camera picture that falls back to a stock image', async () => {
+  it('should trigger an issue for a camera snapshot that falls back to a stock image', async () => {
     // A camera-mode image that cannot be fetched swaps in a bundled stock
     // picture, so something is always on screen. That substitute must not be
     // announced as the camera's media arriving, or the failure it replaced
@@ -589,6 +606,50 @@ describe('MediaUnavailableIssue', () => {
     expect(card.events.getEntries('advanced-camera-card:media:loaded')).toHaveLength(0);
   });
 
+  it('should resolve the issue for a refreshing camera snapshot that recovers by itself', async () => {
+    // Must use the real clock: the picture is refetched on a timer that fake
+    // time would run through instantly, without the fetch in between ever being
+    // answered.
+    vi.useRealTimers();
+
+    const card = await MountedCardFactory.createFromSource(
+      createStillImageCardConfig({
+        status_bar: { style: 'outside' },
+
+        // Automatic retries disabled.
+        view: { issues: { retry_seconds: 0 } },
+        cameras: [
+          {
+            camera_entity: CAMERA_ENTITY,
+            live_provider: 'image',
+            image: { mode: 'camera', refresh_seconds: 1 },
+          },
+        ],
+      }),
+      createGenericCameraHASS({
+        entities: {
+          [CAMERA_ENTITY]: {
+            state: 'idle',
+            attributes: { entity_picture: createTemporarilyFailingMediaURL(1) },
+          },
+        },
+      }),
+    );
+
+    await card.events.waitForFirst('advanced-camera-card:issue:trigger');
+    await waitForIssueReported(card);
+
+    // A picture that is refetched on a timer can recover on its own, and the
+    // issue has to recover with it.
+    await card.events.waitForFirst('advanced-camera-card:media:loaded');
+    await waitForIssueCleared(card);
+
+    expect(isLiveMediaShowing(card.card)).toBe(true);
+
+    // Cameras that fetch each second must not announce failures each second.
+    expect(card.events.getEntries('advanced-camera-card:issue:trigger')).toHaveLength(1);
+  });
+
   it('should report a player that reports a playback error', async () => {
     const card = await mountCard({
       // A provider given nothing to play. It reports that it failed without

tests/card-controller/issues/issues/media-unavailable.test.ts

@@ -230,6 +230,32 @@ describe('MediaUnavailableIssue', () => {
 
       expect(issue.hasIssue()).toBe(false);
     });
+
+    it('should do nothing for a load on a target that never failed', () => {
+      const issue = new MediaUnavailableIssue(createAPIDisplaying('camera-1'));
+
+      issue.resolve({ targetID: 'camera-1', cause: 'media-loaded' });
+
+      expect(issue.hasIssue()).toBe(false);
+    });
+
+    describe('when the media loads', () => {
+      it.each([
+        ['entity_unavailable' as const, false],
+        ['not_loading' as const, true],
+        ['playback_error' as const, false],
+        ['server_error' as const, true],
+        ['stalled' as const, false],
+        ['unsupported' as const, true],
+      ])('should reset a %s failure: %s', (reason, cleared) => {
+        const issue = new MediaUnavailableIssue(createAPIDisplaying('camera-1'));
+
+        issue.trigger({ targetID: 'camera-1', reason });
+        issue.resolve({ targetID: 'camera-1', cause: 'media-loaded' });
+
+        expect(issue.hasIssue()).toBe(!cleared);
+      });
+    });
   });
 
   describe('trigger', () => {

tests/components-lib/live/liveness/detectors/entity-availability.test.ts

@@ -214,11 +214,29 @@ describe('EntityAvailabilityDetector', () => {
 
     // Re-point at the (now available) entity from a fresh state.
     setEntityState('streaming');
-    detector.reset();
+    detector.invalidate('stream-changed');
 
     expect(detector.getVerdict()).toEqual({ state: 'unknown' });
   });
 
+  it('should keep the verdict when media loads', () => {
+    const { detector, fireStateChange } = setup();
+    detector.subscribe();
+    fireStateChange('unavailable');
+    vi.advanceTimersByTime(GRACE_MS);
+
+    // This detector watches the camera entity, so media loading tells it
+    // nothing about the entity it is watching.
+    detector.invalidate('media-loaded');
+
+    expect(detector.getVerdict()).toEqual({
+      state: 'not_live',
+      authority: 'indirect',
+      renderPlaceholder: true,
+      reason: 'entity_unavailable',
+    });
+  });
+
   it('should re-check the entity when reset', () => {
     // always_error makes the re-check produce a verdict immediately rather than
     // waiting out the grace window.
@@ -228,7 +246,7 @@ describe('EntityAvailabilityDetector', () => {
     // An entity that is already unavailable never fires a state change, so
     // resetting must read it rather than wait to be told.
     setEntityState('unavailable');
-    detector.reset();
+    detector.invalidate('stream-changed');
 
     expect(detector.getVerdict()).toEqual(
       expect.objectContaining({ state: 'not_live', authority: 'hard' }),
@@ -238,7 +256,7 @@ describe('EntityAvailabilityDetector', () => {
   it('should do nothing on reset before subscribe', () => {
     const { detector, stateWatcher } = setup();
 
-    detector.reset();
+    detector.invalidate('stream-changed');
 
     expect(stateWatcher.subscribe).not.toHaveBeenCalled();
     expect(detector.getVerdict()).toEqual({ state: 'unknown' });
@@ -249,7 +267,7 @@ describe('EntityAvailabilityDetector', () => {
     detector.subscribe();
 
     setCameraEntity(null);
-    detector.reset();
+    detector.invalidate('stream-changed');
 
     expect(stateWatcher.unsubscribe).toHaveBeenCalled();
     expect(detector.getVerdict()).toEqual({ state: 'unknown' });

tests/components-lib/live/liveness/detectors/media-player-liveness.test.ts

@@ -248,12 +248,34 @@ describe('MediaPlayerLivenessDetector', () => {
     await callIntersectionHandler(true);
     fireMediaPlayerLiveness(false);
 
-    detector.reset();
+    detector.invalidate('stream-changed');
 
     expect(detector.getVerdict()).toEqual({ state: 'unknown' });
     expect(unsubscribe).toHaveBeenCalledTimes(1);
   });
 
+  it('should keep the verdict when media loads', async () => {
+    const { detector, loadMedia } = setup();
+    const { player, unsubscribe, fireMediaPlayerLiveness } = createPlayer();
+    detector.subscribe();
+    loadMedia(player);
+
+    await callIntersectionHandler(true);
+    fireMediaPlayerLiveness(false);
+
+    // The media player reported that media it had already loaded stopped
+    // delivering frames, which another load does not answer.
+    detector.invalidate('media-loaded');
+
+    expect(detector.getVerdict()).toEqual({
+      state: 'not_live',
+      authority: 'direct',
+      renderPlaceholder: true,
+      reason: 'stalled',
+    });
+    expect(unsubscribe).not.toHaveBeenCalled();
+  });
+
   it('should tear down the watch and stop listening on unsubscribe', async () => {
     const { detector, loadMedia } = setup();
     const first = createPlayer();

tests/components-lib/live/liveness/detectors/provider-error.test.ts

@@ -13,7 +13,7 @@ const createHostInDocument = (): HTMLElement => {
 
 // @vitest-environment jsdom
 describe('ProviderErrorDetector', () => {
-  it('should not report a change when reset', () => {
+  it('should not report a change when the stream changes', () => {
     const host = createHostInDocument();
     const onChange = vi.fn();
     const detector = new ProviderErrorDetector(host, onChange);
@@ -21,7 +21,7 @@ describe('ProviderErrorDetector', () => {
     dispatchLiveErrorEvent(host);
     onChange.mockClear();
 
-    detector.reset();
+    detector.invalidate('stream-changed');
 
     expect(onChange).not.toHaveBeenCalled();
   });
@@ -93,14 +93,14 @@ describe('ProviderErrorDetector', () => {
     document.body.removeEventListener(LIVE_ERROR_EVENT, parentListener);
   });
 
-  it('should discard the verdict on reset', () => {
+  it('should discard the verdict when the stream changes', () => {
     const host = createHostInDocument();
     const detector = new ProviderErrorDetector(host, vi.fn());
     detector.subscribe();
     dispatchLiveErrorEvent(host);
     expect(detector.getVerdict().state).toBe('not_live');
 
-    detector.reset();
+    detector.invalidate('stream-changed');
 
     expect(detector.getVerdict()).toEqual({ state: 'unknown' });
   });
@@ -117,4 +117,31 @@ describe('ProviderErrorDetector', () => {
     expect(detector.getVerdict().state).toBe('unknown');
     expect(onChange).not.toHaveBeenCalled();
   });
+
+  it('should clear the failure when media arrives', () => {
+    const host = createHostInDocument();
+    const detector = new ProviderErrorDetector(host, vi.fn());
+    detector.subscribe();
+    dispatchLiveErrorEvent(host);
+
+    // The provider said it could not deliver the media, and then delivered it.
+    detector.invalidate('media-loaded');
+
+    expect(detector.getVerdict()).toEqual({ state: 'unknown' });
+  });
+
+  it('should report a later error after media cleared the previous one', () => {
+    const host = createHostInDocument();
+    const onChange = vi.fn();
+    const detector = new ProviderErrorDetector(host, onChange);
+    detector.subscribe();
+    dispatchLiveErrorEvent(host);
+    detector.invalidate('media-loaded');
+    onChange.mockClear();
+
+    dispatchLiveErrorEvent(host);
+
+    expect(detector.getVerdict().state).toBe('not_live');
+    expect(onChange).toHaveBeenCalledTimes(1);
+  });
 });

tests/components-lib/live/liveness/stream-liveness-controller.test.ts

@@ -372,6 +372,28 @@ describe('StreamLivenessController', () => {
       ]);
     });
 
+    it('should stop reporting a provider error once media loads', () => {
+      const { host, controller, issueResolves, failViaProviderError } = setup();
+      controller.hostConnected();
+      failViaProviderError({ reason: 'not_loading' });
+
+      host.dispatchEvent(createMediaLoadedInfoEvent({ info: createMediaLoadedInfo() }));
+
+      // The detector holding the failure goes quiet rather than claiming the
+      // stream is live, so this controller has nothing to state either way.
+      expect(controller.isLive()).toBe(true);
+      expect(issueResolves).toEqual([]);
+    });
+
+    it('should not resolve when media arrives with no failure to disprove', () => {
+      const { host, controller, issueResolves } = setup();
+      controller.hostConnected();
+
+      host.dispatchEvent(createMediaLoadedInfoEvent({ info: createMediaLoadedInfo() }));
+
+      expect(issueResolves).toEqual([]);
+    });
+
     it('should not resolve while a hard failure exists', async () => {
       const { issueResolves, failViaProviderError, fireMediaPlayerLiveness } =
         await setupWithLiveStream();

tests/components-lib/media-load-watchdog-controller.test.ts

@@ -176,19 +176,20 @@ describe('MediaLoadWatchdogController', () => {
       harness.mediaLoaded('camera-1');
 
       expect(harness.resolveRequests).toEqual([
-        { key: 'media_unavailable', targetID: 'camera-1', reason: 'not_loading' },
+        { key: 'media_unavailable', targetID: 'camera-1', cause: 'media-loaded' },
       ]);
     });
 
-    it('should clear a failure it never reported itself', () => {
+    it('should resolve on a load even when it reported no failure itself', () => {
       const harness = createHarness();
       harness.connect();
 
-      // Another component can report the same kind of failure for this target.
+      // A load resolves whatever failure the target has regardless of the
+      // component that reported it -- not only this watchdog's own timeout.
       harness.mediaLoaded('camera-1');
 
       expect(harness.resolveRequests).toEqual([
-        { key: 'media_unavailable', targetID: 'camera-1', reason: 'not_loading' },
+        { key: 'media_unavailable', targetID: 'camera-1', cause: 'media-loaded' },
       ]);
     });
 
@@ -334,9 +335,11 @@ describe('MediaLoadWatchdogController', () => {
       harness.setTargetID('camera-2');
       harness.mediaLoaded('camera-2');
 
+      // The abandoned target gets only its not-loading failure resolved: no
+      // load arrived for it.
       expect(harness.resolveRequests).toEqual([
         { key: 'media_unavailable', targetID: 'camera-1', reason: 'not_loading' },
-        { key: 'media_unavailable', targetID: 'camera-2', reason: 'not_loading' },
+        { key: 'media_unavailable', targetID: 'camera-2', cause: 'media-loaded' },
       ]);
     });
 

tests/components/image-updating-player.browser.test.ts

@@ -11,6 +11,8 @@ import {
 
 const UNRELATED_ENTITY = 'input_boolean.unrelated';
 
+const IMAGE_ERROR_EVENT = 'advanced-camera-card:image-updating-player:error';
+
 // The size of the static image fixture the player loads. Everything downstream
 // sizes the card from what the player measured, so these are read from the
 // media rather than declared anywhere in the configuration.
@@ -57,6 +59,28 @@ describe('AdvancedCameraCardImageUpdatingPlayer', () => {
     expect(loads[0].info.targetID).toBe(CAMERA_ENTITY);
   });
 
+  it('should ignore an image failure that arrives after the card leaves the page', async () => {
+    const card = await mount();
+    const player = await card.waitForSelector(
+      'advanced-camera-card-image-updating-player',
+    );
+    const image = await card.waitForSelector('img');
+
+    const failures: Event[] = [];
+    player.addEventListener(IMAGE_ERROR_EVENT, (ev) => failures.push(ev));
+
+    image.dispatchEvent(new Event('error'));
+    expect(failures).toHaveLength(1);
+
+    // The browser answers a request the player made before the card came off
+    // the page. Nobody is looking at the media it was for, so the player must
+    // say nothing about it.
+    card.detach();
+    image.dispatchEvent(new Event('error'));
+
+    expect(failures).toHaveLength(1);
+  });
+
   it('should announce the size of the media itself', async () => {
     const card = await mount();
 

tests/components/viewer/carousel.browser.test.ts

@@ -179,6 +179,40 @@ describe('AdvancedCameraCardViewerCarousel', () => {
     );
   });
 
+  it('should build a new player for a failed clip when the retry control is used', async () => {
+    vi.useFakeTimers();
+    onTestFinished(() => {
+      vi.useRealTimers();
+    });
+
+    const { card, frigate } = await mountCardWithFrigate(EVENTS, {
+      // Automatic retries switched off, so the control below is the only one.
+      view: { default: 'clips', issues: { retry_seconds: 0 } },
+      status_bar: { style: 'outside' },
+    });
+    frigate.setMediaURL('newer', 'clips', createFailingMediaURL());
+
+    await waitForThumbnails(card, EVENTS.length);
+    await clickThumbnail(card.card, 0);
+    const failedPlayer = await card.waitForSelector('video');
+    await card.advanceSeconds(MEDIA_LOADING_TIMEOUT_SECONDS);
+    await card.waitForRender(
+      () => getStatusBarItem(card.card, MEDIA_ISSUE_TITLE),
+      `the ${MEDIA_ISSUE_TITLE} issue being reported`,
+    );
+
+    await card.clickControl(MEDIA_ISSUE_TITLE);
+    await card.clickControl('Retry');
+
+    // A second player, not the one that failed: the retry throws the old one
+    // away, and the replacement has to render and ask for the clip again rather
+    // than sit there empty.
+    await card.waitForRender(() => {
+      const player = deepQuery(card.card, 'video');
+      return player && player !== failedPlayer ? player : null;
+    }, 'the clip player being rebuilt');
+  });
+
   it('should return to the gallery from the viewer', async () => {
     const card = await mountViewer(EVENTS, {
       config: {