Skip to content

Commit

Permalink
Fix ice candidate proto to webrtc interface conversion (#386)
Browse files Browse the repository at this point in the history
  • Loading branch information
edaniels authored Oct 11, 2024
1 parent 72b8298 commit 1449855
Show file tree
Hide file tree
Showing 13 changed files with 77 additions and 47 deletions.
6 changes: 6 additions & 0 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
{
Expand Down
7 changes: 4 additions & 3 deletions playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand Down
4 changes: 2 additions & 2 deletions src/app/data-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export class DataClient {
) {
const dataReq = {
filter,
limit: limit ? BigInt(limit) : undefined,
limit: limit === undefined ? undefined : BigInt(limit),
sortOrder,
last,
};
Expand Down Expand Up @@ -185,7 +185,7 @@ export class DataClient {
) {
const dataReq = {
filter,
limit: limit ? BigInt(limit) : undefined,
limit: limit === undefined ? undefined : BigInt(limit),
sortOrder,
last,
};
Expand Down
2 changes: 1 addition & 1 deletion src/app/viam-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
);
Expand Down
6 changes: 3 additions & 3 deletions src/robot/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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)
Expand Down
11 changes: 7 additions & 4 deletions src/robot/dial.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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`
);
Expand Down
2 changes: 1 addition & 1 deletion src/robot/grpc-connection-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/robot/session-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
55 changes: 37 additions & 18 deletions src/rpc/dial.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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
Expand All @@ -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}`);

Expand Down Expand Up @@ -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();
Expand All @@ -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
}

Expand Down Expand Up @@ -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;
Expand All @@ -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'
);
Expand All @@ -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'
Expand Down
23 changes: 12 additions & 11 deletions src/rpc/signaling-exchange.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -263,7 +264,7 @@ export class SignalingExchange {
return;
}

if (!this.callUuid) {
if (this.callUuid === undefined || this.callUuid === '') {
throw new Error(callUUIDUnset);
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/stream-client-stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export class StreamClientStream<
if (signal) {
opt.signal = signal;
}
if (timeoutMs) {
if (timeoutMs !== undefined) {
opt.timeoutMs = timeoutMs;
}

Expand Down
2 changes: 1 addition & 1 deletion src/rpc/unary-client-stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export class UnaryClientStream<
if (signal) {
opt.signal = signal;
}
if (timeoutMs) {
if (timeoutMs !== undefined) {
opt.timeoutMs = timeoutMs;
}
return runUnaryCall<I, O>(opt);
Expand Down
2 changes: 1 addition & 1 deletion src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export const doCommandFromClient = async function doCommandFromClient(

const response = await doCommander(request);
const result = response.result?.toJson();
if (!result) {
if (result === undefined) {
return {};
}
return result;
Expand Down

0 comments on commit 1449855

Please sign in to comment.