Merge pull request twilio#156 from twilio/bugfix/github_issues

Merge branch: Bugfix/GitHub issues

CHANGELOG.md

@@ -1,3 +1,24 @@
+2.4.0 (In Progress)
+===================
+
+Changes
+-------
+
+- Updated the description of [Device.updateToken](https://twilio.github.io/twilio-voice.js/classes/voice.device.html#updatetoken) API. It is recommended to call this API after [Device.tokenWillExpireEvent](https://twilio.github.io/twilio-voice.js/classes/voice.device.html#tokenwillexpireevent) is emitted, and before or after a call to prevent a potential ~1s audio loss during the update process.
+
+- Updated stats reporting to stop using deprecated `RTCIceCandidateStats` - `ip` and `deleted`.
+
+Bug Fixes
+---------
+
+- Fixed an [issue](https://github.com/twilio/twilio-voice.js/issues/100) where a `TypeError` is thrown after rejecting a call then invoking `updateToken`.
+
+- Fixed an issue (https://github.com/twilio/twilio-voice.js/issues/87, https://github.com/twilio/twilio-voice.js/issues/145) where the `PeerConnection` object is not properly disposed.
+
+- Fixed an [issue](https://github.com/twilio/twilio-voice.js/issues/14) where `device.audio.disconnect`, `device.audio.incoming` and `device.audio.outgoing` do not have the correct type definitions.
+
+- Fixed an [issue](https://github.com/twilio/twilio-voice.js/issues/126) where the internal `deviceinfochange` event is being emitted indefinitely, causing high cpu usage.
+
 2.3.2 (February 27, 2023)
 ===================
 

lib/twilio/audiohelper.ts

@@ -7,7 +7,7 @@ import Device from './device';
 import { InvalidArgumentError, NotSupportedError } from './errors';
 import Log from './log';
 import OutputDeviceCollection from './outputdevicecollection';
-import * as defaultMediaDevices from './shims/mediadevices';
+import * as getMediaDevicesInstance from './shims/mediadevices';
 import { average, difference, isFirefox } from './util';
 
 const MediaDeviceInfoShim = require('./shims/mediadeviceinfo');
@@ -90,6 +90,15 @@ class AudioHelper extends EventEmitter {
    */
   private _audioContext?: AudioContext;
 
+  /**
+   * Whether each sound is enabled.
+   */
+  private _enabledSounds: Record<Device.ToggleableSound, boolean> = {
+    [Device.SoundName.Disconnect]: true,
+    [Device.SoundName.Incoming]: true,
+    [Device.SoundName.Outgoing]: true,
+  };
+
   /**
    * The `getUserMedia()` function to use.
    */
@@ -163,7 +172,7 @@ class AudioHelper extends EventEmitter {
     }, options);
 
     this._getUserMedia = getUserMedia;
-    this._mediaDevices = options.mediaDevices || defaultMediaDevices;
+    this._mediaDevices = options.mediaDevices || getMediaDevicesInstance();
     this._onActiveInputChanged = onActiveInputChanged;
 
     const isAudioContextSupported: boolean = !!(options.AudioContext || options.audioContext);
@@ -172,10 +181,6 @@ class AudioHelper extends EventEmitter {
     this.isOutputSelectionSupported = isEnumerationSupported && isSetSinkSupported;
     this.isVolumeSupported = isAudioContextSupported;
 
-    if (options.enabledSounds) {
-      this._addEnabledSounds(options.enabledSounds);
-    }
-
     if (this.isVolumeSupported) {
       this._audioContext = options.audioContext || options.AudioContext && new options.AudioContext();
       if (this._audioContext) {
@@ -287,6 +292,36 @@ class AudioHelper extends EventEmitter {
     }
   }
 
+  /**
+   * Enable or disable the disconnect sound.
+   * @param doEnable Passing `true` will enable the sound and `false` will disable the sound.
+   * Not passing this parameter will not alter the enable-status of the sound.
+   * @returns The enable-status of the sound.
+   */
+  disconnect(doEnable?: boolean): boolean {
+    return this._maybeEnableSound(Device.SoundName.Disconnect, doEnable);
+  }
+
+  /**
+   * Enable or disable the incoming sound.
+   * @param doEnable Passing `true` will enable the sound and `false` will disable the sound.
+   * Not passing this parameter will not alter the enable-status of the sound.
+   * @returns The enable-status of the sound.
+   */
+  incoming(doEnable?: boolean): boolean {
+    return this._maybeEnableSound(Device.SoundName.Incoming, doEnable);
+  }
+
+  /**
+   * Enable or disable the outgoing sound.
+   * @param doEnable Passing `true` will enable the sound and `false` will disable the sound.
+   * Not passing this parameter will not alter the enable-status of the sound.
+   * @returns The enable-status of the sound.
+   */
+  outgoing(doEnable?: boolean): boolean {
+    return this._maybeEnableSound(Device.SoundName.Outgoing, doEnable);
+  }
+
   /**
    * Set the MediaTrackConstraints to be applied on every getUserMedia call for new input
    * device audio. Any deviceId specified here will be ignored. Instead, device IDs should
@@ -343,27 +378,6 @@ class AudioHelper extends EventEmitter {
     });
   }
 
-  /**
-   * Merge the passed enabledSounds into {@link AudioHelper}. Currently used to merge the deprecated
-   *   Device.sounds object onto the new {@link AudioHelper} interface. Mutates
-   *   by reference, sharing state between {@link Device} and {@link AudioHelper}.
-   * @param enabledSounds - The initial sound settings to merge.
-   * @private
-   */
-  private _addEnabledSounds(enabledSounds: { [name: string]: boolean }) {
-    function setValue(key: Device.ToggleableSound, value: boolean) {
-      if (typeof value !== 'undefined') {
-        enabledSounds[key] = value;
-      }
-
-      return enabledSounds[key];
-    }
-
-    Object.keys(enabledSounds).forEach(key => {
-      (this as any)[key] = setValue.bind(null, key);
-    });
-  }
-
   /**
    * Get the index of an un-labeled Device.
    * @param mediaDeviceInfo
@@ -407,6 +421,19 @@ class AudioHelper extends EventEmitter {
     });
   }
 
+  /**
+   * Set whether the sound is enabled or not
+   * @param soundName
+   * @param doEnable
+   * @returns Whether the sound is enabled or not
+   */
+  private _maybeEnableSound(soundName: Device.ToggleableSound, doEnable?: boolean): boolean {
+    if (typeof doEnable !== 'undefined') {
+      this._enabledSounds[soundName] = doEnable;
+    }
+    return this._enabledSounds[soundName];
+  }
+
   /**
    * Remove an input device from inputs
    * @param lostDevice
@@ -669,13 +696,6 @@ namespace AudioHelper {
      */
     audioContext?: AudioContext;
 
-    /**
-     * A Record of sounds. This is modified by reference, and is used to
-     * maintain backward-compatibility. This should be removed or refactored in 2.0.
-     * TODO: Remove / refactor in 2.0. (CLIENT-5302)
-     */
-    enabledSounds?: Record<Device.ToggleableSound, boolean>;
-
     /**
      * A custom MediaDevices instance to use.
      */

lib/twilio/call.ts

@@ -174,6 +174,11 @@ class Call extends EventEmitter {
    */
   private _isCancelled: boolean = false;
 
+  /**
+   * Whether the call has been rejected
+   */
+  private _isRejected: boolean = false;
+
   /**
    * Whether or not the browser uses unified-plan SDP by default.
    */
@@ -515,15 +520,16 @@ class Call extends EventEmitter {
       if (this._options.shouldPlayDisconnect && this._options.shouldPlayDisconnect()
         // Don't play disconnect sound if this was from a cancel event. i.e. the call
         // was ignored or hung up even before it was answered.
-        && !this._isCancelled) {
+        // Similarly, don't play disconnect sound if the call was rejected.
+        && !this._isCancelled && !this._isRejected) {
 
         this._soundcache.get(Device.SoundName.Disconnect).play();
       }
 
       monitor.disable();
       this._publishMetrics();
 
-      if (!this._isCancelled) {
+      if (!this._isCancelled && !this._isRejected) {
         // tslint:disable no-console
         this.emit('disconnect', this);
       }
@@ -630,13 +636,13 @@ class Call extends EventEmitter {
 
       if (this._direction === Call.CallDirection.Incoming) {
         this._isAnswered = true;
-        this._pstream.on('answer', this._onAnswer.bind(this));
+        this._pstream.on('answer', this._onAnswer);
         this._mediaHandler.answerIncomingCall(this.parameters.CallSid, this._options.offerSdp,
           rtcConstraints, rtcConfiguration, onAnswer);
       } else {
         const params = Array.from(this.customParameters.entries()).map(pair =>
          `${encodeURIComponent(pair[0])}=${encodeURIComponent(pair[1])}`).join('&');
-        this._pstream.on('answer', this._onAnswer.bind(this));
+        this._pstream.on('answer', this._onAnswer);
         this._mediaHandler.makeOutgoingCall(this._pstream.token, params, this.outboundConnectionId,
           rtcConstraints, rtcConfiguration, onAnswer);
       }
@@ -784,11 +790,14 @@ class Call extends EventEmitter {
       return;
     }
 
+    this._isRejected = true;
     this._pstream.reject(this.parameters.CallSid);
-    this._status = Call.State.Closed;
-    this.emit('reject');
     this._mediaHandler.reject(this.parameters.CallSid);
     this._publisher.info('connection', 'rejected-by-local', null, this);
+    this._cleanupEventListeners();
+    this._mediaHandler.close();
+    this._status = Call.State.Closed;
+    this.emit('reject');
   }
 
   /**

lib/twilio/device.ts

@@ -354,15 +354,6 @@ class Device extends EventEmitter {
    */
   private _edge: string | null = null;
 
-  /**
-   * Whether each sound is enabled.
-   */
-  private _enabledSounds: Record<Device.ToggleableSound, boolean> = {
-    [Device.SoundName.Disconnect]: true,
-    [Device.SoundName.Incoming]: true,
-    [Device.SoundName.Outgoing]: true,
-  };
-
   /**
    * The name of the home region the {@link Device} is connected to.
    */
@@ -812,6 +803,8 @@ class Device extends EventEmitter {
 
   /**
    * Update the token used by this {@link Device} to connect to Twilio.
+   * It is recommended to call this API after [[Device.tokenWillExpireEvent]] is emitted,
+   * and before or after a call to prevent a potential ~1s audio loss during the update process.
    * @param token
    */
   updateToken(token: string) {
@@ -976,7 +969,7 @@ class Device extends EventEmitter {
       maxAverageBitrate: this._options.maxAverageBitrate,
       preflight: this._options.preflight,
       rtcConstraints: this._options.rtcConstraints,
-      shouldPlayDisconnect: () => this._enabledSounds.disconnect,
+      shouldPlayDisconnect: () => this.audio?.disconnect(),
       twimlParams,
       voiceEventSidGenerator: this._options.voiceEventSidGenerator,
     }, options);
@@ -1001,7 +994,7 @@ class Device extends EventEmitter {
         this._audio._maybeStartPollingVolume();
       }
 
-      if (call.direction === Call.CallDirection.Outgoing && this._enabledSounds.outgoing) {
+      if (call.direction === Call.CallDirection.Outgoing && this.audio?.outgoing()) {
         this._soundcache.get(Device.SoundName.Outgoing).play();
       }
 
@@ -1211,7 +1204,7 @@ class Device extends EventEmitter {
       this._publishNetworkChange();
     });
 
-    const play = (this._enabledSounds.incoming && !wasBusy)
+    const play = (this.audio?.incoming() && !wasBusy)
       ? () => this._soundcache.get(Device.SoundName.Incoming).play()
       : () => Promise.resolve();
 
@@ -1318,10 +1311,7 @@ class Device extends EventEmitter {
       this._updateSinkIds,
       this._updateInputStream,
       getUserMedia,
-      {
-        audioContext: Device.audioContext,
-        enabledSounds: this._enabledSounds,
-      },
+      { audioContext: Device.audioContext },
     );
 
     this._audio.on('deviceChange', (lostActiveDevices: MediaDeviceInfo[]) => {

lib/twilio/rtc/icecandidate.ts

@@ -10,6 +10,7 @@
  */
 interface RTCIceCandidatePayload {
   candidate_type: string;
+  // Deprecated by newer browsers. Will likely not show on most recent versions of browsers.
   deleted: boolean;
   ip: string;
   is_remote: boolean;

lib/twilio/rtc/stats.js

@@ -204,8 +204,9 @@ function createRTCSample(statsReport) {
   }
 
   Object.assign(sample, {
-    localAddress: localCandidate && localCandidate.ip,
-    remoteAddress: remoteCandidate && remoteCandidate.ip,
+    // ip is deprecated. use address first then ip if on older versions of browser
+    localAddress: localCandidate && (localCandidate.address || localCandidate.ip),
+    remoteAddress: remoteCandidate && (remoteCandidate.address || remoteCandidate.ip),
   });
 
   return sample;

lib/twilio/shims/mediadevices.js

@@ -2,7 +2,7 @@ const EventTarget = require('./eventtarget');
 
 const POLL_INTERVAL_MS = 500;
 
-const nativeMediaDevices = typeof navigator !== 'undefined' && navigator.mediaDevices;
+let nativeMediaDevices = null;
 
 /**
  * Make a custom MediaDevices object, and proxy through existing functionality. If
@@ -40,7 +40,7 @@ class MediaDevicesShim extends EventTarget {
 
     if (typeof nativeMediaDevices.enumerateDevices === 'function') {
       nativeMediaDevices.enumerateDevices().then(devices => {
-        devices.sort(sortDevicesById).forEach([].push, knownDevices);
+        devices.sort(sortDevicesById).forEach(d => knownDevices.push(d));
       });
     }
 
@@ -49,6 +49,7 @@ class MediaDevicesShim extends EventTarget {
         return;
       }
 
+      // TODO: Remove polling in the next major release.
       this._pollInterval = this._pollInterval
         || setInterval(sampleDevices.bind(null, this), POLL_INTERVAL_MS);
     }.bind(this));
@@ -62,22 +63,38 @@ class MediaDevicesShim extends EventTarget {
   }
 }
 
-if (nativeMediaDevices && typeof nativeMediaDevices.enumerateDevices === 'function') {
-  MediaDevicesShim.prototype.enumerateDevices = function enumerateDevices() {
+MediaDevicesShim.prototype.enumerateDevices = function enumerateDevices() {
+  if (nativeMediaDevices && typeof nativeMediaDevices.enumerateDevices === 'function') {
     return nativeMediaDevices.enumerateDevices(...arguments);
-  };
-}
+  }
+  return null;
+};
 
 MediaDevicesShim.prototype.getUserMedia = function getUserMedia() {
   return nativeMediaDevices.getUserMedia(...arguments);
 };
 
 function deviceInfosHaveChanged(newDevices, oldDevices) {
-  const oldLabels = oldDevices.reduce((map, device) => map.set(device.deviceId, device.label || null), new Map());
+  newDevices = newDevices.filter(d => d.kind === 'audioinput' || d.kind === 'audiooutput');
+  oldDevices = oldDevices.filter(d => d.kind === 'audioinput' || d.kind === 'audiooutput');
+
+  // On certain browsers, we cannot use deviceId as a key for comparison.
+  // It's missing along with the device label if the customer has not granted permission.
+  // The following checks whether some old devices have empty labels and if they are now available.
+  // This means, the user has granted permissions and the device info have changed.
+  if (oldDevices.some(d => !d.deviceId) &&
+    newDevices.some(d => !!d.deviceId)) {
+    return true;
+  }
+
+  // Use both deviceId and "kind" to create a unique key
+  // since deviceId is not unique across different kinds of devices.
+  const oldLabels = oldDevices.reduce((map, device) =>
+    map.set(`${device.deviceId}-${device.kind}`, device.label), new Map());
 
-  return newDevices.some(newDevice => {
-    const oldLabel = oldLabels.get(newDevice.deviceId);
-    return typeof oldLabel !== 'undefined' && oldLabel !== newDevice.label;
+  return newDevices.some(device => {
+    const oldLabel = oldLabels.get(`${device.deviceId}-${device.kind}`);
+    return typeof oldLabel !== 'undefined' && oldLabel !== device.label;
   });
 }
 
@@ -164,6 +181,7 @@ function sortDevicesById(a, b) {
   return a.deviceId < b.deviceId;
 }
 
-module.exports = (function shimMediaDevices() {
+module.exports = () => {
+  nativeMediaDevices = typeof navigator !== 'undefined' ? navigator.mediaDevices : null;
   return nativeMediaDevices ? new MediaDevicesShim() : null;
-})();
+};