From b198e11092337ef7c5671fd6324def92f2112882 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Sun, 8 Oct 2017 21:48:06 +0200 Subject: [PATCH 1/2] fix($http): set correct xhrStatus in response when using 'timeout' This correctly sets "timeout" if a request fails because the timeout (numerical or $timeout) is exceeded, and "abort" if the request is aborted by resolving a promise that was passed in. --- src/ng/http.js | 5 ++++ src/ng/httpBackend.js | 27 +++++++++++++++------ src/ngMock/angular-mocks.js | 18 ++++++++++---- test/ng/httpBackendSpec.js | 41 +++++++++++++++++++++++++++++--- test/ng/httpSpec.js | 38 +++++++++++++++++++++++++---- test/ngMock/angular-mocksSpec.js | 2 +- 6 files changed, 111 insertions(+), 20 deletions(-) diff --git a/src/ng/http.js b/src/ng/http.js index efae76332372..c0a4054b1d9a 100644 --- a/src/ng/http.js +++ b/src/ng/http.js @@ -845,6 +845,11 @@ function $HttpProvider() { * See {@link $http#caching $http Caching} for more information. * - **timeout** – `{number|Promise}` – timeout in milliseconds, or {@link ng.$q promise} * that should abort the request when resolved. + * + * A numerical timeout or a promise returned from {@link ng.$timeout $timeout}, will set + * the `xhrStatus` in the {@link $http#$http-returns response} to "timeout", and any other + * resolved promise will set it to "abort", following standard XMLHttpRequest behavior. + * * - **withCredentials** - `{boolean}` - whether to set the `withCredentials` flag on the * XHR object. See [requests with credentials](https://developer.mozilla.org/docs/Web/HTTP/Access_control_CORS#Requests_with_credentials) * for more information. diff --git a/src/ng/httpBackend.js b/src/ng/httpBackend.js index 7e4cb6d75680..933ac7493a3a 100644 --- a/src/ng/httpBackend.js +++ b/src/ng/httpBackend.js @@ -70,6 +70,7 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc } else { var xhr = createXhr(method, url); + var abortedByTimeout = false; xhr.open(method, url, true); forEach(headers, function(value, key) { @@ -110,7 +111,7 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc }; var requestAborted = function() { - completeRequest(callback, -1, null, null, '', 'abort'); + completeRequest(callback, -1, null, null, '', abortedByTimeout ? 'timeout' : 'abort'); }; var requestTimeout = function() { @@ -120,11 +121,11 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc }; xhr.onerror = requestError; - xhr.onabort = requestAborted; xhr.ontimeout = requestTimeout; + xhr.onabort = requestAborted; forEach(eventHandlers, function(value, key) { - xhr.addEventListener(key, value); + xhr.addEventListener(key, value); }); forEach(uploadEventHandlers, function(value, key) { @@ -155,14 +156,26 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc xhr.send(isUndefined(post) ? null : post); } + // Since we are using xhr.abort() when a request times out, we have to set a flag that + // indicates to requestAborted if the request timed out or was aborted. + // + // http.timeout = numerical timeout timeout + // http.timeout = $timeout timeout + // http.timeout = promise abort + // xhr.abort() abort (The xhr object is normally inaccessible, but + // can be exposed with the xhrFactory) if (timeout > 0) { - var timeoutId = $browserDefer(timeoutRequest, timeout); + var timeoutId = $browserDefer(function() { + timeoutRequest('timeout'); + }, timeout); } else if (isPromiseLike(timeout)) { - timeout.then(timeoutRequest); + timeout.then(function() { + timeoutRequest(isDefined(timeout.$$timeoutId) ? 'timeout' : 'abort'); + }); } - - function timeoutRequest() { + function timeoutRequest(reason) { + abortedByTimeout = reason === 'timeout'; if (jsonpDone) { jsonpDone(); } diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index 8ee32d27a955..0e7120dddf45 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -1378,9 +1378,13 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) { function wrapResponse(wrapped) { if (!$browser && timeout) { if (timeout.then) { - timeout.then(handleTimeout); + timeout.then(function() { + handlePrematureEnd(angular.isDefined(timeout.$$timeoutId) ? 'timeout' : 'abort'); + }); } else { - $timeout(handleTimeout, timeout); + $timeout(function() { + handlePrematureEnd('timeout'); + }, timeout); } } @@ -1394,11 +1398,11 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) { copy(response[3] || ''), copy(response[4])); } - function handleTimeout() { + function handlePrematureEnd(reason) { for (var i = 0, ii = responses.length; i < ii; i++) { if (responses[i] === handleResponse) { responses.splice(i, 1); - callback(-1, undefined, '', undefined, 'timeout'); + callback(-1, undefined, '', undefined, reason); break; } } @@ -2110,7 +2114,11 @@ function MockXhr() { return lines.join('\n'); }; - this.abort = angular.noop; + this.abort = function() { + if (isFunction(this.onabort)) { + this.onabort(); + } + }; // This section simulates the events on a real XHR object (and the upload object) // When we are testing $httpBackend (inside the AngularJS project) we make partial use of this diff --git a/test/ng/httpBackendSpec.js b/test/ng/httpBackendSpec.js index adb76f60b097..cd18da263a97 100644 --- a/test/ng/httpBackendSpec.js +++ b/test/ng/httpBackendSpec.js @@ -244,7 +244,7 @@ describe('$httpBackend', function() { expect(callback).toHaveBeenCalledOnce(); }); - it('should abort request on timeout', function() { + it('should abort request on numerical timeout', function() { callback.and.callFake(function(status, response) { expect(status).toBe(-1); }); @@ -264,9 +264,10 @@ describe('$httpBackend', function() { }); - it('should abort request on timeout promise resolution', inject(function($timeout) { - callback.and.callFake(function(status, response) { + it('should abort request on $timeout promise resolution', inject(function($timeout) { + callback.and.callFake(function(status, response, headers, statusText, xhrStatus) { expect(status).toBe(-1); + expect(xhrStatus).toBe('timeout'); }); $backend('GET', '/url', null, callback, {}, $timeout(noop, 2000)); @@ -300,6 +301,24 @@ describe('$httpBackend', function() { })); + it('should abort request on canceler promise resolution', inject(function($q, $browser) { + var canceler = $q.defer(); + + callback.and.callFake(function(status, response, headers, statusText, xhrStatus) { + expect(status).toBe(-1); + expect(xhrStatus).toBe('abort'); + }); + + $backend('GET', '/url', null, callback, {}, canceler.promise); + xhr = MockXhr.$$lastInstance; + + canceler.resolve(); + $browser.defer.flush(); + + expect(callback).toHaveBeenCalledOnce(); + })); + + it('should cancel timeout on completion', function() { callback.and.callFake(function(status, response) { expect(status).toBe(200); @@ -320,6 +339,22 @@ describe('$httpBackend', function() { }); + it('should call callback with xhrStatus "abort" on explicit xhr.abort() when $timeout is set', inject(function($timeout) { + callback.and.callFake(function(status, response, headers, statusText, xhrStatus) { + expect(status).toBe(-1); + expect(xhrStatus).toBe('abort'); + }); + + $backend('GET', '/url', null, callback, {}, $timeout(noop, 2000)); + xhr = MockXhr.$$lastInstance; + spyOn(xhr, 'abort').and.callThrough(); + + xhr.abort(); + + expect(callback).toHaveBeenCalledOnce(); + })); + + it('should set withCredentials', function() { $backend('GET', '/some.url', null, callback, {}, null, true); expect(MockXhr.$$lastInstance.withCredentials).toBe(true); diff --git a/test/ng/httpSpec.js b/test/ng/httpSpec.js index aad004790842..73a5fccbe44c 100644 --- a/test/ng/httpSpec.js +++ b/test/ng/httpSpec.js @@ -2,6 +2,8 @@ /* global MockXhr: false */ +// The http specs run against the mocked httpBackend + describe('$http', function() { var callback, mockedCookies; @@ -1907,7 +1909,7 @@ describe('$http', function() { function(response) { expect(response.data).toBeUndefined(); expect(response.status).toBe(-1); - expect(response.xhrStatus).toBe('timeout'); + expect(response.xhrStatus).toBe('abort'); expect(response.headers()).toEqual(Object.create(null)); expect(response.config.url).toBe('/some'); callback(); @@ -1923,17 +1925,45 @@ describe('$http', function() { })); + it('should timeout request when numerical timeout is exceeded', inject(function($timeout) { + var onFulfilled = jasmine.createSpy('onFulfilled'); + var onRejected = jasmine.createSpy('onRejected').and.callFake(function(response) { + expect(response.xhrStatus).toBe('timeout'); + }); + + $httpBackend.expect('GET', '/some').respond(200); + + $http({ + method: 'GET', + url: '/some', + timeout: 10 + }).then(onFulfilled, onRejected); + + $timeout.flush(100); + + expect(onFulfilled).not.toHaveBeenCalled(); + expect(onRejected).toHaveBeenCalled(); + })); + + it('should reject promise when timeout promise resolves', inject(function($timeout) { var onFulfilled = jasmine.createSpy('onFulfilled'); - var onRejected = jasmine.createSpy('onRejected'); + var onRejected = jasmine.createSpy('onRejected').and.callFake(function(response) { + expect(response.xhrStatus).toBe('timeout'); + }); + $httpBackend.expect('GET', '/some').respond(200); - $http({method: 'GET', url: '/some', timeout: $timeout(noop, 10)}).then(onFulfilled, onRejected); + $http({ + method: 'GET', + url: '/some', + timeout: $timeout(noop, 10) + }).then(onFulfilled, onRejected); $timeout.flush(100); expect(onFulfilled).not.toHaveBeenCalled(); - expect(onRejected).toHaveBeenCalledOnce(); + expect(onRejected).toHaveBeenCalled(); })); }); diff --git a/test/ngMock/angular-mocksSpec.js b/test/ngMock/angular-mocksSpec.js index e3637f7c235f..43c39772a9ea 100644 --- a/test/ngMock/angular-mocksSpec.js +++ b/test/ngMock/angular-mocksSpec.js @@ -1669,7 +1669,7 @@ describe('ngMock', function() { canceler(); // simulate promise resolution - expect(callback).toHaveBeenCalledWith(-1, undefined, '', undefined, 'timeout'); + expect(callback).toHaveBeenCalledWith(-1, undefined, '', undefined, 'abort'); hb.verifyNoOutstandingExpectation(); hb.verifyNoOutstandingRequest(); }); From 2ac56fa2c856be26f9df1b72b272f9d7d99e766b Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Tue, 6 Mar 2018 18:50:19 +0100 Subject: [PATCH 2/2] docs($http): move response object documentation into Usage section --- src/ng/http.js | 86 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 51 insertions(+), 35 deletions(-) diff --git a/src/ng/http.js b/src/ng/http.js index c0a4054b1d9a..0e44977cfc8b 100644 --- a/src/ng/http.js +++ b/src/ng/http.js @@ -437,7 +437,9 @@ function $HttpProvider() { * * ## General usage * The `$http` service is a function which takes a single argument — a {@link $http#usage configuration object} — - * that is used to generate an HTTP request and returns a {@link ng.$q promise}. + * that is used to generate an HTTP request and returns a {@link ng.$q promise} that is + * resolved (request success) or rejected (request failure) with a + * {@link ng.$http#$http-returns response} object. * * ```js * // Simple GET request example: @@ -453,24 +455,6 @@ function $HttpProvider() { * }); * ``` * - * The response object has these properties: - * - * - **data** – `{string|Object}` – The response body transformed with the transform - * functions. - * - **status** – `{number}` – HTTP status code of the response. - * - **headers** – `{function([headerName])}` – Header getter function. - * - **config** – `{Object}` – The configuration object that was used to generate the request. - * - **statusText** – `{string}` – HTTP status text of the response. - * - **xhrStatus** – `{string}` – Status of the XMLHttpRequest (`complete`, `error`, `timeout` or `abort`). - * - * A response status code between 200 and 299 is considered a success status and will result in - * the success callback being called. Any response status code outside of that range is - * considered an error status and will result in the error callback being called. - * Also, status codes less than -1 are normalized to zero. -1 usually means the request was - * aborted, e.g. using a `config.timeout`. - * Note that if the response is a redirect, XMLHttpRequest will transparently follow it, meaning - * that the outcome (success or error) will be determined by the final response status code. - * * * ## Shortcut methods * @@ -856,8 +840,33 @@ function $HttpProvider() { * - **responseType** - `{string}` - see * [XMLHttpRequest.responseType](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest#xmlhttprequest-responsetype). * - * @returns {HttpPromise} Returns a {@link ng.$q `Promise}` that will be resolved to a response object - * when the request succeeds or fails. + * @returns {HttpPromise} A {@link ng.$q `Promise}` that will be resolved (request success) + * or rejected (request failure) with a response object. + * + * The response object has these properties: + * + * - **data** – `{string|Object}` – The response body transformed with + * the transform functions. + * - **status** – `{number}` – HTTP status code of the response. + * - **headers** – `{function([headerName])}` – Header getter function. + * - **config** – `{Object}` – The configuration object that was used + * to generate the request. + * - **statusText** – `{string}` – HTTP status text of the response. + * - **xhrStatus** – `{string}` – Status of the XMLHttpRequest + * (`complete`, `error`, `timeout` or `abort`). + * + * + * A response status code between 200 and 299 is considered a success status + * and will result in the success callback being called. Any response status + * code outside of that range is considered an error status and will result + * in the error callback being called. + * Also, status codes less than -1 are normalized to zero. -1 usually means + * the request was aborted, e.g. using a `config.timeout`. More information + * about the status might be available in the `xhrStatus` property. + * + * Note that if the response is a redirect, XMLHttpRequest will transparently + * follow it, meaning that the outcome (success or error) will be determined + * by the final response status code. * * * @property {Array.} pendingRequests Array of config objects for currently pending @@ -1107,8 +1116,9 @@ function $HttpProvider() { * * @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested; * or an object created by a call to `$sce.trustAsResourceUrl(url)`. - * @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage - * @returns {HttpPromise} Future object + * @param {Object=} config Optional configuration object. See {@link ng.$http#$http-arguments `$http()` arguments}. + * @returns {HttpPromise} A Promise that will be resolved or rejected with a response object. + * See {@link ng.$http#$http-returns `$http()` return value}. */ /** @@ -1120,8 +1130,9 @@ function $HttpProvider() { * * @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested; * or an object created by a call to `$sce.trustAsResourceUrl(url)`. - * @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage - * @returns {HttpPromise} Future object + * @param {Object=} config Optional configuration object. See {@link ng.$http#$http-arguments `$http()` arguments}. + * @returns {HttpPromise} A Promise that will be resolved or rejected with a response object. + * See {@link ng.$http#$http-returns `$http()` return value}. */ /** @@ -1133,8 +1144,9 @@ function $HttpProvider() { * * @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested; * or an object created by a call to `$sce.trustAsResourceUrl(url)`. - * @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage - * @returns {HttpPromise} Future object + * @param {Object=} config Optional configuration object. See {@link ng.$http#$http-arguments `$http()` arguments}. + * @returns {HttpPromise} A Promise that will be resolved or rejected with a response object. + * See {@link ng.$http#$http-returns `$http()` return value}. */ /** @@ -1175,8 +1187,9 @@ function $HttpProvider() { * * @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested; * or an object created by a call to `$sce.trustAsResourceUrl(url)`. - * @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage - * @returns {HttpPromise} Future object + * @param {Object=} config Optional configuration object. See {@link ng.$http#$http-arguments `$http()` arguments}. + * @returns {HttpPromise} A Promise that will be resolved or rejected with a response object. + * See {@link ng.$http#$http-returns `$http()` return value}. */ createShortMethods('get', 'delete', 'head', 'jsonp'); @@ -1189,8 +1202,9 @@ function $HttpProvider() { * * @param {string} url Relative or absolute URL specifying the destination of the request * @param {*} data Request content - * @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage - * @returns {HttpPromise} Future object + * @param {Object=} config Optional configuration object. See {@link ng.$http#$http-arguments `$http()` arguments}. + * @returns {HttpPromise} A Promise that will be resolved or rejected with a response object. + * See {@link ng.$http#$http-returns `$http()` return value}. */ /** @@ -1202,8 +1216,9 @@ function $HttpProvider() { * * @param {string} url Relative or absolute URL specifying the destination of the request * @param {*} data Request content - * @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage - * @returns {HttpPromise} Future object + * @param {Object=} config Optional configuration object. See {@link ng.$http#$http-arguments `$http()` arguments}. + * @returns {HttpPromise} A Promise that will be resolved or rejected with a response object. + * See {@link ng.$http#$http-returns `$http()` return value}. */ /** @@ -1215,8 +1230,9 @@ function $HttpProvider() { * * @param {string} url Relative or absolute URL specifying the destination of the request * @param {*} data Request content - * @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage - * @returns {HttpPromise} Future object + * @param {Object=} config Optional configuration object. See {@link ng.$http#$http-arguments `$http()` arguments}. + * @returns {HttpPromise} A Promise that will be resolved or rejected with a response object. + * See {@link ng.$http#$http-returns `$http()` return value}. */ createShortMethodsWithData('post', 'put', 'patch');