From ded00445134f6c089c3e7c9ba2dab8894446b046 Mon Sep 17 00:00:00 2001 From: AAA <35992542+TNTcraftHIM@users.noreply.github.com> Date: Thu, 20 Aug 2026 03:26:40 +0800 Subject: [PATCH] fix(media): bound initial viewer recovery (#58) --- .../0005-automatic-hybrid-media-routing.md | 11 +- docs/research/advanced-peer-distribution.md | 3 +- docs/status.md | 3 +- src/client/webrtc/viewer-peer.ts | 40 ++++++ tests/viewer-peer-generation.test.ts | 118 +++++++++++++++++- 5 files changed, 162 insertions(+), 13 deletions(-) diff --git a/docs/adr/0005-automatic-hybrid-media-routing.md b/docs/adr/0005-automatic-hybrid-media-routing.md index 6666665..b356a3f 100644 --- a/docs/adr/0005-automatic-hybrid-media-routing.md +++ b/docs/adr/0005-automatic-hybrid-media-routing.md @@ -202,10 +202,13 @@ complete LiveKit tuple. 2. A failed edge performs one ICE restart, one same-parent connection rebuild, and then one alternate eligible peer-parent attempt; it does not repeat an identical action three times. A short `disconnected` state first uses - generation-bound bytes/stats evidence. The target gives initial `connecting` - a visible soft wait followed by a separately measured hard window; that - initial deadline is not implemented yet and must not reuse the existing - three-second recovery constant without mobile-network evidence. When + generation-bound bytes/stats evidence. After the Viewer successfully sends + its answer, an initial `new` or `connecting` connection now has a separate, + conservative 15-second hard deadline. `connected`, disposal, or replacement + by a newer connection generation cancels it; expiry enters the existing + staged recovery at one ICE restart. It does not reuse or change the existing + three-second recovery deadline. The 15-second boundary is unit-tested + but remains subject to mobile-network measurement. When admission has no eligible peer path, or recovery is exhausted, select the SFU root plan without first advertising TURN to every peer edge. A future ordinary coturn transport, if a separate complete change diff --git a/docs/research/advanced-peer-distribution.md b/docs/research/advanced-peer-distribution.md index 6108963..603d820 100644 --- a/docs/research/advanced-peer-distribution.md +++ b/docs/research/advanced-peer-distribution.md @@ -245,7 +245,8 @@ Pinned LiveKit client 2.22.0 uses reconnect delays of 0, 300, 1,200, 2,700, and a four-second state reconciliation that requires three consecutive mismatches before a full reconnect. Jitsi Videobridge similarly defaults to a 15-second first-transfer timeout and an eight-second inactivity limit. These are -reference boundaries, not thresholds copied into Screener. +reference boundaries, not universal prescriptions. Screener's separately owned +15-second initial deadline remains a candidate pending mobile-network evidence. The accepted Screener order is therefore: soft visible wait while initial ICE is still making progress; one ICE restart on hard failure; one same-parent PC diff --git a/docs/status.md b/docs/status.md index 083aa74..c0291ad 100644 --- a/docs/status.md +++ b/docs/status.md @@ -10,8 +10,6 @@ room-scoped Viewer access and the bounded SFU failure-stage diagnostic. Ordinary ICE is STUN-only process-wide; persistent room `1` alone enables the automatic peer/SFU-UDP controller with at most two SFU roots. The old `9610032fc5f5` release, v1 backup, and coturn relay remain rollback-only. -Coherent low-risk milestones may deploy after narrow tests, independent review, -one full gate, CI and rollback preflight; protocol/database changes stay atomic. ## Current Snapshot @@ -20,6 +18,7 @@ one full gate, CI and rollback preflight; protocol/database changes stay atomic. - SQLite v2 keeps Host and nullable Viewer-grant digests in one checked `STRICT` row. The stopped migration retained four rooms and locked each old room private without minting a raw grant. - PR #44's exact-room STUN-only/SFU-UDP router is enabled only for production room `1`: host/SFU roots <=2, browser relay <=1, bounded failure. HTTPS/WSS stays TLS/TCP. - Fallback prewarm is token-free and limited to room `1`; mobile/iPad viewers are leaves and healthy edges stay sticky. +- Viewer initial-connect: after an answer, a generation-bound 15s deadline enters ICE restart; success/replacement/disposal cancels it. Unit-tested, not mobile-verified. - Room `1` can publish exactly `HIGH+LOW` with Dynacast/backup codec off and subscriber `HIGH` ceilings; it has no retained real media frame. - A+B/P2P C remains read-only. Autonomous BWE is only `suspect`; confirmed fallback evacuates children and loses parent capacity. Root impact remains a default-on gate. diff --git a/src/client/webrtc/viewer-peer.ts b/src/client/webrtc/viewer-peer.ts index 1346503..a4d4d7c 100644 --- a/src/client/webrtc/viewer-peer.ts +++ b/src/client/webrtc/viewer-peer.ts @@ -11,6 +11,7 @@ import { const MAX_PENDING_CANDIDATES = 64; const MAX_AUTOMATIC_RECOVERY_REQUESTS = 2; +const INITIAL_CONNECTION_TIMEOUT_MS = 15_000; const AUTOMATIC_RECOVERY_TIMEOUT_MS = 3_000; type SignalCandidate = Extract< SignalPayload, @@ -45,6 +46,7 @@ export class ViewerPeer { private statsTimer: number | null = null; private statsInFlightConnection: RTCPeerConnection | null = null; private disconnectTimer: number | null = null; + private initialConnectionTimer: number | null = null; private recoveryTimer: number | null = null; private offerRecoveryAttempts = 0; private automaticRecoveryRequests = 0; @@ -119,6 +121,7 @@ export class ViewerPeer { throw new Error("Signaling is unavailable while sending the answer"); } this.offerRecoveryAttempts = 0; + this.scheduleInitialConnectionDeadline(connection, connectionId); } else if ( this.connection && this.parentPeerId === parentPeerId && @@ -187,6 +190,7 @@ export class ViewerPeer { false, ); if (sent) { + this.clearInitialConnectionTimer(); this.automaticRecoveryRequests = Math.max( this.automaticRecoveryRequests, 1, @@ -295,6 +299,7 @@ export class ViewerPeer { private handleConnectionState(state: RTCPeerConnectionState): void { if (state === "connected") { this.clearDisconnectTimer(); + this.clearInitialConnectionTimer(); this.resetAutomaticRecovery(); return; } @@ -314,6 +319,7 @@ export class ViewerPeer { } private attemptAutomaticRecovery(): void { + this.clearInitialConnectionTimer(); if ( this.disposed || this.recoveryTimer !== null || @@ -343,6 +349,32 @@ export class ViewerPeer { this.scheduleRecoveryDeadline(); } + private scheduleInitialConnectionDeadline( + connection: RTCPeerConnection, + connectionId: string, + ): void { + if (!this.isCurrentConnection(connection, connectionId)) { + return; + } + this.clearInitialConnectionTimer(); + if (connection.connectionState === "connected") { + return; + } + const timer = window.setTimeout(() => { + if (this.initialConnectionTimer !== timer) { + return; + } + this.initialConnectionTimer = null; + if ( + this.isCurrentConnection(connection, connectionId) && + connection.connectionState !== "connected" + ) { + this.attemptAutomaticRecovery(); + } + }, INITIAL_CONNECTION_TIMEOUT_MS); + this.initialConnectionTimer = timer; + } + private scheduleRecoveryDeadline(): void { this.clearRecoveryTimer(); this.recoveryTimer = window.setTimeout(() => { @@ -502,8 +534,16 @@ export class ViewerPeer { } } + private clearInitialConnectionTimer(): void { + if (this.initialConnectionTimer !== null) { + window.clearTimeout(this.initialConnectionTimer); + this.initialConnectionTimer = null; + } + } + private disposeConnection(): void { this.clearDisconnectTimer(); + this.clearInitialConnectionTimer(); if (this.statsTimer !== null) { window.clearInterval(this.statsTimer); this.statsTimer = null; diff --git a/tests/viewer-peer-generation.test.ts b/tests/viewer-peer-generation.test.ts index 5870e31..a2a1761 100644 --- a/tests/viewer-peer-generation.test.ts +++ b/tests/viewer-peer-generation.test.ts @@ -105,6 +105,7 @@ class FakePeerConnection extends EventTarget { const intervalCallbacks = new Map void>(); let nextIntervalId = 1; const timeoutCallbacks = new Map void>(); +const timeoutDelays = new Map(); let nextTimeoutId = 1; function offer(connectionId: string): SignalPayload { @@ -157,6 +158,7 @@ beforeEach(() => { intervalCallbacks.clear(); nextIntervalId = 1; timeoutCallbacks.clear(); + timeoutDelays.clear(); nextTimeoutId = 1; vi.stubGlobal("MediaStream", FakeMediaStream); vi.stubGlobal("RTCPeerConnection", FakePeerConnection); @@ -167,12 +169,16 @@ beforeEach(() => { return id; }, clearInterval: (id: number) => intervalCallbacks.delete(id), - setTimeout: (callback: () => void) => { + setTimeout: (callback: () => void, delay: number) => { const id = nextTimeoutId++; timeoutCallbacks.set(id, callback); + timeoutDelays.set(id, delay); return id; }, - clearTimeout: (id: number) => timeoutCallbacks.delete(id), + clearTimeout: (id: number) => { + timeoutCallbacks.delete(id); + timeoutDelays.delete(id); + }, }); }); @@ -181,6 +187,105 @@ afterEach(() => { }); describe("ViewerPeer connection generations", () => { + it("restarts ICE when an answered initial connection stays stuck", async () => { + const restartRequests: Array<{ + peerId: string; + connectionId: string; + rebuild: boolean; + }> = []; + const peer = new ViewerPeer( + { iceServers: [] }, + { + sendSignal: () => true, + sendRestartRequest: (peerId, connectionId, rebuild) => { + restartRequests.push({ peerId, connectionId, rebuild }); + return true; + }, + onStream: () => undefined, + onUpdate: () => undefined, + }, + ); + + await peer.acceptSignal("relay-parent", offer("stuck-connection")); + + expect(timeoutDelays.get(1)).toBe(15_000); + timeoutCallbacks.get(1)!(); + expect(restartRequests).toEqual([ + { + peerId: "relay-parent", + connectionId: "stuck-connection", + rebuild: false, + }, + ]); + expect(timeoutDelays.get(2)).toBe(3_000); + }); + + it("cancels the initial deadline after connecting", async () => { + const restartRequests: string[] = []; + const peer = new ViewerPeer( + { iceServers: [] }, + { + sendSignal: () => true, + sendRestartRequest: (_peerId, connectionId) => { + restartRequests.push(connectionId); + return true; + }, + onStream: () => undefined, + onUpdate: () => undefined, + }, + ); + + await peer.acceptSignal("host", offer("connected-generation")); + const initialDeadline = timeoutCallbacks.get(1)!; + const connection = FakePeerConnection.instances[0]!; + connection.connectionState = "connected"; + connection.dispatchEvent(new Event("connectionstatechange")); + + expect(timeoutCallbacks.has(1)).toBe(false); + initialDeadline(); + expect(restartRequests).toEqual([]); + }); + + it("ignores an initial deadline from an old connection generation", async () => { + const restartRequests: string[] = []; + const peer = new ViewerPeer( + { iceServers: [] }, + { + sendSignal: () => true, + sendRestartRequest: (_peerId, connectionId) => { + restartRequests.push(connectionId); + return true; + }, + onStream: () => undefined, + onUpdate: () => undefined, + }, + ); + + await peer.acceptSignal("host", offer("connection-old")); + const oldDeadline = timeoutCallbacks.get(1)!; + await peer.acceptSignal("host", offer("connection-new")); + + expect(timeoutCallbacks.has(1)).toBe(false); + expect(timeoutDelays.get(2)).toBe(15_000); + oldDeadline(); + expect(restartRequests).toEqual([]); + + const currentConnection = FakePeerConnection.instances[1]!; + currentConnection.connectionState = "connected"; + currentConnection.dispatchEvent(new Event("connectionstatechange")); + expect(timeoutCallbacks.has(2)).toBe(false); + }); + + it("clears the initial deadline when disposed", async () => { + const peer = createPeer([], []); + await peer.acceptSignal("host", offer("disposed-connection")); + + peer.dispose(); + + expect(timeoutCallbacks).toEqual(new Map()); + expect(timeoutDelays).toEqual(new Map()); + }); + it("reports one exhausted edge after bounded restart and rebuild attempts", async () => { const restartRequests: Array<{ peerId: string; @@ -216,13 +321,13 @@ describe("ViewerPeer connection generations", () => { rebuild: false, }, ]); - timeoutCallbacks.get(1)!(); + timeoutCallbacks.get(2)!(); expect(restartRequests.at(-1)).toEqual({ peerId: "relay-parent", connectionId: "failed-connection", rebuild: true, }); - timeoutCallbacks.get(2)!(); + timeoutCallbacks.get(3)!(); expect(exhausted).toEqual([ { peerId: "relay-parent", connectionId: "failed-connection" }, ]); @@ -253,12 +358,12 @@ describe("ViewerPeer connection generations", () => { connection.connectionState = "failed"; connection.dispatchEvent(new Event("connectionstatechange")); - timeoutCallbacks.get(1)!(); timeoutCallbacks.get(2)!(); + timeoutCallbacks.get(3)!(); expect(exhausted).toEqual(["failed-connection"]); reportAvailable = true; - timeoutCallbacks.get(3)!(); + timeoutCallbacks.get(4)!(); expect(exhausted).toEqual(["failed-connection", "failed-connection"]); connection.dispatchEvent(new Event("connectionstatechange")); @@ -328,6 +433,7 @@ describe("ViewerPeer connection generations", () => { rebuild: true, }, ]); + expect([...timeoutDelays.values()]).toEqual([3_000]); }); it("targets answers and recovery requests at the current parent", async () => {