fix(http): complete the request on timeout (#39807)

arturovt · alxhub · commit 61a0b6de6d17 · 2021-02-09T14:44:17.000-08:00
When using the [timeout attribute](https://xhr.spec.whatwg.org/#the-timeout-attribute) and an XHR request times out, browsers trigger the `timeout` event (and execute the XHR's `ontimeout` callback). Additionally, Safari 9 handles timed-out requests in the same way, even if no `timeout` has been explicitly set on the XHR. In the above cases, `HttpClient` would fail to capture the XHR's completing (with an error), so the corresponding `Observable` would never complete. PR Close #26453 PR Close #39807
diff --git a/packages/common/http/src/xhr.ts b/packages/common/http/src/xhr.ts
@@ -311,6 +311,7 @@ export class HttpXhrBackend implements HttpBackend {
       // By default, register for load and error events.
       xhr.addEventListener('load', onLoad);
       xhr.addEventListener('error', onError);
+      xhr.addEventListener('timeout', onError);
 
       // Progress events are only enabled if requested.
       if (req.reportProgress) {
@@ -333,6 +334,7 @@ export class HttpXhrBackend implements HttpBackend {
         // On a cancellation, remove all registered event listeners.
         xhr.removeEventListener('error', onError);
         xhr.removeEventListener('load', onLoad);
+        xhr.removeEventListener('timeout', onError);
         if (req.reportProgress) {
           xhr.removeEventListener('progress', onDownProgress);
           if (reqBody !== null && xhr.upload) {
diff --git a/packages/common/http/test/xhr_mock.ts b/packages/common/http/test/xhr_mock.ts
@@ -54,6 +54,7 @@ export class MockXMLHttpRequest {
 
   listeners: {
     error?: (event: ErrorEvent) => void,
+    timeout?: (event: ErrorEvent) => void,
     load?: () => void,
     progress?: (event: ProgressEvent) => void,
     uploadProgress?: (event: ProgressEvent) => void,
@@ -70,11 +71,12 @@ export class MockXMLHttpRequest {
     this.body = body;
   }
 
-  addEventListener(event: 'error'|'load'|'progress'|'uploadProgress', handler: Function): void {
+  addEventListener(event: 'error'|'timeout'|'load'|'progress'|'uploadProgress', handler: Function):
+      void {
     this.listeners[event] = handler as any;
   }
 
-  removeEventListener(event: 'error'|'load'|'progress'|'uploadProgress'): void {
+  removeEventListener(event: 'error'|'timeout'|'load'|'progress'|'uploadProgress'): void {
     delete this.listeners[event];
   }
 
@@ -129,6 +131,12 @@ export class MockXMLHttpRequest {
     }
   }
 
+  mockTimeoutEvent(error: any): void {
+    if (this.listeners.timeout) {
+      this.listeners.timeout(error);
+    }
+  }
+
   abort() {
     this.mockAborted = true;
   }
diff --git a/packages/common/http/test/xhr_spec.ts b/packages/common/http/test/xhr_spec.ts
@@ -9,7 +9,7 @@
 import {HttpRequest} from '@angular/common/http/src/request';
 import {HttpDownloadProgressEvent, HttpErrorResponse, HttpEvent, HttpEventType, HttpHeaderResponse, HttpResponse, HttpResponseBase, HttpStatusCode, HttpUploadProgressEvent} from '@angular/common/http/src/response';
 import {HttpXhrBackend} from '@angular/common/http/src/xhr';
-import {describe, it} from '@angular/core/testing/src/testing_internal';
+import {describe, expect, it} from '@angular/core/testing/src/testing_internal';
 import {Observable} from 'rxjs';
 import {toArray} from 'rxjs/operators';
 
@@ -151,6 +151,17 @@ const XSSI_PREFIX = ')]}\'\n';
       });
       factory.mock.mockErrorEvent(new Error('blah'));
     });
+    it('emits timeout if the request times out', done => {
+      backend.handle(TEST_POST).subscribe({
+        error: (error: HttpErrorResponse) => {
+          expect(error instanceof HttpErrorResponse).toBeTrue();
+          expect(error.error instanceof Error).toBeTrue();
+          expect(error.url).toBe('/test');
+          done();
+        },
+      });
+      factory.mock.mockTimeoutEvent(new Error('timeout'));
+    });
     it('avoids abort a request when fetch operation is completed', done => {
       const abort = jasmine.createSpy('abort');
 

Original file line number	Diff line number	Diff line change
`@@ -54,6 +54,7 @@ export class MockXMLHttpRequest {`
`54`	`54`
`55`	`55`	`listeners: {`
`56`	`56`	`error?: (event: ErrorEvent) => void,`
	`57`	`+ timeout?: (event: ErrorEvent) => void,`
`57`	`58`	`load?: () => void,`
`58`	`59`	`progress?: (event: ProgressEvent) => void,`
`59`	`60`	`uploadProgress?: (event: ProgressEvent) => void,`
`@@ -70,11 +71,12 @@ export class MockXMLHttpRequest {`
`70`	`71`	`this.body = body;`
`71`	`72`	`}`
`72`	`73`
`73`		`- addEventListener(event: 'error'\|'load'\|'progress'\|'uploadProgress', handler: Function): void {`
	`74`	`+ addEventListener(event: 'error'\|'timeout'\|'load'\|'progress'\|'uploadProgress', handler: Function):`
	`75`	`+ void {`
`74`	`76`	`this.listeners[event] = handler as any;`
`75`	`77`	`}`
`76`	`78`
`77`		`- removeEventListener(event: 'error'\|'load'\|'progress'\|'uploadProgress'): void {`
	`79`	`+ removeEventListener(event: 'error'\|'timeout'\|'load'\|'progress'\|'uploadProgress'): void {`
`78`	`80`	`delete this.listeners[event];`
`79`	`81`	`}`
`80`	`82`
`@@ -129,6 +131,12 @@ export class MockXMLHttpRequest {`
`129`	`131`	`}`
`130`	`132`	`}`
`131`	`133`
	`134`	`+ mockTimeoutEvent(error: any): void {`
	`135`	`+ if (this.listeners.timeout) {`
	`136`	`+ this.listeners.timeout(error);`
	`137`	`+ }`
	`138`	`+ }`
	`139`	`+`
`132`	`140`	`abort() {`
`133`	`141`	`this.mockAborted = true;`
`134`	`142`	`}`