From 2d4a52f857aa2ab69e51c4ebca3ec773e92ea6f3 Mon Sep 17 00:00:00 2001
From: Ben Lesh <ben@benlesh.com>
Date: Mon, 22 Feb 2021 10:55:30 -0600
Subject: [PATCH] fix(ajax): now errors on forced abort

In the event of a network disconnection or other abort event, the returned observable will now error with an `AjaxError` and the message `"aborted"`.

Resolves #4251
---
 spec/observables/dom/ajax-spec.ts | 73 ++++++++++---------------------
 src/internal/ajax/ajax.ts         | 44 ++++++++++++++++---
 2 files changed, 61 insertions(+), 56 deletions(-)

diff --git a/spec/observables/dom/ajax-spec.ts b/spec/observables/dom/ajax-spec.ts
index e4458e2e95..23182d897b 100644
--- a/spec/observables/dom/ajax-spec.ts
+++ b/spec/observables/dom/ajax-spec.ts
@@ -898,56 +898,6 @@ describe('ajax', () => {
     });
   });
 
-  it('should work fine when XMLHttpRequest ontimeout property is monkey patched', function (done) {
-    Object.defineProperty(root.XMLHttpRequest.prototype, 'ontimeout', {
-      set(fn: (e: ProgressEvent) => any) {
-        const wrapFn = (ev: ProgressEvent) => {
-          const result = fn.call(this, ev);
-          if (result === false) {
-            ev.preventDefault();
-          }
-        };
-        this['_ontimeout'] = wrapFn;
-      },
-      get() {
-        return this['_ontimeout'];
-      },
-      configurable: true,
-    });
-
-    const ajaxRequest: AjaxConfig = {
-      url: '/flibbertyJibbet',
-    };
-
-    ajax(ajaxRequest).subscribe({
-      error(err) {
-        expect(err.name).to.equal('AjaxTimeoutError');
-        done();
-      },
-    });
-
-    const request = MockXMLHttpRequest.mostRecent;
-    try {
-      request.ontimeout('ontimeout' as any);
-    } catch (e) {
-      expect(e.message).to.equal(
-        new AjaxTimeoutError(request as any, {
-          url: ajaxRequest.url,
-          method: 'GET',
-          headers: {
-            'content-type': 'application/json;encoding=Utf-8',
-          },
-          withCredentials: false,
-          async: true,
-          timeout: 0,
-          crossDomain: false,
-          responseType: 'json',
-        }).message
-      );
-    }
-    delete root.XMLHttpRequest.prototype.ontimeout;
-  });
-
   describe('ajax.patch', () => {
     it('should create an AjaxObservable with correct options', () => {
       const expected = { foo: 'bar', hi: 'there you' };
@@ -1033,6 +983,29 @@ describe('ajax', () => {
     });
   });
 
+  it('should error if aborted early', () => {
+    let thrown: any = null;
+
+    ajax({
+      method: 'GET',
+      url: '/flibbertyJibbett',
+    }).subscribe({
+      next: () => {
+        throw new Error('should not be called');
+      },
+      error: (err) => {
+        thrown = err;
+      },
+    });
+
+    const mockXHR = MockXMLHttpRequest.mostRecent;
+    expect(thrown).to.be.null;
+
+    mockXHR.triggerEvent('abort', { type: 'abort' });
+    expect(thrown).to.be.an.instanceOf(AjaxError);
+    expect(thrown.message).to.equal('aborted');
+  });
+
   describe('with includeDownloadProgress', () => {
     it('should emit download progress', () => {
       const results: any[] = [];
diff --git a/src/internal/ajax/ajax.ts b/src/internal/ajax/ajax.ts
index 4de5b3ff34..99b27edfc4 100644
--- a/src/internal/ajax/ajax.ts
+++ b/src/internal/ajax/ajax.ts
@@ -350,21 +350,53 @@ export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> {
 
       const { progressSubscriber, includeDownloadProgress = false, includeUploadProgress = false } = config;
 
+      /**
+       * Wires up an event handler that will emit an error when fired. Used
+       * for timeout and abort events.
+       * @param type The type of event we're treating as an error
+       * @param errorFactory A function that creates the type of error to emit.
+       */
+      const addErrorEvent = (type: string, errorFactory: () => any) => {
+        xhr.addEventListener(type, () => {
+          const error = errorFactory();
+          progressSubscriber?.error?.(error);
+          destination.error(error);
+        });
+      };
+
+      // If the request times out, handle errors appropriately.
+      addErrorEvent('timeout', () => new AjaxTimeoutError(xhr, _request));
+
+      // If the request aborts (due to a network disconnection or the like), handle
+      // it as an error.
+      addErrorEvent('abort', () => new AjaxError('aborted', xhr, _request));
+
+      /**
+       * Creates a response object to emit to the consumer.
+       * @param direction the direction related to the event. Prefixes the event `type` in the response
+       * object. So "upload_" for events related to uploading, and "download_" for events related to
+       * downloading.
+       * @param event the actual event object.
+       */
       const createResponse = (direction: 'upload' | 'download', event: ProgressEvent) =>
         new AjaxResponse<T>(event, xhr, _request, `${direction}_${event.type}`);
 
+      /**
+       * Wires up an event handler that emits a Response object to the consumer, used for
+       * all events that emit responses, loadstart, progress, and load.
+       * Note that download load handling is a bit different below, because it has
+       * more logic it needs to run.
+       * @param target The target, either the XHR itself or the Upload object.
+       * @param type The type of event to wire up
+       * @param direction The "direction", used to prefix the response object that is
+       * emitted to the consumer. (e.g. "upload_" or "download_")
+       */
       const addProgressEvent = (target: any, type: string, direction: 'upload' | 'download') => {
         target.addEventListener(type, (event: ProgressEvent) => {
           destination.next(createResponse(direction, event));
         });
       };
 
-      xhr.ontimeout = () => {
-        const timeoutError = new AjaxTimeoutError(xhr, _request);
-        progressSubscriber?.error?.(timeoutError);
-        destination.error(timeoutError);
-      };
-
       if (includeUploadProgress) {
         [LOADSTART, PROGRESS, LOAD].forEach((type) => addProgressEvent(xhr.upload, type, 'upload'));
       }