Fix ice candidate proto to webrtc interface conversion (#386)

Author: edaniels

.eslintrc.cjs

@@ -30,6 +30,12 @@ module.exports = {
     '@typescript-eslint/no-explicit-any': 'warn',
     '@typescript-eslint/prefer-promise-reject-errors': 'warn',
     'unicorn/prefer-add-event-listener': 'warn',
+    '@typescript-eslint/strict-boolean-expressions': [
+      'error',
+      {
+        allowNullableBoolean: true,
+      },
+    ],
   },
   overrides: [
     {

playwright.config.ts

@@ -4,12 +4,13 @@ export default defineConfig({
   testDir: 'e2e/tests',
   outputDir: 'e2e/artifacts',
   forbidOnly: Boolean(process.env.CI),
-  retries: process.env.CI ? 1 : 0,
-  workers: process.env.CI ? 1 : 3,
+  retries: process.env.CI === undefined ? 0 : 1,
+  workers: process.env.CI === undefined ? 3 : 1,
   reporter: [['html', { open: 'never' }]],
   use: {
     baseURL: 'http://localhost:5173',
-    trace: process.env.CI ? 'on-first-retry' : 'retain-on-failure',
+    trace:
+      process.env.CI === undefined ? 'retain-on-failure' : 'on-first-retry',
   },
 
   projects: [

src/app/data-client.ts

@@ -113,7 +113,7 @@ export class DataClient {
   ) {
     const dataReq = {
       filter,
-      limit: limit ? BigInt(limit) : undefined,
+      limit: limit === undefined ? undefined : BigInt(limit),
       sortOrder,
       last,
     };
@@ -185,7 +185,7 @@ export class DataClient {
   ) {
     const dataReq = {
       filter,
-      limit: limit ? BigInt(limit) : undefined,
+      limit: limit === undefined ? undefined : BigInt(limit),
       sortOrder,
       last,
     };

src/app/viam-client.ts

@@ -77,7 +77,7 @@ export class ViamClient {
       locationId = mainPart.locationId;
     }
 
-    if (!address) {
+    if (address === undefined || address === '') {
       throw new Error(
         'Host was not provided and could not be obtained from the machine ID'
       );

src/robot/client.ts

@@ -196,7 +196,7 @@ export class RobotClient extends EventDispatcher implements Robot {
   private onDisconnect(event?: Event) {
     this.emit(MachineConnectionEvent.DISCONNECTED, event ?? {});
 
-    if (this.noReconnect) {
+    if (this.noReconnect !== undefined && this.noReconnect) {
       return;
     }
 
@@ -220,10 +220,10 @@ export class RobotClient extends EventDispatcher implements Robot {
         return true;
       },
     };
-    if (this.reconnectMaxWait) {
+    if (this.reconnectMaxWait !== undefined) {
       backOffOpts.maxDelay = this.reconnectMaxWait;
     }
-    if (this.reconnectMaxAttempts) {
+    if (this.reconnectMaxAttempts !== undefined) {
       backOffOpts.numOfAttempts = this.reconnectMaxAttempts;
     }
     void backOff(async () => this.connect(), backOffOpts)

src/robot/dial.ts

@@ -173,10 +173,10 @@ export const createRobotClient = async (
       return !conf.reconnectAbortSignal?.abort;
     },
   };
-  if (conf.reconnectMaxWait) {
+  if (conf.reconnectMaxWait !== undefined) {
     backOffOpts.maxDelay = conf.reconnectMaxWait;
   }
-  if (conf.reconnectMaxAttempts) {
+  if (conf.reconnectMaxAttempts !== undefined) {
     backOffOpts.numOfAttempts = conf.reconnectMaxAttempts;
   }
 
@@ -221,12 +221,15 @@ const validateDialConf = (conf: DialConf) => {
     }
   }
 
-  if (conf.reconnectMaxAttempts && !isPosInt(conf.reconnectMaxAttempts)) {
+  if (
+    conf.reconnectMaxAttempts !== undefined &&
+    !isPosInt(conf.reconnectMaxAttempts)
+  ) {
     throw new Error(
       `Value of max reconnect attempts (${conf.reconnectMaxAttempts}) should be a positive integer`
     );
   }
-  if (conf.reconnectMaxWait && !isPosInt(conf.reconnectMaxWait)) {
+  if (conf.reconnectMaxWait !== undefined && !isPosInt(conf.reconnectMaxWait)) {
     throw new Error(
       `Value of max reconnect wait (${conf.reconnectMaxWait}) should be a positive integer`
     );

src/robot/grpc-connection-manager.ts

@@ -44,7 +44,7 @@ export default class GRPCConnectionManager {
     };
 
     // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
-    if (window.Worker) {
+    if (globalThis.Worker !== undefined) {
       const url = window.URL.createObjectURL(timeoutBlob);
       worker = new Worker(url);
       URL.revokeObjectURL(url);

src/robot/session-manager.ts

@@ -104,7 +104,7 @@ export default class SessionManager {
      * case in the future we make this toggleable (e.g. foreground).
      */
     // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
-    if (this.backgroundHeartbeat && globalThis.Worker) {
+    if (this.backgroundHeartbeat && globalThis.Worker !== undefined) {
       const url = window.URL.createObjectURL(timeoutBlob);
       worker = new Worker(url);
       URL.revokeObjectURL(url);

src/rpc/dial.ts

@@ -115,15 +115,24 @@ export const dialDirect = async (
 
   // Client already has access token with no external auth, skip Authenticate process.
   if (
-    opts?.accessToken &&
-    !(opts.externalAuthAddress && opts.externalAuthToEntity)
+    opts?.accessToken !== undefined &&
+    opts.accessToken !== '' &&
+    !(
+      opts.externalAuthAddress !== undefined &&
+      opts.externalAuthAddress !== '' &&
+      opts.externalAuthToEntity !== undefined &&
+      opts.externalAuthToEntity !== ''
+    )
   ) {
     const headers = new Headers(opts.extraHeaders);
     headers.set('authorization', `Bearer ${opts.accessToken}`);
     return new AuthenticatedTransport(transportOpts, createTransport, headers);
   }
 
-  if (!opts || (!opts.credentials && !opts.accessToken)) {
+  if (
+    opts === undefined ||
+    (opts.credentials === undefined && opts.accessToken === undefined)
+  ) {
     return createTransport(transportOpts);
   }
 
@@ -146,7 +155,7 @@ const makeAuthenticatedTransport = async (
   const authHeaders = new Headers(opts.extraHeaders);
 
   let accessToken;
-  if (!opts.accessToken || opts.accessToken === '') {
+  if (opts.accessToken === undefined || opts.accessToken === '') {
     const request = new AuthenticateRequest({
       entity:
         isCredential(opts.credentials) && opts.credentials.authEntity
@@ -169,7 +178,12 @@ const makeAuthenticatedTransport = async (
     accessToken = opts.accessToken;
   }
 
-  if (opts.externalAuthAddress && opts.externalAuthToEntity) {
+  if (
+    opts.externalAuthAddress !== undefined &&
+    opts.externalAuthAddress !== '' &&
+    opts.externalAuthToEntity !== undefined &&
+    opts.externalAuthToEntity !== ''
+  ) {
     const extAuthHeaders = new Headers();
     extAuthHeaders.set('authorization', `Bearer ${accessToken}`);
 
@@ -385,7 +399,7 @@ export const dialWebRTC = async (
   );
   try {
     // set timeout for dial attempt if a timeout is specified
-    if (dialOpts?.dialTimeout) {
+    if (dialOpts?.dialTimeout !== undefined) {
       setTimeout(() => {
         if (!successful) {
           exchange.terminate();
@@ -395,10 +409,13 @@ export const dialWebRTC = async (
 
     const cc = await exchange.doExchange();
 
-    if (dialOpts?.externalAuthAddress) {
+    if (
+      dialOpts?.externalAuthAddress !== undefined &&
+      dialOpts.externalAuthAddress !== ''
+    ) {
       // TODO(GOUT-11): prepare AuthenticateTo here  for client channel.
       // eslint-disable-next-line sonarjs/no-duplicated-branches
-    } else if (dialOpts?.credentials?.type) {
+    } else if (dialOpts?.credentials?.type !== undefined) {
       // TODO(GOUT-11): prepare Authenticate here for client channel
     }
 
@@ -477,14 +494,16 @@ const processSignalingExchangeOpts = (
   if (dialOpts) {
     optsCopy = { ...dialOpts } as DialOptions;
 
-    if (!dialOpts.accessToken) {
+    if (dialOpts.accessToken === undefined) {
       if (
         isCredential(optsCopy.credentials) &&
         !optsCopy.credentials.authEntity
       ) {
-        optsCopy.credentials.authEntity = optsCopy.externalAuthAddress
-          ? optsCopy.externalAuthAddress.replace(addressCleanupRegex, '')
-          : signalingAddress.replace(addressCleanupRegex, '');
+        optsCopy.credentials.authEntity =
+          optsCopy.externalAuthAddress !== undefined &&
+          optsCopy.externalAuthAddress !== ''
+            ? optsCopy.externalAuthAddress.replace(addressCleanupRegex, '')
+            : signalingAddress.replace(addressCleanupRegex, '');
       }
       optsCopy.credentials = dialOpts.webrtcOptions?.signalingCredentials;
       optsCopy.accessToken = dialOpts.webrtcOptions?.signalingAccessToken;
@@ -504,18 +523,18 @@ const validateDialOptions = (opts?: DialOptions) => {
     return;
   }
 
-  if (opts.accessToken && opts.accessToken.length > 0) {
+  if (opts.accessToken !== undefined && opts.accessToken.length > 0) {
     if (opts.credentials) {
       throw new Error('cannot set credentials with accessToken');
     }
 
-    if (opts.webrtcOptions) {
-      if (opts.webrtcOptions.signalingAccessToken) {
+    if (opts.webrtcOptions !== undefined) {
+      if (opts.webrtcOptions.signalingAccessToken !== undefined) {
         throw new Error(
           'cannot set webrtcOptions.signalingAccessToken with accessToken'
         );
       }
-      if (opts.webrtcOptions.signalingCredentials) {
+      if (opts.webrtcOptions.signalingCredentials !== undefined) {
         throw new Error(
           'cannot set webrtcOptions.signalingCredentials with accessToken'
         );
@@ -524,9 +543,9 @@ const validateDialOptions = (opts?: DialOptions) => {
   }
 
   if (
-    opts.webrtcOptions?.signalingAccessToken &&
+    opts.webrtcOptions?.signalingAccessToken !== undefined &&
     opts.webrtcOptions.signalingAccessToken.length > 0 &&
-    opts.webrtcOptions.signalingCredentials
+    opts.webrtcOptions.signalingCredentials !== undefined
   ) {
     throw new Error(
       'cannot set webrtcOptions.signalingCredentials with webrtcOptions.signalingAccessToken'

src/rpc/signaling-exchange.ts

@@ -242,8 +242,9 @@ export class SignalingExchange {
     try {
       await this.pc.addIceCandidate(cand);
     } catch (error) {
+      console.log('error adding ice candidate', error); // eslint-disable-line no-console
       await this.sendError(JSON.stringify(error));
-      return false;
+      throw error;
     }
 
     return true;
@@ -263,7 +264,7 @@ export class SignalingExchange {
       return;
     }
 
-    if (!this.callUuid) {
+    if (this.callUuid === undefined || this.callUuid === '') {
       throw new Error(callUUIDUnset);
     }
 
@@ -307,7 +308,7 @@ export class SignalingExchange {
     if (this.sentDoneOrErrorOnce) {
       return;
     }
-    if (!this.callUuid) {
+    if (this.callUuid === undefined || this.callUuid === '') {
       throw new Error(callUUIDUnset);
     }
     this.sentDoneOrErrorOnce = true;
@@ -337,7 +338,7 @@ export class SignalingExchange {
     if (this.sentDoneOrErrorOnce) {
       return;
     }
-    if (!this.callUuid) {
+    if (this.callUuid === undefined || this.callUuid === '') {
       throw new Error(callUUIDUnset);
     }
     this.sentDoneOrErrorOnce = true;
@@ -365,30 +366,30 @@ const iceCandidateFromProto = (i: ICECandidate) => {
   const candidate: RTCIceCandidateInit = {
     candidate: i.candidate,
   };
-  if (i.sdpMid) {
+  if (i.sdpMid !== undefined) {
     candidate.sdpMid = i.sdpMid;
   }
-  if (i.sdpmLineIndex) {
+  if (i.sdpmLineIndex !== undefined) {
     candidate.sdpMLineIndex = i.sdpmLineIndex;
   }
-  if (i.usernameFragment) {
+  if (i.usernameFragment !== undefined) {
     candidate.usernameFragment = i.usernameFragment;
   }
   return candidate;
 };
 
 const iceCandidateToProto = (i: RTCIceCandidateInit) => {
   const candidate = new ICECandidate();
-  if (i.candidate) {
+  if (i.candidate !== undefined) {
     candidate.candidate = i.candidate;
   }
-  if (i.sdpMid) {
+  if (i.sdpMid !== undefined && i.sdpMid !== null) {
     candidate.sdpMid = i.sdpMid;
   }
-  if (i.sdpMLineIndex) {
+  if (i.sdpMLineIndex !== undefined && i.sdpMLineIndex !== null) {
     candidate.sdpmLineIndex = i.sdpMLineIndex;
   }
-  if (i.usernameFragment) {
+  if (i.usernameFragment !== undefined && i.usernameFragment !== null) {
     candidate.usernameFragment = i.usernameFragment;
   }
   return candidate;