From 5ba5905637e6b468ce6b4ba3adb298c6f155afca Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Wed, 1 Jul 2026 22:19:22 -0500 Subject: [PATCH] =?UTF-8?q?fix(#146):=20web=20player=20=E2=80=94=20guard?= =?UTF-8?q?=20PlayerMediaHealth=20calls=20by=20METHOD,=20not=20object=20(s?= =?UTF-8?q?tale-module=20TypeError)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live error: "Uncaught (in promise) TypeError: PlayerMediaHealth.shouldShowIdle is not a function" inside the device:paired socket handler. WHAT WAS ACTUALLY WRONG: not a missing/misnamed definition — the module DOES export shouldShowIdle (served + unit-tested). It's a VERSION SKEW: /player/* is served network-first by the service worker, so a transient module-fetch failure falls back to the STALE cache (an older player-media-health.js that predates shouldShowIdle) while index.html loads fresh with the call. window.PlayerMediaHealth then exists but lacks the method, and the call site guarded the OBJECT (`window.PlayerMediaHealth ? ...`) not the METHOD — so it threw, aborting the rest of the device:paired handler (the showStatus after it was skipped). FIX: guard the METHOD at both call sites (typeof X.method === 'function') so a stale/partial module can never throw — it falls back to the safe inline default (!isPlaying for the idle decision) and the handler runs to completion. SIBLING (errors travel in pairs): the needsReattach call in the "Playlist unchanged" branch had the SAME object-not-method guard. It was inside a try/catch so it couldn't throw uncaught, but a stale module would silently skip the re-attach/hideStatus. Guarded it the same way. No service-worker change needed: network-first already self-heals on the next good load; the method-guard covers the transient/offline-fallback skew permanently. Tests: player-media-health.test.js +1 module-surface test (both needsReattach and shouldShowIdle are exported functions — catches the define-vs-call class). Inline player JS syntax-checked; both guards verified present. Suite 319/319. Co-Authored-By: Claude Opus 4.8 (1M context) --- server/player/index.html | 11 ++++++++--- server/test/player-media-health.test.js | 11 ++++++++++- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/server/player/index.html b/server/player/index.html index dc788c3..c5b8418 100644 --- a/server/player/index.html +++ b/server/player/index.html @@ -812,8 +812,13 @@ // playing. Showing the idle "Waiting for content..." overlay unconditionally covered // the live video (audio kept playing underneath) and the subsequent "Playlist // unchanged" left it up. Only fall to idle when nothing is actually playing. - const showIdle = window.PlayerMediaHealth - ? PlayerMediaHealth.shouldShowIdle({ isPlaying: isPlaying, hasContent: playlist.length > 0 }) + // Guard the METHOD, not just the object: a device running a stale-cached + // player-media-health.js (older module, no shouldShowIdle) would otherwise throw + // "shouldShowIdle is not a function" here and abort the rest of this handler. The + // !isPlaying fallback is equivalent for the playing case (playing -> not idle). + const PMH = window.PlayerMediaHealth; + const showIdle = (PMH && typeof PMH.shouldShowIdle === 'function') + ? PMH.shouldShowIdle({ isPlaying: isPlaying, hasContent: playlist.length > 0 }) : !isPlaying; if (showIdle) showStatus('Waiting for content...'); }); @@ -1338,7 +1343,7 @@ : null, surfaceAttached: !!(container && container.querySelector('video,img,iframe,.wall-stage')), }; - if (window.PlayerMediaHealth && PlayerMediaHealth.needsReattach(state)) { + if (window.PlayerMediaHealth && typeof PlayerMediaHealth.needsReattach === 'function' && PlayerMediaHealth.needsReattach(state)) { console.log('[refresh] media surface lost on no-change refresh — re-attaching current item'); playCurrentItem(); } else if (isPlaying) { diff --git a/server/test/player-media-health.test.js b/server/test/player-media-health.test.js index afdd0ba..150014a 100644 --- a/server/test/player-media-health.test.js +++ b/server/test/player-media-health.test.js @@ -6,7 +6,16 @@ const { test } = require('node:test'); const assert = require('node:assert/strict'); -const { needsReattach, shouldShowIdle } = require('../lib/player-media-health'); +const PlayerMediaHealth = require('../lib/player-media-health'); +const { needsReattach, shouldShowIdle } = PlayerMediaHealth; + +// Guards the define-vs-call class of bug: every method the player calls on +// PlayerMediaHealth must actually be exported (a missing one throws "X is not a function" +// at the browser call site and aborts the socket handler). +test('module surface: needsReattach AND shouldShowIdle are both exported functions', () => { + assert.equal(typeof PlayerMediaHealth.needsReattach, 'function'); + assert.equal(typeof PlayerMediaHealth.shouldShowIdle, 'function'); +}); const video = (o) => ({ isPlaying: true, hasCurrentItem: true, itemKind: 'video', videoEl: o, surfaceAttached: true });