From 7e7c3ec580313b070a0daca99f4e52a3b5b0f38a Mon Sep 17 00:00:00 2001 From: Marcin Wojtczak Date: Thu, 21 Nov 2024 21:52:45 +0000 Subject: [PATCH] fix: added 'forced' param to IpNetworkDetector.detect() --- .../src/ipNetworkDetector.ts | 34 +++++++- .../test/unit/spec/ipNetworkDetector.js | 85 ++++++++++++++++++- .../plugin-meetings/src/reachability/index.ts | 6 +- .../test/unit/spec/reachability/index.ts | 2 +- 4 files changed, 119 insertions(+), 8 deletions(-) diff --git a/packages/@webex/internal-plugin-device/src/ipNetworkDetector.ts b/packages/@webex/internal-plugin-device/src/ipNetworkDetector.ts index cf7b828d002..44c3c9c9649 100644 --- a/packages/@webex/internal-plugin-device/src/ipNetworkDetector.ts +++ b/packages/@webex/internal-plugin-device/src/ipNetworkDetector.ts @@ -4,6 +4,12 @@ import {WebexPlugin} from '@webex/webex-core'; +const STATE = { + INITIAL: 'initial', + IN_PROGRESS: 'in-progress', + IDLE: 'idle', +}; + /** * @class */ @@ -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: { @@ -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} + * @param {boolean} force - if false, the detection will only be done if we haven't managed to get any meaningful results yet + * @returns {Promise} */ - 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); + } }, }); diff --git a/packages/@webex/internal-plugin-device/test/unit/spec/ipNetworkDetector.js b/packages/@webex/internal-plugin-device/test/unit/spec/ipNetworkDetector.js index 5e44e318fe5..b5d57f6ce30 100644 --- a/packages/@webex/internal-plugin-device/test/unit/spec/ipNetworkDetector.js +++ b/packages/@webex/internal-plugin-device/test/unit/spec/ipNetworkDetector.js @@ -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; @@ -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); @@ -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'); diff --git a/packages/@webex/plugin-meetings/src/reachability/index.ts b/packages/@webex/plugin-meetings/src/reachability/index.ts index 7b6868f04f0..ff6d47a14b7 100644 --- a/packages/@webex/plugin-meetings/src/reachability/index.ts +++ b/packages/@webex/plugin-meetings/src/reachability/index.ts @@ -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(); diff --git a/packages/@webex/plugin-meetings/test/unit/spec/reachability/index.ts b/packages/@webex/plugin-meetings/test/unit/spec/reachability/index.ts index 29b9ae371ec..029668742ee 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/reachability/index.ts +++ b/packages/@webex/plugin-meetings/test/unit/spec/reachability/index.ts @@ -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', {