Merge pull request #1634 from matrix-org/dbkr/check_turn_interval

Check TURN servers periodically, and at start of calls

spec/unit/webrtc/call.spec.ts

@@ -79,6 +79,7 @@ class MockRTCPeerConnection {
         return Promise.resolve();
     }
     close() {}
+    getStats() { return []; }
 }
 
 describe('Call', function() {
@@ -122,6 +123,7 @@ describe('Call', function() {
         // We just stub out sendEvent: we're not interested in testing the client's
         // event sending code here
         client.client.sendEvent = () => {};
+        client.httpBackend.when("GET", "/voip/turnServer").respond(200, {});
         call = new MatrixCall({
             client: client.client,
             roomId: '!foo:bar',
@@ -138,7 +140,9 @@ describe('Call', function() {
     });
 
     it('should ignore candidate events from non-matching party ID', async function() {
-        await call.placeVoiceCall();
+        const callPromise = call.placeVoiceCall();
+        await client.httpBackend.flush();
+        await callPromise;
         await call.onAnswerReceived({
             getContent: () => {
                 return {
@@ -192,7 +196,9 @@ describe('Call', function() {
     });
 
     it('should add candidates received before answer if party ID is correct', async function() {
-        await call.placeVoiceCall();
+        const callPromise = call.placeVoiceCall();
+        await client.httpBackend.flush();
+        await callPromise;
         call.peerConn.addIceCandidate = jest.fn();
 
         call.onRemoteIceCandidatesReceived({

src/client.js

@@ -61,6 +61,7 @@ import {DEHYDRATION_ALGORITHM} from "./crypto/dehydration";
 const SCROLLBACK_DELAY_MS = 3000;
 export const CRYPTO_ENABLED = isCryptoAvailable();
 const CAPABILITIES_CACHE_MS = 21600000; // 6 hours - an arbitrary value
+const TURN_CHECK_INTERVAL = 10 * 60 * 1000; // poll for turn credentials every 10 minutes
 
 function keysFromRecoverySession(sessions, decryptionKey, roomId) {
     const keys = [];
@@ -394,7 +395,8 @@ export function MatrixClient(opts) {
     this._clientWellKnownPromise = undefined;
 
     this._turnServers = [];
-    this._turnServersExpiry = null;
+    this._turnServersExpiry = 0;
+    this._checkTurnServersIntervalID = null;
 
     // The SDK doesn't really provide a clean way for events to recalculate the push
     // actions for themselves, so we have to kinda help them out when they are encrypted.
@@ -4955,6 +4957,48 @@ MatrixClient.prototype.getTurnServersExpiry = function() {
     return this._turnServersExpiry;
 };
 
+MatrixClient.prototype._checkTurnServers = async function() {
+    if (!this._supportsVoip) {
+        return;
+    }
+
+    let credentialsGood = false;
+    const remainingTime = this._turnServersExpiry - Date.now();
+    if (remainingTime > TURN_CHECK_INTERVAL) {
+        logger.debug("TURN creds are valid for another " + remainingTime + " ms: not fetching new ones.");
+        credentialsGood = true;
+    } else {
+        logger.debug("Fetching new TURN credentials");
+        try {
+            const res = await this.turnServer();
+            if (res.uris) {
+                logger.log("Got TURN URIs: " + res.uris + " refresh in " + res.ttl + " secs");
+                // map the response to a format that can be fed to RTCPeerConnection
+                const servers = {
+                    urls: res.uris,
+                    username: res.username,
+                    credential: res.password,
+                };
+                this._turnServers = [servers];
+                // The TTL is in seconds but we work in ms
+                this._turnServersExpiry = Date.now() + (res.ttl * 1000);
+                credentialsGood = true;
+            }
+        } catch (err) {
+            logger.error("Failed to get TURN URIs", err);
+            // If we get a 403, there's no point in looping forever.
+            if (err.httpStatus === 403) {
+                logger.info("TURN access unavailable for this account: stopping credentials checks");
+                if (this._checkTurnServersIntervalID !== null) global.clearInterval(this._checkTurnServersIntervalID);
+                this._checkTurnServersIntervalID = null;
+            }
+        }
+        // otherwise, if we failed for whatever reason, try again the next time we're called.
+    }
+
+    return credentialsGood;
+};
+
 /**
  * Set whether to allow a fallback ICE server should be used for negotiating a
  * WebRTC connection if the homeserver doesn't provide any servers. Defaults to
@@ -5107,7 +5151,12 @@ MatrixClient.prototype.startClient = async function(opts) {
     }
 
     // periodically poll for turn servers if we support voip
-    checkTurnServers(this);
+    if (this._supportsVoip) {
+        this._checkTurnServersIntervalID = setInterval(() => {
+            this._checkTurnServers();
+        }, TURN_CHECK_INTERVAL);
+        this._checkTurnServers();
+    }
 
     if (this._syncApi) {
         // This shouldn't happen since we thought the client was not running
@@ -5219,7 +5268,7 @@ MatrixClient.prototype.stopClient = function() {
         this._callEventHandler = null;
     }
 
-    global.clearTimeout(this._checkTurnServersTimeoutID);
+    global.clearInterval(this._checkTurnServersIntervalID);
     if (this._clientWellKnownIntervalID !== undefined) {
         global.clearInterval(this._clientWellKnownIntervalID);
     }
@@ -5436,42 +5485,6 @@ async function(roomId, eventId, relationType, eventType, opts = {}) {
     };
 };
 
-function checkTurnServers(client) {
-    if (!client._supportsVoip) {
-        return;
-    }
-
-    client.turnServer().then(function(res) {
-        if (res.uris) {
-            logger.log("Got TURN URIs: " + res.uris + " refresh in " +
-                res.ttl + " secs");
-            // map the response to a format that can be fed to
-            // RTCPeerConnection
-            const servers = {
-                urls: res.uris,
-                username: res.username,
-                credential: res.password,
-            };
-            client._turnServers = [servers];
-            client._turnServersExpiry = Date.now() + res.ttl;
-            // re-fetch when we're about to reach the TTL
-            client._checkTurnServersTimeoutID = setTimeout(() => {
-                checkTurnServers(client);
-            }, (res.ttl || (60 * 60)) * 1000 * 0.9);
-        }
-    }, function(err) {
-        logger.error("Failed to get TURN URIs");
-        // If we get a 403, there's no point in looping forever.
-        if (err.httpStatus === 403) {
-            logger.info("TURN access unavailable for this account");
-            return;
-        }
-        client._checkTurnServersTimeoutID = setTimeout(function() {
-            checkTurnServers(client);
-        }, 60000);
-    });
-}
-
 function _reject(callback, reject, err) {
     if (callback) {
         callback(err);

src/webrtc/call.ts

@@ -333,11 +333,11 @@ export class MatrixCall extends EventEmitter {
      * Place a voice call to this room.
      * @throws If you have not specified a listener for 'error' events.
      */
-    placeVoiceCall() {
+    async placeVoiceCall() {
         logger.debug("placeVoiceCall");
         this.checkForErrorListener();
         const constraints = getUserMediaContraints(ConstraintsType.Audio);
-        this.placeCallWithConstraints(constraints);
+        await this.placeCallWithConstraints(constraints);
         this.type = CallType.Voice;
     }
 
@@ -349,13 +349,13 @@ export class MatrixCall extends EventEmitter {
      * to render the local camera preview.
      * @throws If you have not specified a listener for 'error' events.
      */
-    placeVideoCall(remoteVideoElement: HTMLVideoElement, localVideoElement: HTMLVideoElement) {
+    async placeVideoCall(remoteVideoElement: HTMLVideoElement, localVideoElement: HTMLVideoElement) {
         logger.debug("placeVideoCall");
         this.checkForErrorListener();
         this.localVideoElement = localVideoElement;
         this.remoteVideoElement = remoteVideoElement;
         const constraints = getUserMediaContraints(ConstraintsType.Video);
-        this.placeCallWithConstraints(constraints);
+        await this.placeCallWithConstraints(constraints);
         this.type = CallType.Video;
     }
 
@@ -527,6 +527,13 @@ export class MatrixCall extends EventEmitter {
         const invite = event.getContent();
         this.direction = CallDirection.Inbound;
 
+        // make sure we have valid turn creds. Unless something's gone wrong, it should
+        // poll and keep the credentials valid so this should be instant.
+        const haveTurnCreds = await this.client._checkTurnServers();
+        if (!haveTurnCreds) {
+            logger.warn("Failed to get TURN credentials! Proceeding with call anyway...");
+        }
+
         this.peerConn = this.createPeerConnection();
         // we must set the party ID before await-ing on anything: the call event
         // handler will start giving us more call events (eg. candidates) so if
@@ -857,7 +864,6 @@ export class MatrixCall extends EventEmitter {
 
         // why do we enable audio (and only audio) tracks here? -- matthew
         setTracksEnabled(stream.getAudioTracks(), true);
-        this.peerConn = this.createPeerConnection();
 
         for (const audioTrack of stream.getAudioTracks()) {
             logger.info("Adding audio track with id " + audioTrack.id);
@@ -1662,11 +1668,18 @@ export class MatrixCall extends EventEmitter {
         this.setState(CallState.WaitLocalMedia);
         this.direction = CallDirection.Outbound;
         this.config = constraints;
-        // It would be really nice if we could start gathering candidates at this point
+
-        // so the ICE agent could be gathering while we open our media devices: we already
+        // make sure we have valid turn creds. Unless something's gone wrong, it should
-        // know the type of the call and therefore what tracks we want to send.
+        // poll and keep the credentials valid so this should be instant.
-        // Perhaps we could do this by making fake tracks now and then using replaceTrack()
+        const haveTurnCreds = await this.client._checkTurnServers();
-        // once we have the actual tracks? (Can we make fake tracks?)
+        if (!haveTurnCreds) {
+            logger.warn("Failed to get TURN credentials! Proceeding with call anyway...");
+        }
+
+        // create the peer connection now so it can be gathering candidates while we get user
+        // media (assuming a candidate pool size is configured)
+        this.peerConn = this.createPeerConnection();
+
         try {
             const mediaStream = await navigator.mediaDevices.getUserMedia(constraints);
             this.gotUserMediaForInvite(mediaStream);

src/webrtc/callEventHandler.ts

@@ -139,7 +139,7 @@ export class CallEventHandler {
             }
 
             const timeUntilTurnCresExpire = this.client.getTurnServersExpiry() - Date.now();
-            logger.info("Current turn creds expire in " + timeUntilTurnCresExpire + " seconds");
+            logger.info("Current turn creds expire in " + timeUntilTurnCresExpire + " ms");
             call = createNewMatrixCall(this.client, event.getRoomId(), {
                 forceTURN: this.client._forceTURN,
             });