From 61a0b6de6d17aa5e761dd02876f6e1537dcf3895 Mon Sep 17 00:00:00 2001 From: arturovt Date: Mon, 23 Nov 2020 00:05:39 +0200 Subject: [PATCH] fix(http): complete the request on timeout (#39807) 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 --- packages/common/http/src/xhr.ts | 2 ++ packages/common/http/test/xhr_mock.ts | 12 ++++++++++-- packages/common/http/test/xhr_spec.ts | 13 ++++++++++++- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/packages/common/http/src/xhr.ts b/packages/common/http/src/xhr.ts index 056f516b17..eeb66d1dc4 100644 --- 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 index 785a9aabae..26766ffa4b 100644 --- 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 index d645be91e3..b422b20f6b 100644 --- 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');