fix: Fix users unable to see or hear each other in voice channels due to

stale server sockets, passive non-initiators, and race conditions during peer connection setup. Fix users unable to see or hear each other in voice channels due to stale server sockets, passive non-initiators, and race conditions during peer connection setup. Server: - Close stale WebSocket connections sharing the same oderId in handleIdentify instead of letting them linger up to 45s - Make user_joined/user_left broadcasts identity-aware so duplicate sockets don't produce phantom join/leave events - Include serverIds in user_left payload for multi-room presence - Simplify findUserByOderId now that stale sockets are cleaned up Client - signaling: - Add fallback offer system with 1s timer for missed user_joined races - Add non-initiator takeover after 5s when the initiator fails to send an offer (NON_INITIATOR_GIVE_UP_MS) - Scope peerServerMap per signaling URL to prevent cross-server collisions - Add socket identity guards on all signaling event handlers - Replace canReusePeerConnection with hasActivePeerConnection and isPeerConnectionNegotiating with extended grace periods Client - peer connections: - Extract replaceUnusablePeer helper to deduplicate stale peer replacement in offer and ICE handlers - Add stale connectionstatechange guard to ignore events from replaced RTCPeerConnection instances - Use deterministic initiator election in peer recovery reconnects - Track createdAt on PeerData for staleness detection Client - presence: - Add multi-room presence tracking via presenceServerIds on User - Replace clearUsers + individual userJoined with syncServerPresence for atomic server roster updates - Make userLeft handle partial server removal instead of full eviction Documentation: - Add server-side connection hygiene, non-initiator takeover, and stale peer replacement sections to the realtime README
2026-04-04 02:47:58 +02:00
parent ae0ee8fac7
commit de2d3300d4
24 changed files with 1128 additions and 164 deletions
--- a/toju-app/src/app/infrastructure/realtime/signaling/signaling.manager.ts
+++ b/toju-app/src/app/infrastructure/realtime/signaling/signaling.manager.ts
@@ -3,7 +3,11 @@
 * Manages the WebSocket connection to the signaling server,
 * including automatic reconnection and heartbeats.
 */
-import { Observable, Subject } from 'rxjs';
+import {
+  Observable,
+  Subject,
+  of
+} from 'rxjs';
 import type { SignalingMessage } from '../../../shared-kernel';
 import { recordDebugNetworkSignalingPayload } from '../logging/debug-network-metrics';
 import { IdentifyCredentials, JoinedServerInfo } from '../realtime.types';
