From 2943fbb9f72e69fb4f4b91ea7661e22b6b3fd999 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sat, 24 Jan 2026 21:42:37 +0000 Subject: [PATCH] Fix embed player crash and prevent duplicate initialization - Fix `TypeError: this.videoModal.resetStats is not a function` by implementing `resetStats` (no-op) in `EmbedVideoModal` and adding a safety guard in `Application.resetTorrentStats`. - Prevent duplicate `NostrClient` initialization within the same frame by adding idempotent guards in `js/embed.js` and `js/nostr/client.js`. - Add documentation (`docs/embed-notes.md`) on embed architecture and recommendations for multi-frame resource sharing (SharedWorker). - Add regression test `tests/embed_playback_check.spec.mjs`. --- docs/embed-notes.md | 48 ++++++++++ js/app.js | 8 +- js/embed.js | 9 ++ js/nostr/client.js | 6 ++ tests/embed_playback_check.spec.mjs | 134 ++++++++++++++++++++++++++++ 5 files changed, 204 insertions(+), 1 deletion(-) create mode 100644 docs/embed-notes.md create mode 100644 tests/embed_playback_check.spec.mjs diff --git a/docs/embed-notes.md b/docs/embed-notes.md new file mode 100644 index 00000000..0040fc81 --- /dev/null +++ b/docs/embed-notes.md @@ -0,0 +1,48 @@ +# Embed Mode Notes and Recommendations + +## Current Behavior +The embed player (`embed.html` / `js/embed.js`) initializes a full `NostrClient` instance within its iframe context. This includes: +- Initializing `SimplePool` from `nostr-tools`. +- Connecting to default relays (or configured ones). +- Subscribing to metadata (Kind 0) for liveness check. + +## Duplicate Initialization Issue +When multiple embeds are present on a single parent page: +1. Each iframe creates its own `NostrClient` and `SimplePool`. +2. Each iframe opens its own WebSocket connections to the same set of relays. +3. This results in $N \times M$ connections (N embeds, M relays), which can be resource-intensive and trigger rate limits. + +## Fixes Implemented +- **Idempotent Initialization**: Added guards in `js/embed.js` (`window.__bitvidEmbedStarted`) and `js/nostr/client.js` (`this.isInitialized`) to prevent accidental double-initialization within the *same* execution context. +- **Crash Fix**: Added missing `resetStats` method to `EmbedVideoModal` and a safety guard in `Application.resetTorrentStats` to prevent `TypeError`. + +## Recommended Long-Term Fixes + +To solve the multi-iframe connection duplication, a mechanism to share the relay connection across Same-Origin iframes is required. + +### 1. SharedWorker Relay Manager (Recommended) +Implement a `SharedWorker` (`js/relayWorker.js`) that acts as a singleton relay manager for the origin. +- **Architecture**: + - The worker holds the `SimplePool` and WebSocket connections. + - Embeds (and the main app) connect to the worker via `MessagePort`. + - The worker proxies subscription requests and events between the frames and the relays. +- **Pros**: + - Single connection per relay for the entire browser session/origin. + - Efficient resource usage. +- **Cons**: + - Requires refactoring `NostrClient` to support a "Worker Relay Adapter" instead of direct `SimplePool` usage. + - Browser support (SharedWorker not available in all contexts/browsers, fallback needed). + +### 2. BroadcastChannel Leader Election +Use `BroadcastChannel` to elect a "Leader" tab/frame that manages connections. +- **Architecture**: + - Frames join a channel. + - One frame becomes Leader. + - Followers request data via the channel. +- **Pros**: + - Works where SharedWorker might fail (though support is similar). +- **Cons**: + - Complex leader election/failover logic (what if leader frame closes?). + +### 3. Embed-Specific Minimal Runtime +Create a stripped-down `EmbedClient` that only implements the subset of Nostr functionality needed for playback (fetch by ID/pointer), bypassing the heavier `NostrClient` if possible, or optimizing `NostrClient` to be lighter. diff --git a/js/app.js b/js/app.js index 98b67078..3daa1d63 100644 --- a/js/app.js +++ b/js/app.js @@ -4902,7 +4902,13 @@ class Application { resetTorrentStats() { if (this.videoModal) { - this.videoModal.resetStats(); + if (typeof this.videoModal.resetStats === "function") { + this.videoModal.resetStats(); + } else { + devLogger.info( + "[Application] resetTorrentStats: videoModal.resetStats not available in this context — skipping." + ); + } } } diff --git a/js/embed.js b/js/embed.js index abd48003..f5b92583 100644 --- a/js/embed.js +++ b/js/embed.js @@ -75,6 +75,10 @@ class EmbedVideoModal { return this.video; } + resetStats() { + // No-op for embed view + } + updateStatus(message) { setStatus(message, "default"); } @@ -353,6 +357,11 @@ const setPointerState = ({ video, hint } = {}) => { }; const startEmbed = async () => { + if (window.__bitvidEmbedStarted) { + return; + } + window.__bitvidEmbedStarted = true; + if (!(videoEl instanceof HTMLVideoElement)) { setStatus("Embed video element is missing.", "error"); return; diff --git a/js/nostr/client.js b/js/nostr/client.js index 43222025..50e2b35d 100644 --- a/js/nostr/client.js +++ b/js/nostr/client.js @@ -1218,6 +1218,7 @@ export class NostrClient { this.sessionActorCipherClosures = null; this.sessionActorCipherClosuresPrivateKey = null; this.countUnsupportedRelays = new Set(); + this.isInitialized = false; this.nip46Client = null; this.remoteSignerListeners = new Set(); this.sessionActorListeners = new Set(); @@ -3735,6 +3736,11 @@ export class NostrClient { * @returns {Promise} Resolves when the relay pool is initialized and connections are attempted. */ async init() { + if (this.isInitialized) { + return; + } + this.isInitialized = true; + devLogger.log("Connecting to relays..."); // 1. Restore cache for immediate UI render (Stale-While-Revalidate) diff --git a/tests/embed_playback_check.spec.mjs b/tests/embed_playback_check.spec.mjs new file mode 100644 index 00000000..02f4957b --- /dev/null +++ b/tests/embed_playback_check.spec.mjs @@ -0,0 +1,134 @@ +import { chromium } from 'playwright'; +import { spawn } from 'child_process'; +import { startRelay } from '../scripts/agent/simple-relay.mjs'; +import fs from 'fs'; +import path from 'path'; +import { fileURLToPath } from 'url'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); +const ROOT_DIR = path.resolve(__dirname, '../'); + +async function runRepro() { + console.log('Starting reproduction script...'); + + const relayPort = 8899; + const relayUrl = `ws://localhost:${relayPort}`; + console.log(`Starting local relay on port ${relayPort}...`); + const relay = startRelay(relayPort); + + const serverPort = 8099; + console.log(`Starting web server on port ${serverPort}...`); + const serverProcess = spawn('npx', ['serve', '-p', String(serverPort)], { + cwd: ROOT_DIR, + stdio: 'ignore', + shell: true + }); + + await new Promise(resolve => setTimeout(resolve, 2000)); + + let browser; + let errorFound = false; + + try { + console.log('Launching headless browser...'); + browser = await chromium.launch({ headless: true }); + const page = await browser.newPage(); + + page.on('console', msg => { + console.log(`[BROWSER CONSOLE] ${msg.type()}: ${msg.text()}`); + if (msg.text().includes('this.videoModal.resetStats is not a function')) { + console.log('!!! CAUGHT EXPECTED ERROR IN CONSOLE !!!'); + errorFound = true; + } + }); + + page.on('pageerror', err => { + console.log(`[BROWSER PAGEERROR] ${err.message}`); + if (err.message.includes('this.videoModal.resetStats is not a function')) { + console.log('!!! CAUGHT EXPECTED ERROR IN PAGEERROR !!!'); + errorFound = true; + } + }); + + const baseUrl = `http://localhost:${serverPort}`; + console.log(`Navigating to ${baseUrl} to setup...`); + + await page.goto(`${baseUrl}/index.html`); + await page.waitForFunction(() => window.NostrTools); + + const eventId = await page.evaluate(async (relayUrl) => { + const { nostrClient } = await import('./js/nostrClientFacade.js'); + const { generateSecretKey, getPublicKey } = window.NostrTools; + const { bytesToHex } = window.NostrTools.utils || window.NostrTools; + + const sk = generateSecretKey(); + const privateKey = typeof sk === 'string' ? sk : bytesToHex(sk); + const pubkey = getPublicKey(sk); + + nostrClient.relays = [relayUrl]; + nostrClient.readRelays = [relayUrl]; + nostrClient.writeRelays = [relayUrl]; + await nostrClient.init(); + await nostrClient.registerPrivateKeySigner({ privateKey, pubkey }); + + const videoPayload = { + legacyFormData: { + title: 'Repro Video', + description: 'Repro description', + url: 'https://example.com/video.mp4', + mimeType: 'video/mp4', + } + }; + + const event = await nostrClient.publishVideo(videoPayload, pubkey); + return event.id; + }, relayUrl); + + console.log(`Published video event: ${eventId}`); + + const nevent = await page.evaluate(({ id, relayUrl }) => { + return window.NostrTools.nip19.neventEncode({ + id, + relays: [relayUrl] + }); + }, { id: eventId, relayUrl }); + + const embedUrl = `${baseUrl}/embed?pointer=${nevent}&playback=url`; + console.log(`Navigating to embed: ${embedUrl}`); + + const response = await page.goto(embedUrl); + console.log(`Response status: ${response.status()} ${response.statusText()}`); + console.log(`Response url: ${response.url()}`); + + const locationHref = await page.evaluate(() => window.location.href); + console.log(`Current Location: ${locationHref}`); + + console.log('Waiting for error...'); + await page.waitForTimeout(10000); + + const statusText = await page.evaluate(() => { + return document.getElementById('embedStatus')?.textContent; + }); + console.log(`Current Embed Status: "${statusText}"`); + + if (errorFound) { + console.error('FAILURE: TypeError still present.'); + } else { + console.log('SUCCESS: TypeError fixed (not found).'); + } + + } catch (err) { + console.error('Test script error:', err); + } finally { + if (browser) await browser.close(); + serverProcess.kill(); + relay.close(); + + if (errorFound) { + process.exit(1); + } + } +} + +runRepro();