feat (datafile manager): Abortable requests, abort on stop (#2)

Summary:

Update expected return value of abstract method `makeGetRequest`. Should return a request object with an `abort` method. This will be useful in the Node datafile manager for aborting the request on `stop`.

Test plan:

Updated unit tests

Issues:

https://optimizely.atlassian.net/browse/OASIS-4309

Author: mjc1283

packages/datafile-manager/__test__/httpPollingDatafileManager.spec.ts

@@ -15,7 +15,7 @@
  */
 
 import HTTPPollingDatafileManager from '../src/httpPollingDatafileManager'
-import { Headers, Response } from '../src/http'
+import { Headers, AbortableRequest, Response } from '../src/http'
 import { TimeoutFactory } from '../src/timeoutFactory'
 import { DatafileManagerConfig } from '../src/datafileManager';
 
@@ -46,14 +46,16 @@ class TestDatafileManager extends HTTPPollingDatafileManager {
 
   responsePromises: Promise<Response>[] = []
 
-  makeGetRequest(url: string, headers: Headers): Promise<Response> {
+  makeGetRequest(url: string, headers: Headers): AbortableRequest {
     const nextResponse: Response | undefined = this.queuedResponses.pop()
+    let responsePromise: Promise<Response>
     if (nextResponse === undefined) {
-      return Promise.reject('No responses queued')
+      responsePromise = Promise.reject('No responses queued')
+    } else {
+      responsePromise = Promise.resolve(nextResponse)
     }
-    const respPromise = Promise.resolve(nextResponse)
-    this.responsePromises.push(respPromise)
-    return respPromise
+    this.responsePromises.push(responsePromise)
+    return  { responsePromise, abort: jest.fn() }
   }
 }
 
@@ -103,10 +105,6 @@ describe('httpPollingDatafileManager', () => {
       manager = createTestManager({ sdkKey: '123', updateInterval: 10 })
     })
 
-    afterEach(() => {
-      manager.stop()
-    })
-
     describe('initial state', () => {
       it('returns null from get before becoming ready', () => {
         expect(manager.get()).toBeNull()
@@ -239,6 +237,23 @@ describe('httpPollingDatafileManager', () => {
           expect(manager.get()).toBe('{"foo": "bar"}')
         })
 
+        it('calls abort on the current request if there is a current request when stop is called', async () => {
+          manager.queuedResponses.push(
+            {
+              statusCode: 200,
+              body: '{"foo2": "bar2"}',
+              headers: {},
+            }
+          )
+          const makeGetRequestSpy = jest.spyOn(manager, 'makeGetRequest')
+          manager.start()
+          const currentRequest = makeGetRequestSpy.mock.results[0]
+          expect(currentRequest.type).toBe('return')
+          expect(currentRequest.value.abort).toBeCalledTimes(0)
+          manager.stop()
+          expect(currentRequest.value.abort).toBeCalledTimes(1)
+        })
+
         it('can fail to become ready on the initial request, but succeed after a later polling update', async () => {
           manager.queuedResponses.push(
             {
@@ -366,7 +381,7 @@ describe('httpPollingDatafileManager', () => {
         body: '{"foo": "bar"}',
         headers: {},
       })
-      manager.makeGetRequest = () => Promise.reject(new Error('Could not connect'))
+      manager.makeGetRequest = () => ({ abort() {}, responsePromise: Promise.reject(new Error('Could not connect')) })
       manager.start()
       let didReject = false
       try {

packages/datafile-manager/src/http.ts

@@ -23,3 +23,8 @@ export interface Response {
   body: string
   headers: Headers
 }
+
+export interface AbortableRequest {
+  abort(): void
+  responsePromise: Promise<Response>
+}

packages/datafile-manager/src/httpPollingDatafileManager.ts

@@ -17,7 +17,7 @@
 import { getLogger } from '@optimizely/js-sdk-logging'
 import { DatafileManager, DatafileManagerConfig, DatafileUpdate } from './datafileManager';
 import EventEmitter from './eventEmitter'
-import { Response, Headers } from './http';
+import { AbortableRequest, Response, Headers } from './http';
 import { DEFAULT_UPDATE_INTERVAL, MIN_UPDATE_INTERVAL, DEFAULT_URL_TEMPLATE, SDK_KEY_TOKEN } from './config'
 import { TimeoutFactory, DEFAULT_TIMEOUT_FACTORY } from './timeoutFactory'
 
@@ -31,8 +31,10 @@ function isValidUpdateInterval(updateInterval: number): boolean {
 
 export default abstract class HTTPPollingDatafileManager implements DatafileManager {
   // Make an HTTP get request to the given URL with the given headers
-  // Return a promise for a Response. If we can't get a response, the promise is rejected
-  protected abstract makeGetRequest(reqUrl: string, headers: Headers): Promise<Response>
+  // Return an AbortableRequest, which has a promise for a Response.
+  // If we can't get a response, the promise is rejected.
+  // The request will be aborted if the manager is stopped while the request is in flight.
+  protected abstract makeGetRequest(reqUrl: string, headers: Headers): AbortableRequest
 
   public readonly onReady: Promise<void>
 
@@ -62,6 +64,8 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana
 
   private timeoutFactory: TimeoutFactory
 
+  private currentRequest?: AbortableRequest
+
   constructor(config: DatafileManagerConfig) {
     const {
       datafile,
@@ -125,7 +129,14 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana
       this.cancelTimeout()
       this.cancelTimeout = undefined
     }
+
     this.emitter.removeAllListeners()
+
+    if (this.currentRequest) {
+      this.currentRequest.abort()
+      this.currentRequest = undefined
+    }
+
     return Promise.resolve()
   }
 
@@ -178,6 +189,8 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana
       return
     }
 
+    this.currentRequest = undefined
+
     if (this.liveUpdates) {
       this.scheduleNextUpdate()
     }
@@ -196,7 +209,7 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana
     const datafileUrl = this.getUrl(this.sdkKey)
 
     logger.debug('Making datafile request to url %s with headers: %s', datafileUrl, () => JSON.stringify(headers))
-    const datafileFetch = this.makeGetRequest(datafileUrl, headers)
+    this.currentRequest = this.makeGetRequest(datafileUrl, headers)
 
     const onFetchComplete = () => {
       this.onFetchComplete()
@@ -207,7 +220,7 @@ export default abstract class HTTPPollingDatafileManager implements DatafileMana
     const logMakeGetRequestError = (err: any) => {
       this.logMakeGetRequestError(err)
     }
-    datafileFetch
+    this.currentRequest.responsePromise
       .then(tryUpdatingDatafile, logMakeGetRequestError)
       .then(onFetchComplete, onFetchComplete)
   }