Prevent ongoing renegotiations from declaring the negotiation as timed out (#1813)

Author: lukasIO

.changeset/great-crabs-yawn.md

@@ -0,0 +1,5 @@
+---
+"livekit-client": patch
+---
+
+Prevent ongoing renegotiations from declaring the negotiation as timed out

src/room/PCTransport.ts

@@ -278,6 +278,7 @@ export default class PCTransport extends EventEmitter {
           await this._pc.setRemoteDescription(currentSD);
         } else {
           this.renegotiate = true;
+          this.log.debug('requesting renegotiation', { ...this.logContext });
           return;
         }
       } else if (!this._pc || this._pc.signalingState === 'closed') {

src/room/PCTransportManager.ts

@@ -229,28 +229,46 @@ export class PCTransportManager {
 
   async negotiate(abortController: AbortController) {
     return new TypedPromise<void, NegotiationError | Error>(async (resolve, reject) => {
-      const negotiationTimeout = setTimeout(() => {
+      let negotiationTimeout = setTimeout(() => {
         reject(new NegotiationError('negotiation timed out'));
       }, this.peerConnectionTimeout);
 
-      const abortHandler = () => {
+      const cleanup = () => {
         clearTimeout(negotiationTimeout);
+        this.publisher.off(PCEvents.NegotiationStarted, onNegotiationStarted);
+        abortController.signal.removeEventListener('abort', abortHandler);
+      };
+
+      const abortHandler = () => {
+        cleanup();
         reject(new NegotiationError('negotiation aborted'));
       };
 
-      abortController.signal.addEventListener('abort', abortHandler);
-      this.publisher.once(PCEvents.NegotiationStarted, () => {
+      // Reset the timeout each time a renegotiation cycle starts. This
+      // prevents premature timeouts when the negotiation machinery is
+      // actively renegotiating (offers going out, answers coming back) but
+      // NegotiationComplete hasn't fired yet because new requirements keep
+      // arriving between offer/answer round-trips.
+      const onNegotiationStarted = () => {
         if (abortController.signal.aborted) {
           return;
         }
-        this.publisher.once(PCEvents.NegotiationComplete, () => {
-          clearTimeout(negotiationTimeout);
-          resolve();
-        });
+        clearTimeout(negotiationTimeout);
+        negotiationTimeout = setTimeout(() => {
+          cleanup();
+          reject(new NegotiationError('negotiation timed out'));
+        }, this.peerConnectionTimeout);
+      };
+
+      abortController.signal.addEventListener('abort', abortHandler);
+      this.publisher.on(PCEvents.NegotiationStarted, onNegotiationStarted);
+      this.publisher.once(PCEvents.NegotiationComplete, () => {
+        cleanup();
+        resolve();
       });
 
       await this.publisher.negotiate((e) => {
-        clearTimeout(negotiationTimeout);
+        cleanup();
         if (e instanceof Error) {
           reject(e);
         } else {

src/room/RTCEngine.ts

@@ -1524,6 +1524,7 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
         reject(new NegotiationError('cannot negotiate on closed engine'));
       }
       this.on(EngineEvent.Closing, handleClosed);
+      this.on(EngineEvent.Restarting, handleClosed);
 
       this.pcManager.publisher.once(
         PCEvents.RTPVideoPayloadTypes,
@@ -1543,6 +1544,12 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
         await this.pcManager.negotiate(abortController);
         resolve();
       } catch (e: unknown) {
+        if (abortController.signal.aborted) {
+          // negotiation was aborted due to engine close or restart, resolve
+          // cleanly to avoid triggering a cascading reconnect loop
+          resolve();
+          return;
+        }
         if (e instanceof NegotiationError) {
           this.fullReconnectOnNext = true;
         }
@@ -1554,6 +1561,7 @@ export default class RTCEngine extends (EventEmitter as new () => TypedEventEmit
         }
       } finally {
         this.off(EngineEvent.Closing, handleClosed);
+        this.off(EngineEvent.Restarting, handleClosed);
       }
     });
   }