mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
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`.
This commit is contained in:
1 parent
62d0a366ee
commit
2943fbb9f7
5 files changed
+204
-1
No files matched your search
@@ -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.
|
||||||
@@ -4902,7 +4902,13 @@ class Application {
|
|||||||
|
|
||||||
resetTorrentStats() {
|
resetTorrentStats() {
|
||||||
if (this.videoModal) {
|
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."
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -75,6 +75,10 @@ class EmbedVideoModal {
|
|||||||
return this.video;
|
return this.video;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
resetStats() {
|
||||||
|
// No-op for embed view
|
||||||
|
}
|
||||||
|
|
||||||
updateStatus(message) {
|
updateStatus(message) {
|
||||||
setStatus(message, "default");
|
setStatus(message, "default");
|
||||||
}
|
}
|
||||||
@@ -353,6 +357,11 @@ const setPointerState = ({ video, hint } = {}) => {
|
|||||||
};
|
};
|
||||||
|
|
||||||
const startEmbed = async () => {
|
const startEmbed = async () => {
|
||||||
|
if (window.__bitvidEmbedStarted) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
window.__bitvidEmbedStarted = true;
|
||||||
|
|
||||||
if (!(videoEl instanceof HTMLVideoElement)) {
|
if (!(videoEl instanceof HTMLVideoElement)) {
|
||||||
setStatus("Embed video element is missing.", "error");
|
setStatus("Embed video element is missing.", "error");
|
||||||
return;
|
return;
|
||||||
|
|||||||
@@ -1218,6 +1218,7 @@ export class NostrClient {
|
|||||||
this.sessionActorCipherClosures = null;
|
this.sessionActorCipherClosures = null;
|
||||||
this.sessionActorCipherClosuresPrivateKey = null;
|
this.sessionActorCipherClosuresPrivateKey = null;
|
||||||
this.countUnsupportedRelays = new Set();
|
this.countUnsupportedRelays = new Set();
|
||||||
|
this.isInitialized = false;
|
||||||
this.nip46Client = null;
|
this.nip46Client = null;
|
||||||
this.remoteSignerListeners = new Set();
|
this.remoteSignerListeners = new Set();
|
||||||
this.sessionActorListeners = new Set();
|
this.sessionActorListeners = new Set();
|
||||||
@@ -3735,6 +3736,11 @@ export class NostrClient {
|
|||||||
* @returns {Promise<void>} Resolves when the relay pool is initialized and connections are attempted.
|
* @returns {Promise<void>} Resolves when the relay pool is initialized and connections are attempted.
|
||||||
*/
|
*/
|
||||||
async init() {
|
async init() {
|
||||||
|
if (this.isInitialized) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
this.isInitialized = true;
|
||||||
|
|
||||||
devLogger.log("Connecting to relays...");
|
devLogger.log("Connecting to relays...");
|
||||||
|
|
||||||
// 1. Restore cache for immediate UI render (Stale-While-Revalidate)
|
// 1. Restore cache for immediate UI render (Stale-While-Revalidate)
|
||||||
|
|||||||
@@ -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();
|
||||||
Reference in new issue
Block a user