fix(player): guard play/pause/removeEventListener against null refs

[miguel-heygen]

Summary

• Guard .play(), .pause(), and .removeEventListener() calls across the player runtime, parent-media proxy, and studio player hooks against null, stale, or non-media element refs
• Fixes ~133 unhandled errors/month: e.play is not a function (15/mo), e.pause is not a function (91/mo), removeEventListener is not a function (27/mo)
• All guards use typeof x.method === "function" checks so they silently skip destroyed or non-conformant elements rather than throwing

Changed files

File	What
packages/core/src/runtime/media.ts	Guard el.play() and el.pause() in syncRuntimeMedia active/inactive paths
packages/core/src/runtime/init.ts	Guard node.removeEventListener in cleanup handler, mediaEl.removeEventListener in metadata unbind
packages/player/src/parent-media.ts	Guard m.el.play/pause in playAll, pauseAll, destroy, _detachIframeMedia, _adoptIframeMedia
packages/studio/src/player/hooks/usePlaybackKeyboard.ts	Guard iframeWin/iframeDoc.removeEventListener in shortcut cleanup closure
packages/studio/src/player/components/PlayerControls.tsx	Guard target.removeEventListener in scrub pointer cleanup
packages/studio/src/player/components/Player.tsx	Guard iframe/player.removeEventListener in mount-effect cleanup

Test plan

• bun run build passes (all packages)
• bunx oxlint clean on all 6 files
• bunx oxfmt --check clean on all 6 files
• packages/core tests pass (983 tests)
• packages/player tests pass (110 tests)
• packages/studio tests pass (606 tests)
• Verify in Studio that play/pause/seek still works after composition reload
• Verify no new console errors when switching compositions rapidly

Closes PRINFRA-174

[jrusso1020]

Verdict: COMMENT (would-be APPROVE)

Defensive PR with appropriate scope. Holding for James's stamp greenlight.

Sanity checks

• Scope is correctly bounded — element refs only, globals untouched. Cross-checked every removeEventListener call site across packages/core/src/runtime/, packages/player/src/, and packages/studio/src/player/. The PR guards exactly the sites that operate on element refs (node, mediaEl, iframe, player, target, iframeWin, iframeDoc) — every one of which can become stale if the DOM element is removed/replaced before cleanup runs. It deliberately doesn't guard window.removeEventListener / document.removeEventListener calls (e.g. picker.ts:248-250, init.ts:2109-2121, controls.ts:366-374, useTimelineClipDrag.ts:372-374, EditModal.tsx:43,53), which is right — window and document can't be destroyed during a session. ✓ Correct parity logic.

• All play()/pause() call sites on DOM media accounted for. media.ts:261/277/286 — guarded. parent-media.ts:101/121/124/128/283/297 — guarded. The remaining .play()/.pause() sites in runtime/player.ts, runtime/adapters/{animejs,gsap}.ts operate on JS timeline objects (GSAP/AnimeJS instances), not DOM media — different invariants, not in scope. ✓

• Tests pass at scale: 983 (core) + 110 (player) + 606 (studio) = 1,699 tests. CI mid-flight with 11 success / 17 in_progress / 0 failures so far.

• Shape of each guard matches the loop semantics:

  • playAll(): continue early (skips the element entirely — right, since src+play are both prerequisites)
  • pauseAll(), destroy(), _adoptIframeMedia, _detachIframeMedia: no-op the pause but continue with other work like clearing src (right, pause is best-effort)
  • syncRuntimeMedia el.play !== "function": continue to skip the whole element (right — if play is missing, the element is in an invalid state)
  • All cleanup-style guards (Player.tsx, PlayerControls.tsx, usePlaybackKeyboard.ts, init.ts): no-op the removeEventListener, continue with surrounding cleanup (right — failed unbind doesn't gate other cleanup)

One observation worth surfacing — observability gap

This is a symptom fix, not a root cause fix. The 133 errors/month come from somewhere — most likely stale refs in tracked collections (metadataBoundMedia Set, this._entries array, React-captured iframe/player refs) that are not being cleaned up when their underlying DOM elements are removed. The fix correctly hardens the consumer side, but loses the signal that something is wrong upstream:

• Before: 133 errors/month tell us "something is putting destroyed elements into our tracked collections." Noisy but informative.
• After: 0 errors/month. Quiet but blind — we no longer see when the data-structure invariants break.

Optional defensive-with-observability variant: emit a low-volume telemetry event (media_ref_destroyed or similar) the first time per session a guard trips, so we keep a signal channel for the root cause without re-flooding the channel:

if (typeof el.play !== "function") {
  if (!sessionFlags.has("guard:media_play")) {
    sessionFlags.add("guard:media_play");
    trackStudioEvent("guard_triggered", { site: "syncRuntimeMedia:play" });
  }
  continue;
}

Not blocking — the current PR is a valid first step to clean up the telemetry. But worth a follow-up either to find the root causes (e.g., proper unbind paths from metadataBoundMedia when nodes detach, MutationObserver / WeakRef-based collection hygiene) or to add the gated telemetry above.

— Rames Jusso