@@ -54,19 +58,42 @@ export class SignalingManager {

  /** Open (or re-open) a WebSocket to the signaling server. */
  connect(serverUrl: string): Observable<boolean> {
+    if (this.lastSignalingUrl === serverUrl) {
+      if (this.isSocketOpen()) {
+        return of(true);
+      }
+
+      if (this.isSocketConnecting()) {
+        return this.waitForOpen();
+      }
+    }
+
    this.lastSignalingUrl = serverUrl;
    return new Observable<boolean>((observer) => {
      try {
        this.logger.info('[signaling] Connecting to signaling server', { serverUrl });

-        if (this.signalingWebSocket) {
-          this.signalingWebSocket.close();
-        }
+        const previousSocket = this.signalingWebSocket;

        this.lastSignalingUrl = serverUrl;
-        this.signalingWebSocket = new WebSocket(serverUrl);
+        const socket = new WebSocket(serverUrl);
+
+        this.signalingWebSocket = socket;
+
+        if (previousSocket && previousSocket !== socket) {
+          try {
+            previousSocket.close();
+          } catch {
+            this.logger.warn('[signaling] Failed to close previous signaling socket', {
+              url: serverUrl
+            });
+          }
+        }
+
+        socket.onopen = () => {
+          if (socket !== this.signalingWebSocket)
+            return;

-        this.signalingWebSocket.onopen = () => {
          this.logger.info('[signaling] Connected to signaling server', {
            serverUrl,
            readyState: this.getSocketReadyStateLabel()
@@ -77,9 +104,13 @@ export class SignalingManager {
          this.connectionStatus$.next({ connected: true });
          this.reIdentifyAndRejoin();
          observer.next(true);
+          observer.complete();
        };

-        this.signalingWebSocket.onmessage = (event) => {
+        socket.onmessage = (event) => {
+          if (socket !== this.signalingWebSocket)
+            return;
+
          const rawPayload = this.stringifySocketPayload(event.data);
          const payloadBytes = rawPayload ? this.measurePayloadBytes(rawPayload) : null;

@@ -109,7 +140,10 @@ export class SignalingManager {
          }
        };

-        this.signalingWebSocket.onerror = (error) => {
+        socket.onerror = (error) => {
+          if (socket !== this.signalingWebSocket)
+            return;
+
          this.logger.error('[signaling] Signaling socket error', error, {
            readyState: this.getSocketReadyStateLabel(),
            url: serverUrl
@@ -121,7 +155,10 @@ export class SignalingManager {
          observer.error(error);
        };

-        this.signalingWebSocket.onclose = (event) => {
+        socket.onclose = (event) => {
+          if (socket !== this.signalingWebSocket)
+            return;
+
          this.logger.warn('[signaling] Disconnected from signaling server', {
            attempts: this.signalingReconnectAttempts,
            code: event.code,
@@ -216,9 +253,12 @@ export class SignalingManager {
    this.stopHeartbeat();
    this.clearReconnect();

-    if (this.signalingWebSocket) {
-      this.signalingWebSocket.close();
-      this.signalingWebSocket = null;
+    const socket = this.signalingWebSocket;
+
+    this.signalingWebSocket = null;
+
+    if (socket) {
+      socket.close();
    }
  }

@@ -227,6 +267,10 @@ export class SignalingManager {
    return this.signalingWebSocket !== null && this.signalingWebSocket.readyState === WebSocket.OPEN;
  }

+  isSocketConnecting(): boolean {
+    return this.signalingWebSocket !== null && this.signalingWebSocket.readyState === WebSocket.CONNECTING;
+  }
+
  /** The URL last used to connect (needed for reconnection). */
  getLastUrl(): string | null {
    return this.lastSignalingUrl;
@@ -273,7 +317,7 @@ export class SignalingManager {
   * No-ops if a timer is already pending or no URL is stored.
   */
  private scheduleReconnect(): void {
-    if (this.signalingReconnectTimer || !this.lastSignalingUrl)
+    if (this.signalingReconnectTimer || !this.lastSignalingUrl || this.isSocketOpen() || this.isSocketConnecting())
      return;

    const delay = Math.min(
@@ -283,6 +327,11 @@ export class SignalingManager {

    this.signalingReconnectTimer = setTimeout(() => {
      this.signalingReconnectTimer = null;
+
+      if (this.isSocketOpen() || this.isSocketConnecting()) {
+        return;
+      }
+
      this.signalingReconnectAttempts++;
      this.logger.info('[signaling] Attempting reconnect', {
        attempt: this.signalingReconnectAttempts,
@@ -297,6 +346,44 @@ export class SignalingManager {
    }, delay);
  }

+  private waitForOpen(timeoutMs: number = SIGNALING_CONNECT_TIMEOUT_MS): Observable<boolean> {
+    if (this.isSocketOpen()) {
+      return of(true);
+    }
+
+    return new Observable<boolean>((observer) => {
+      let settled = false;
+
+      const subscription = this.connectionStatus$.subscribe(({ connected }) => {
+        if (!connected || settled) {
+          return;
+        }
+
+        settled = true;
+        clearTimeout(timeout);
+        subscription.unsubscribe();
+        observer.next(true);
+        observer.complete();
+      });
+      const timeout = setTimeout(() => {
+        if (settled) {
+          return;
+        }
+
+        settled = true;
+        subscription.unsubscribe();
+        observer.next(this.isSocketOpen());
+        observer.complete();
+      }, timeoutMs);
+
+      return () => {
+        settled = true;
+        clearTimeout(timeout);
+        subscription.unsubscribe();
+      };
+    });
+  }
+
  /** Cancel any pending reconnect timer and reset the attempt counter. */
  private clearReconnect(): void {
    if (this.signalingReconnectTimer) {
@@ -415,21 +502,23 @@ export class SignalingManager {
    const record = payload as Record<string, unknown>;
    const voiceState = this.summarizeVoiceState(record['voiceState']);
    const users = this.summarizeUsers(record['users']);
-
-    return {
-      displayName: typeof record['displayName'] === 'string' ? record['displayName'] : undefined,
-      fromUserId: typeof record['fromUserId'] === 'string' ? record['fromUserId'] : undefined,
-      isScreenSharing: typeof record['isScreenSharing'] === 'boolean' ? record['isScreenSharing'] : undefined,
+    const preview: Record<string, unknown> = {
      keys: Object.keys(record).slice(0, 10),
-      oderId: typeof record['oderId'] === 'string' ? record['oderId'] : undefined,
-      roomId: typeof record['serverId'] === 'string' ? record['serverId'] : undefined,
-      serverId: typeof record['serverId'] === 'string' ? record['serverId'] : undefined,
-      targetPeerId: typeof record['targetUserId'] === 'string' ? record['targetUserId'] : undefined,
-      type: typeof record['type'] === 'string' ? record['type'] : 'unknown',
-      userCount: Array.isArray(record['users']) ? record['users'].length : undefined,
-      users,
-      voiceState
+      type: typeof record['type'] === 'string' ? record['type'] : 'unknown'
    };
+
+    this.assignPreviewValue(preview, 'displayName', typeof record['displayName'] === 'string' ? record['displayName'] : undefined);
+    this.assignPreviewValue(preview, 'fromUserId', typeof record['fromUserId'] === 'string' ? record['fromUserId'] : undefined);
+    this.assignPreviewValue(preview, 'isScreenSharing', typeof record['isScreenSharing'] === 'boolean' ? record['isScreenSharing'] : undefined);
+    this.assignPreviewValue(preview, 'oderId', typeof record['oderId'] === 'string' ? record['oderId'] : undefined);
+    this.assignPreviewValue(preview, 'roomId', typeof record['roomId'] === 'string' ? record['roomId'] : undefined);
+    this.assignPreviewValue(preview, 'serverId', typeof record['serverId'] === 'string' ? record['serverId'] : undefined);
+    this.assignPreviewValue(preview, 'targetPeerId', typeof record['targetUserId'] === 'string' ? record['targetUserId'] : undefined);
+    this.assignPreviewValue(preview, 'userCount', Array.isArray(record['users']) ? record['users'].length : undefined);
+    this.assignPreviewValue(preview, 'users', users);
+    this.assignPreviewValue(preview, 'voiceState', voiceState);
+
+    return preview;
  }

  private summarizeVoiceState(value: unknown): Record<string, unknown> | undefined {
@@ -438,15 +527,18 @@ export class SignalingManager {
    if (!voiceState)
      return undefined;

-    return {
+    const summary: Record<string, unknown> = {
      isConnected: voiceState['isConnected'] === true,
      isMuted: voiceState['isMuted'] === true,
      isDeafened: voiceState['isDeafened'] === true,
-      isSpeaking: voiceState['isSpeaking'] === true,
-      roomId: typeof voiceState['roomId'] === 'string' ? voiceState['roomId'] : undefined,
-      serverId: typeof voiceState['serverId'] === 'string' ? voiceState['serverId'] : undefined,
-      volume: typeof voiceState['volume'] === 'number' ? voiceState['volume'] : undefined
+      isSpeaking: voiceState['isSpeaking'] === true
    };
+
+    this.assignPreviewValue(summary, 'roomId', typeof voiceState['roomId'] === 'string' ? voiceState['roomId'] : undefined);
+    this.assignPreviewValue(summary, 'serverId', typeof voiceState['serverId'] === 'string' ? voiceState['serverId'] : undefined);
+    this.assignPreviewValue(summary, 'volume', typeof voiceState['volume'] === 'number' ? voiceState['volume'] : undefined);
+
+    return summary;
  }

  private summarizeUsers(value: unknown): Record<string, unknown>[] | undefined {
@@ -461,15 +553,22 @@ export class SignalingManager {
      if (!user)
        continue;

-      users.push({
-        displayName: typeof user['displayName'] === 'string' ? user['displayName'] : undefined,
-        oderId: typeof user['oderId'] === 'string' ? user['oderId'] : undefined
-      });
+      const summary: Record<string, unknown> = {};
+
+      this.assignPreviewValue(summary, 'displayName', typeof user['displayName'] === 'string' ? user['displayName'] : undefined);
+      this.assignPreviewValue(summary, 'oderId', typeof user['oderId'] === 'string' ? user['oderId'] : undefined);
+
+      users.push(summary);
    }

    return users;
  }

+  private assignPreviewValue(target: Record<string, unknown>, key: string, value: unknown): void {
+    if (value !== undefined)
+      target[key] = value;
+  }
+
  private asRecord(value: unknown): Record<string, unknown> | null {
    if (!value || typeof value !== 'object' || Array.isArray(value))
      return null;