Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: added 'forced' param to IpNetworkDetector.detect() #4001

Open
wants to merge 2 commits into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions packages/@webex/internal-plugin-device/src/ipNetworkDetector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@

import {WebexPlugin} from '@webex/webex-core';

const STATE = {
INITIAL: 'initial',
IN_PROGRESS: 'in-progress',
IDLE: 'idle',
};

/**
* @class
*/
Expand All @@ -17,6 +23,8 @@ const IpNetworkDetector = WebexPlugin.extend({
firstIpV6: ['number', true, -1], // time [ms] it took to receive first IPv6 candidate
firstMdns: ['number', true, -1], // time [ms] it took to receive first mDNS candidate
totalTime: ['number', true, -1], // total time [ms] it took to do the last IP network detection
state: ['string', true, STATE.INITIAL],
pendingDetection: ['object', false, undefined],
},

derived: {
Expand Down Expand Up @@ -155,21 +163,41 @@ const IpNetworkDetector = WebexPlugin.extend({
* Detects if we are on IPv4 and/or IPv6 network. Once it resolves, read the
* supportsIpV4 and supportsIpV6 props to find out the result.
*
* @returns {Promise<Object>}
* @param {boolean} force - if false, the detection will only be done if we haven't managed to get any meaningful results yet
* @returns {Promise<void>}
*/
async detect() {
async detect(force = false) {
let results;
let pc;

if (this.state === STATE.IN_PROGRESS) {
this.pendingDetection = {force};

return;
}

if (!force && this.state !== STATE.INITIAL && !this.receivedOnlyMDnsCandidates()) {
// we already have the results, no need to do the detection again
return;
}

try {
this.state = STATE.IN_PROGRESS;

pc = new RTCPeerConnection();

results = await this.gatherLocalCandidates(pc);
} finally {
pc.close();
this.state = STATE.IDLE;
}

return results;
if (this.pendingDetection) {
const {force: forceParam} = this.pendingDetection;

this.pendingDetection = undefined;
this.detect(forceParam);
}
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import sinon from 'sinon';
import IpNetworkDetector from '@webex/internal-plugin-device/src/ipNetworkDetector';
import MockWebex from '@webex/test-helper-mock-webex';

const flushPromises = () => new Promise(setImmediate);

describe('plugin-device', () => {
describe('IpNetworkDetector', () => {
let webex;
Expand Down Expand Up @@ -326,7 +328,7 @@ describe('plugin-device', () => {
});

// now call detect() again
const promise2 = ipNetworkDetector.detect();
const promise2 = ipNetworkDetector.detect(true);

// everything should have been reset
assert.equal(ipNetworkDetector.supportsIpV4, undefined);
Expand All @@ -340,6 +342,87 @@ describe('plugin-device', () => {
await promise2;
});

it('queues another detect() call if one is already in progress', async () => {
const promise = ipNetworkDetector.detect();

simulateCandidate(50, '192.168.0.1');

await flushPromises();

assert.calledOnce(fakePeerConnection.createDataChannel);
assert.calledOnce(fakePeerConnection.createOffer);
assert.calledOnce(fakePeerConnection.setLocalDescription);

// now call detect() again
ipNetworkDetector.detect(true);

// simulate the end of the detection -> another one should be started
simulateEndOfCandidateGathering(10);

await promise;

assert.calledTwice(fakePeerConnection.createDataChannel);
assert.calledTwice(fakePeerConnection.createOffer);
assert.calledTwice(fakePeerConnection.setLocalDescription);

simulateCandidate(50, '2a02:c7c:a0d0:8a00:db9b:d4de:d1f7:4c49');
simulateEndOfCandidateGathering(10);

// results should reflect the last run detection
checkResults({
supportsIpV4: false,
supportsIpV6: true,
timings: {
totalTime: 60,
ipv4: -1,
ipv6: 50,
mdns: -1,
},
});

await flushPromises();

// no more detections should be started
assert.calledTwice(fakePeerConnection.createDataChannel);
});

it.each`
force | state | receivedOnlyMDnsCandidates | expectedToRunDetection
${true} | ${'initial'} | ${false} | ${true}
${true} | ${'idle'} | ${false} | ${true}
${true} | ${'initial'} | ${true} | ${true}
${true} | ${'idle'} | ${true} | ${true}
${false} | ${'initial'} | ${false} | ${true}
${false} | ${'initial'} | ${true} | ${true}
${false} | ${'idle'} | ${true} | ${true}
${false} | ${'idle'} | ${false} | ${false}
`(
'force=$force, state=$state, receivedOnlyMDnsCandidates=$receivedOnlyMDnsCandidates => expectedToRunDetection=$expectedToRunDetection',
async ({force, state, receivedOnlyMDnsCandidates, expectedToRunDetection}) => {
ipNetworkDetector.state = state;
sinon
.stub(ipNetworkDetector, 'receivedOnlyMDnsCandidates')
.returns(receivedOnlyMDnsCandidates);

const result = ipNetworkDetector.detect(force);

if (expectedToRunDetection) {
simulateEndOfCandidateGathering(10);
}
await result;

if (expectedToRunDetection) {
assert.calledOnce(fakePeerConnection.createDataChannel);
assert.calledOnce(fakePeerConnection.createOffer);
assert.calledOnce(fakePeerConnection.setLocalDescription);
} else {
assert.notCalled(fakePeerConnection.createDataChannel);
assert.notCalled(fakePeerConnection.createOffer);
assert.notCalled(fakePeerConnection.setLocalDescription);
}
}
);

it('rejects if one of RTCPeerConnection operations fails', async () => {
const fakeError = new Error('fake error');

Expand Down
6 changes: 3 additions & 3 deletions packages/@webex/plugin-meetings/src/reachability/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,10 @@ export default class Reachability extends EventsScope {
try {
this.lastTrigger = trigger;

// kick off ip version detection. For now we don't await it, as we're doing it
// to gather the timings and send them with our reachability metrics
// kick off ip version detection. We don't await it, as we don't want to waste time
// and if it fails, that's ok we can still carry on
// @ts-ignore
this.webex.internal.device.ipNetworkDetector.detect();
this.webex.internal.device.ipNetworkDetector.detect(true);

const {clusters, joinCookie} = await this.getClusters();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1234,7 +1234,7 @@ describe('gatherReachability', () => {
assert.equal(receivedEvents['done'], 1);

// and that ip network detection was started
assert.calledOnceWithExactly(webex.internal.device.ipNetworkDetector.detect);
assert.calledOnceWithExactly(webex.internal.device.ipNetworkDetector.detect, true);

// finally, check the metrics - they should contain values from ipNetworkDetector
assert.calledWith(Metrics.sendBehavioralMetric, 'js_sdk_reachability_completed', {
Expand Down
Loading