Fix data channel close race condition (#1978)

Author: 1egoman

.changeset/gold-candies-switch.md

@@ -0,0 +1,5 @@
+---
+'livekit-client': patch
+---
+
+Fix data channel close race condition

src/room/RTCEngine.ts

@@ -456,18 +456,25 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
   }
 
   async cleanupPeerConnections() {
-    await this.pcManager?.close();
-    this.pcManager = undefined;
-
     const dcCleanup = (dc: RTCDataChannel | undefined) => {
-      if (!dc) return;
-      dc.close();
+      if (!dc) {
+        return;
+      }
+
+      // Detach the data channel handlers before closing anything. Closing a peer connection tears
+      // down the SCTP transport, which can dispatch `error`/`close` events on the still-open data
+      // channels; if our handlers are still attached at that point, handleDataError logs a spurious
+      // "Unknown DataChannel error" during an otherwise graceful disconnect. Removing the handlers
+      // before dc.close()/pcManager.close() makes this deterministic regardless of how/when the
+      // browser dispatches those teardown events. See livekit/client-sdk-js#1953.
       dc.onbufferedamountlow = null;
       dc.onclose = null;
       dc.onclosing = null;
       dc.onerror = null;
       dc.onmessage = null;
       dc.onopen = null;
+
+      dc.close();
     };
     dcCleanup(this.lossyDC);
     dcCleanup(this.lossyDCSub);
@@ -476,6 +483,9 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
     dcCleanup(this.dataTrackDC);
     dcCleanup(this.dataTrackDCSub);
 
+    await this.pcManager?.close();
+    this.pcManager = undefined;
+
     this.lossyDC = undefined;
     this.lossyDCSub = undefined;
     this.reliableDC = undefined;
@@ -1038,12 +1048,24 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
   };
 
   private handleDataError = (event: Event) => {
+    // Errors fired while we're tearing the connection down (e.g. the SCTP transport aborting as
+    // the peer connection closes) carry no actionable information — the channel is going away
+    // regardless. Suppress them so a graceful disconnect doesn't surface spurious errors.
+    // See livekit/client-sdk-js#1953.
+    if (this._isClosed) {
+      return;
+    }
+
     const channel = event.currentTarget as RTCDataChannel;
     const channelKind = channel.maxRetransmits === 0 ? 'lossy' : 'reliable';
 
-    if (event instanceof ErrorEvent && event.error) {
-      const { error } = event.error;
-      this.log.error(`DataChannel error on ${channelKind}: ${event.message}`, { error });
+    if (typeof RTCErrorEvent !== 'undefined' && event instanceof RTCErrorEvent && event.error) {
+      const { error } = event;
+      this.log.error(`DataChannel error on ${channelKind}: ${error.message}`, {
+        error,
+        errorDetail: error.errorDetail,
+        sctpCauseCode: error.sctpCauseCode,
+      });
     } else {
       this.log.error(`Unknown DataChannel error on ${channelKind}`, { event });
     }