From 749e84611ec9a2e38dffd07a165abf93c25d9b9e Mon Sep 17 00:00:00 2001 From: Khalid Adil Date: Mon, 6 Dec 2021 14:14:14 -0600 Subject: [PATCH] [Telemetry] Implement Abort Request (#4504) * Add a requestAbortController for the telemetryAPI and trigger an abort of all requests on navigation via the router. * Check for the rejection and make sure it's not an abort error to show the error --- API.md | 2 + src/api/telemetry/TelemetryAPI.js | 18 +++++++- src/api/telemetry/TelemetryAPISpec.js | 58 +++++++++++++++++++++++++- src/ui/router/ApplicationRouter.js | 2 + src/ui/router/ApplicationRouterSpec.js | 6 +++ 5 files changed, 82 insertions(+), 4 deletions(-) diff --git a/API.md b/API.md index 87467b43df..d5416e72fc 100644 --- a/API.md +++ b/API.md @@ -481,6 +481,8 @@ In this case, the `domain` is the currently selected time-system, and the start A telemetry provider's `request` method should return a promise for an array of telemetry datums. These datums must be sorted by `domain` in ascending order. +The telemetry provider's `request` method will also return an object `signal` with an `aborted` property with a value `true` if the request has been aborted by user navigation. This can be used to trigger actions when a request has been aborted. + #### Request Strategies **draft** To improve performance views may request a certain strategy for data reduction. These are intended to improve visualization performance by reducing the amount of data needed to be sent to the client. These strategies will be indicated by additional parameters in the request options. You may choose to handle them or ignore them. diff --git a/src/api/telemetry/TelemetryAPI.js b/src/api/telemetry/TelemetryAPI.js index b65a80b96d..6a774b6360 100644 --- a/src/api/telemetry/TelemetryAPI.js +++ b/src/api/telemetry/TelemetryAPI.js @@ -144,8 +144,14 @@ define([ this.metadataCache = new WeakMap(); this.formatMapCache = new WeakMap(); this.valueFormatterCache = new WeakMap(); + this.requestAbortControllers = new Set(); } + TelemetryAPI.prototype.abortAllRequests = function () { + this.requestAbortControllers.forEach((controller) => controller.abort()); + this.requestAbortControllers.clear(); + }; + /** * Return Custom String Formatter * @@ -312,6 +318,10 @@ define([ arguments[1] = {}; } + const abortController = new AbortController(); + arguments[1].signal = abortController.signal; + this.requestAbortControllers.add(abortController); + this.standardizeRequestOptions(arguments[1]); const provider = this.findRequestProvider.apply(this, arguments); if (!provider) { @@ -319,10 +329,14 @@ define([ } return provider.request.apply(provider, arguments).catch((rejected) => { - this.openmct.notifications.error('Error requesting telemetry data, see console for details'); - console.error(rejected); + if (rejected.name !== 'AbortError') { + this.openmct.notifications.error('Error requesting telemetry data, see console for details'); + console.error(rejected); + } return Promise.reject(rejected); + }).finally(() => { + this.requestAbortControllers.delete(abortController); }); }; diff --git a/src/api/telemetry/TelemetryAPISpec.js b/src/api/telemetry/TelemetryAPISpec.js index 3d182dde88..a3188c0f0d 100644 --- a/src/api/telemetry/TelemetryAPISpec.js +++ b/src/api/telemetry/TelemetryAPISpec.js @@ -19,6 +19,7 @@ * this source code distribution or the Licensing information page available * at runtime from the About dialog for additional information. *****************************************************************************/ +import { createOpenMct, resetApplicationState } from 'utils/testing'; import TelemetryAPI from './TelemetryAPI'; const { TelemetryCollection } = require("./TelemetryCollection"); @@ -268,9 +269,11 @@ describe('Telemetry API', function () { telemetryAPI.addProvider(telemetryProvider); telemetryAPI.request(domainObject).then(() => { + const { signal } = new AbortController(); expect(telemetryProvider.supportsRequest).toHaveBeenCalledWith( jasmine.any(Object), { + signal, start: 0, end: 1, domain: 'system' @@ -280,6 +283,7 @@ describe('Telemetry API', function () { expect(telemetryProvider.request).toHaveBeenCalledWith( jasmine.any(Object), { + signal, start: 0, end: 1, domain: 'system' @@ -293,6 +297,7 @@ describe('Telemetry API', function () { expect(telemetryProvider.supportsRequest).toHaveBeenCalledWith( jasmine.any(Object), { + signal, start: 0, end: 1, domain: 'system' @@ -302,6 +307,7 @@ describe('Telemetry API', function () { expect(telemetryProvider.request).toHaveBeenCalledWith( jasmine.any(Object), { + signal, start: 0, end: 1, domain: 'system' @@ -322,12 +328,14 @@ describe('Telemetry API', function () { end: 30, domain: 'someDomain' }).then(() => { + const { signal } = new AbortController(); expect(telemetryProvider.supportsRequest).toHaveBeenCalledWith( jasmine.any(Object), { start: 20, end: 30, - domain: 'someDomain' + domain: 'someDomain', + signal } ); @@ -336,7 +344,8 @@ describe('Telemetry API', function () { { start: 20, end: 30, - domain: 'someDomain' + domain: 'someDomain', + signal } ); @@ -611,3 +620,48 @@ describe('Telemetry API', function () { }); }); +describe('Telemetery', () => { + let openmct; + let telemetryProvider; + let telemetryAPI; + let watchedSignal; + + beforeEach(() => { + openmct = createOpenMct(); + openmct.install(openmct.plugins.MyItems()); + + telemetryAPI = openmct.telemetry; + + telemetryProvider = { + request: (obj, options) => { + watchedSignal = options.signal; + + return Promise.resolve(); + } + }; + spyOn(telemetryAPI, 'findRequestProvider').and.returnValue(telemetryProvider); + }); + + afterEach(() => { + return resetApplicationState(openmct); + }); + + it('should not abort request without navigation', function (done) { + telemetryAPI.addProvider(telemetryProvider); + + telemetryAPI.request({}).finally(() => { + expect(watchedSignal.aborted).toBe(false); + done(); + }); + }); + + it('should abort request on navigation', function (done) { + telemetryAPI.addProvider(telemetryProvider); + + telemetryAPI.request({}).finally(() => { + expect(watchedSignal.aborted).toBe(true); + done(); + }); + openmct.router.doPathChange('newPath', 'oldPath'); + }); +}); diff --git a/src/ui/router/ApplicationRouter.js b/src/ui/router/ApplicationRouter.js index a034f06053..8bb1c10274 100644 --- a/src/ui/router/ApplicationRouter.js +++ b/src/ui/router/ApplicationRouter.js @@ -318,6 +318,8 @@ class ApplicationRouter extends EventEmitter { route.callback(newPath, route.matcher.exec(newPath), this.currentLocation.params); } + this.openmct.telemetry.abortAllRequests(); + this.emit('change:path', newPath, oldPath); } diff --git a/src/ui/router/ApplicationRouterSpec.js b/src/ui/router/ApplicationRouterSpec.js index 6a9a56d615..f6b2072122 100644 --- a/src/ui/router/ApplicationRouterSpec.js +++ b/src/ui/router/ApplicationRouterSpec.js @@ -77,4 +77,10 @@ describe('Application router utility functions', () => { expect(searchParams.get('testParam2')).toBe('updatedtestValue2'); expect(searchParams.get('newTestParam3')).toBe('newTestValue3'); }); + + it('The doPathChange function triggers aborting all requests when doing a path change', () => { + const abortSpy = spyOn(openmct.telemetry, 'abortAllRequests'); + openmct.router.doPathChange('newPath', 'oldPath'); + expect(abortSpy).toHaveBeenCalledTimes(1); + }); });