diff --git a/src/ng/http.js b/src/ng/http.js index 708a065058ab..fbdc89a50cf0 100644 --- a/src/ng/http.js +++ b/src/ng/http.js @@ -548,7 +548,8 @@ function $HttpProvider() { * GET request, otherwise if a cache instance built with * {@link ng.$cacheFactory $cacheFactory}, this cache will be used for * caching. - * - **timeout** – `{number}` – timeout in milliseconds. + * - **timeout** – `{number|Promise}` – timeout in milliseconds, or {@link ng.$q promise} + * that should abort the request when resolved. * - **withCredentials** - `{boolean}` - whether to to set the `withCredentials` flag on the * XHR object. See {@link https://developer.mozilla.org/en/http_access_control#section_5 * requests with credentials} for more information. @@ -927,7 +928,7 @@ function $HttpProvider() { } resolvePromise(response, status, headersString); - $rootScope.$apply(); + if (!$rootScope.$$phase) $rootScope.$apply(); } diff --git a/src/ng/httpBackend.js b/src/ng/httpBackend.js index 69ac597648be..ed8404f96cfb 100644 --- a/src/ng/httpBackend.js +++ b/src/ng/httpBackend.js @@ -107,20 +107,25 @@ function createHttpBackend($browser, XHR, $browserDefer, callbacks, rawDocument, } if (timeout > 0) { - var timeoutId = $browserDefer(function() { - status = -1; - jsonpDone && jsonpDone(); - xhr && xhr.abort(); - }, timeout); + var timeoutId = $browserDefer(timeoutRequest, timeout); + } else if (timeout && timeout.then) { + timeout.then(timeoutRequest); } + function timeoutRequest() { + status = -1; + jsonpDone && jsonpDone(); + xhr && xhr.abort(); + } + function completeRequest(callback, status, response, headersString) { // URL_MATCH is defined in src/service/location.js var protocol = (url.match(SERVER_MATCH) || ['', locationProtocol])[1]; - // cancel timeout + // cancel timeout and subsequent timeout promise resolution timeoutId && $browserDefer.cancel(timeoutId); + jsonpDone = xhr = null; // fix status code for file protocol (it's always 0) status = (protocol == 'file') ? (response ? 200 : 404) : status; diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index 076738027213..be71e326bd3b 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -937,7 +937,7 @@ function createHttpBackendMock($rootScope, $delegate, $browser) { } // TODO(vojta): change params to: method, url, data, headers, callback - function $httpBackend(method, url, data, callback, headers) { + function $httpBackend(method, url, data, callback, headers, timeout) { var xhr = new MockXhr(), expectation = expectations[0], wasExpected = false; @@ -948,6 +948,28 @@ function createHttpBackendMock($rootScope, $delegate, $browser) { : angular.toJson(data); } + function wrapResponse(wrapped) { + if (!$browser && timeout && timeout.then) timeout.then(handleTimeout); + + return handleResponse; + + function handleResponse() { + var response = wrapped.response(method, url, data, headers); + xhr.$$respHeaders = response[2]; + callback(response[0], response[1], xhr.getAllResponseHeaders()); + } + + function handleTimeout() { + for (var i = 0, ii = responses.length; i < ii; i++) { + if (responses[i] === handleResponse) { + responses.splice(i, 1); + callback(-1, undefined, ''); + break; + } + } + } + } + if (expectation && expectation.match(method, url)) { if (!expectation.matchData(data)) throw Error('Expected ' + expectation + ' with different data\n' + @@ -961,11 +983,7 @@ function createHttpBackendMock($rootScope, $delegate, $browser) { expectations.shift(); if (expectation.response) { - responses.push(function() { - var response = expectation.response(method, url, data, headers); - xhr.$$respHeaders = response[2]; - callback(response[0], response[1], xhr.getAllResponseHeaders()); - }); + responses.push(wrapResponse(expectation)); return; } wasExpected = true; @@ -976,13 +994,9 @@ function createHttpBackendMock($rootScope, $delegate, $browser) { if (definition.match(method, url, data, headers || {})) { if (definition.response) { // if $browser specified, we do auto flush all requests - ($browser ? $browser.defer : responsesPush)(function() { - var response = definition.response(method, url, data, headers); - xhr.$$respHeaders = response[2]; - callback(response[0], response[1], xhr.getAllResponseHeaders()); - }); + ($browser ? $browser.defer : responsesPush)(wrapResponse(definition)); } else if (definition.passThrough) { - $delegate(method, url, data, callback, headers); + $delegate(method, url, data, callback, headers, timeout); } else throw Error('No response defined !'); return; } diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js index c165ffab1422..59ac3b3a2723 100644 --- a/src/ngResource/resource.js +++ b/src/ngResource/resource.js @@ -85,7 +85,8 @@ * GET request, otherwise if a cache instance built with * {@link ng.$cacheFactory $cacheFactory}, this cache will be used for * caching. - * - **`timeout`** – `{number}` – timeout in milliseconds. + * - **`timeout`** – `{number|Promise}` – timeout in milliseconds, or {@link ng.$q promise} that + * should abort the request when resolved. * - **`withCredentials`** - `{boolean}` - whether to to set the `withCredentials` flag on the * XHR object. See {@link https://developer.mozilla.org/en/http_access_control#section_5 * requests with credentials} for more information. diff --git a/test/ng/httpBackendSpec.js b/test/ng/httpBackendSpec.js index da4fed168de7..c65ab2e17c5b 100644 --- a/test/ng/httpBackendSpec.js +++ b/test/ng/httpBackendSpec.js @@ -117,6 +117,44 @@ describe('$httpBackend', function() { }); + it('should abort request on timeout promise resolution', inject(function($timeout) { + callback.andCallFake(function(status, response) { + expect(status).toBe(-1); + }); + + $backend('GET', '/url', null, callback, {}, $timeout(noop, 2000)); + xhr = MockXhr.$$lastInstance; + spyOn(xhr, 'abort'); + + $timeout.flush(); + expect(xhr.abort).toHaveBeenCalledOnce(); + + xhr.status = 0; + xhr.readyState = 4; + xhr.onreadystatechange(); + expect(callback).toHaveBeenCalledOnce(); + })); + + + it('should not abort resolved request on timeout promise resolution', inject(function($timeout) { + callback.andCallFake(function(status, response) { + expect(status).toBe(200); + }); + + $backend('GET', '/url', null, callback, {}, $timeout(noop, 2000)); + xhr = MockXhr.$$lastInstance; + spyOn(xhr, 'abort'); + + xhr.status = 200; + xhr.readyState = 4; + xhr.onreadystatechange(); + expect(callback).toHaveBeenCalledOnce(); + + $timeout.flush(); + expect(xhr.abort).not.toHaveBeenCalled(); + })); + + it('should cancel timeout on completion', function() { callback.andCallFake(function(status, response) { expect(status).toBe(200); diff --git a/test/ng/httpSpec.js b/test/ng/httpSpec.js index 5984106c3325..4ddb36611ab0 100644 --- a/test/ng/httpSpec.js +++ b/test/ng/httpSpec.js @@ -1273,6 +1273,33 @@ describe('$http', function() { }); + describe('timeout', function() { + + it('should abort requests when timeout promise resolves', inject(function($q) { + var canceler = $q.defer(); + + $httpBackend.expect('GET', '/some').respond(200); + + $http({method: 'GET', url: '/some', timeout: canceler.promise}).error( + function(data, status, headers, config) { + expect(data).toBeUndefined(); + expect(status).toBe(0); + expect(headers()).toEqual({}); + expect(config.url).toBe('/some'); + callback(); + }); + + $rootScope.$apply(function() { + canceler.resolve(); + }); + + expect(callback).toHaveBeenCalled(); + $httpBackend.verifyNoOutstandingExpectation(); + $httpBackend.verifyNoOutstandingRequest(); + })); + }); + + describe('pendingRequests', function() { it('should be an array of pending requests', function() { diff --git a/test/ngMock/angular-mocksSpec.js b/test/ngMock/angular-mocksSpec.js index 176c5c920ea4..220f8e58a746 100644 --- a/test/ngMock/angular-mocksSpec.js +++ b/test/ngMock/angular-mocksSpec.js @@ -798,6 +798,24 @@ describe('ngMock', function() { }); + it('should abort requests when timeout promise resolves', function() { + hb.expect('GET', '/url1').respond(200); + + var canceler, then = jasmine.createSpy('then').andCallFake(function(fn) { + canceler = fn; + }); + + hb('GET', '/url1', null, callback, null, {then: then}); + expect(typeof canceler).toBe('function'); + + canceler(); // simulate promise resolution + + expect(callback).toHaveBeenCalledWith(-1, undefined, ''); + hb.verifyNoOutstandingExpectation(); + hb.verifyNoOutstandingRequest(); + }); + + it('should throw an exception if no response defined', function() { hb.when('GET', '/test'); expect(function() { @@ -1006,8 +1024,8 @@ describe('ngMockE2E', function() { hb.when('GET', /\/passThrough\/.*/).passThrough(); hb('GET', '/passThrough/23', null, callback); - expect(realHttpBackend). - toHaveBeenCalledOnceWith('GET', '/passThrough/23', null, callback, undefined); + expect(realHttpBackend).toHaveBeenCalledOnceWith( + 'GET', '/passThrough/23', null, callback, undefined, undefined); }); });