From bc7dc5233c870470caaf2a8e9b9257b226bfd26c Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Sun, 31 Jul 2016 16:20:45 +0300 Subject: [PATCH] fix($httpBackend): complete the request on timeout 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, `$httpBackend` would fail to capture the XHR's completing (with an error), so the corresponding `$http` promise would never get fulfilled. Fixes #14969 --- src/ng/httpBackend.js | 1 + test/ng/httpBackendSpec.js | 38 ++++++++++++++++++++++++-------------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/ng/httpBackend.js b/src/ng/httpBackend.js index 51c87ab57ed1..d686a118f387 100644 --- a/src/ng/httpBackend.js +++ b/src/ng/httpBackend.js @@ -108,6 +108,7 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc xhr.onerror = requestError; xhr.onabort = requestError; + xhr.ontimeout = requestError; forEach(eventHandlers, function(value, key) { xhr.addEventListener(key, value); diff --git a/test/ng/httpBackendSpec.js b/test/ng/httpBackendSpec.js index bf27cf290a02..b282119e9d1e 100644 --- a/test/ng/httpBackendSpec.js +++ b/test/ng/httpBackendSpec.js @@ -155,10 +155,11 @@ describe('$httpBackend', function() { }); it('should not try to read response data when request is aborted', function() { - callback.and.callFake(function(status, response, headers) { + callback.and.callFake(function(status, response, headers, statusText) { expect(status).toBe(-1); expect(response).toBe(null); expect(headers).toBe(null); + expect(statusText).toBe(''); }); $backend('GET', '/url', null, callback, {}, 2000); xhr = MockXhr.$$lastInstance; @@ -172,6 +173,22 @@ describe('$httpBackend', function() { expect(callback).toHaveBeenCalledOnce(); }); + it('should complete the request on timeout', function() { + callback.and.callFake(function(status, response, headers, statusText) { + expect(status).toBe(-1); + expect(response).toBe(null); + expect(headers).toBe(null); + expect(statusText).toBe(''); + }); + $backend('GET', '/url', null, callback, {}); + xhr = MockXhr.$$lastInstance; + + expect(callback).not.toHaveBeenCalled(); + + xhr.ontimeout(); + expect(callback).toHaveBeenCalledOnce(); + }); + it('should abort request on timeout', function() { callback.and.callFake(function(status, response) { expect(status).toBe(-1); @@ -253,6 +270,7 @@ describe('$httpBackend', function() { expect(MockXhr.$$lastInstance.withCredentials).toBe(true); }); + it('should call $xhrFactory with method and url', function() { var mockXhrFactory = jasmine.createSpy('mockXhrFactory').and.callFake(createMockXhr); $backend = createHttpBackend($browser, mockXhrFactory, $browser.defer, $jsonpCallbacks, fakeDocument); @@ -391,6 +409,7 @@ describe('$httpBackend', function() { // TODO(vojta): test whether it fires "async-end" on both success and error }); + describe('protocols that return 0 status code', function() { function respond(status, content) { @@ -400,10 +419,12 @@ describe('$httpBackend', function() { xhr.onload(); } - - it('should convert 0 to 200 if content and file protocol', function() { + beforeEach(function() { $backend = createHttpBackend($browser, createMockXhr); + }); + + it('should convert 0 to 200 if content and file protocol', function() { $backend('GET', 'file:///whatever/index.html', null, callback); respond(0, 'SOME CONTENT'); @@ -412,8 +433,6 @@ describe('$httpBackend', function() { }); it('should convert 0 to 200 if content for protocols other than file', function() { - $backend = createHttpBackend($browser, createMockXhr); - $backend('GET', 'someProtocol:///whatever/index.html', null, callback); respond(0, 'SOME CONTENT'); @@ -422,8 +441,6 @@ describe('$httpBackend', function() { }); it('should convert 0 to 404 if no content and file protocol', function() { - $backend = createHttpBackend($browser, createMockXhr); - $backend('GET', 'file:///whatever/index.html', null, callback); respond(0, ''); @@ -432,8 +449,6 @@ describe('$httpBackend', function() { }); it('should not convert 0 to 404 if no content for protocols other than file', function() { - $backend = createHttpBackend($browser, createMockXhr); - $backend('GET', 'someProtocol:///whatever/index.html', null, callback); respond(0, ''); @@ -460,8 +475,6 @@ describe('$httpBackend', function() { try { - $backend = createHttpBackend($browser, createMockXhr); - $backend('GET', '/whatever/index.html', null, callback); respond(0, ''); @@ -473,10 +486,7 @@ describe('$httpBackend', function() { } }); - it('should return original backend status code if different from 0', function() { - $backend = createHttpBackend($browser, createMockXhr); - // request to http:// $backend('POST', 'http://rest_api/create_whatever', null, callback); respond(201, '');