Skip to content

Commit

Permalink
feat($http): allow differentiation between XHR completion, error, abo…
Browse files Browse the repository at this point in the history
…rt, timeout

Previously, it wasn't possible to tell if an `$http`-initiated XMLHttpRequest
was completed normally or with an error or it was aborted or timed out.
This commit adds a new property on the `response` object (`xhrStatus`) which
allows to defferentiate between the possible statuses.

Fixes angular#15924

Closes angular#15847
  • Loading branch information
frederikprijck authored and gkalpak committed Jul 4, 2017
1 parent 122d89b commit e872f0e
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 40 deletions.
18 changes: 10 additions & 8 deletions src/ng/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ function $HttpProvider() {
* - **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
Expand Down Expand Up @@ -1294,9 +1295,9 @@ function $HttpProvider() {
} else {
// serving from cache
if (isArray(cachedResp)) {
resolvePromise(cachedResp[1], cachedResp[0], shallowCopy(cachedResp[2]), cachedResp[3]);
resolvePromise(cachedResp[1], cachedResp[0], shallowCopy(cachedResp[2]), cachedResp[3], cachedResp[4]);
} else {
resolvePromise(cachedResp, 200, {}, 'OK');
resolvePromise(cachedResp, 200, {}, 'OK', 'complete');
}
}
} else {
Expand Down Expand Up @@ -1353,18 +1354,18 @@ function $HttpProvider() {
* - resolves the raw $http promise
* - calls $apply
*/
function done(status, response, headersString, statusText) {
function done(status, response, headersString, statusText, xhrStatus) {
if (cache) {
if (isSuccess(status)) {
cache.put(url, [status, response, parseHeaders(headersString), statusText]);
cache.put(url, [status, response, parseHeaders(headersString), statusText, xhrStatus]);
} else {
// remove promise from the cache
cache.remove(url);
}
}

function resolveHttpPromise() {
resolvePromise(response, status, headersString, statusText);
resolvePromise(response, status, headersString, statusText, xhrStatus);
}

if (useApplyAsync) {
Expand All @@ -1379,7 +1380,7 @@ function $HttpProvider() {
/**
* Resolves the raw $http promise.
*/
function resolvePromise(response, status, headers, statusText) {
function resolvePromise(response, status, headers, statusText, xhrStatus) {
//status: HTTP response status code, 0, -1 (aborted by timeout / promise)
status = status >= -1 ? status : 0;

Expand All @@ -1388,12 +1389,13 @@ function $HttpProvider() {
status: status,
headers: headersGetter(headers),
config: config,
statusText: statusText
statusText: statusText,
xhrStatus: xhrStatus
});
}

function resolvePromiseWithResult(result) {
resolvePromise(result.data, result.status, shallowCopy(result.headers()), result.statusText);
resolvePromise(result.data, result.status, shallowCopy(result.headers()), result.statusText, result.xhrStatus);
}

function removePendingReq() {
Expand Down
25 changes: 18 additions & 7 deletions src/ng/httpBackend.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
var jsonpDone = jsonpReq(url, callbackPath, function(status, text) {
// jsonpReq only ever sets status to 200 (OK), 404 (ERROR) or -1 (WAITING)
var response = (status === 200) && callbacks.getResponse(callbackPath);
completeRequest(callback, status, response, '', text);
completeRequest(callback, status, response, '', text, 'complete');
callbacks.removeCallback(callbackPath);
});
} else {
Expand Down Expand Up @@ -99,18 +99,29 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
status,
response,
xhr.getAllResponseHeaders(),
statusText);
statusText,
'complete');
};

var requestError = function() {
// The response is always empty
// See https://xhr.spec.whatwg.org/#request-error-steps and https://fetch.spec.whatwg.org/#concept-network-error
completeRequest(callback, -1, null, null, '');
completeRequest(callback, -1, null, null, '', 'error');
};

var requestAborted = function() {
completeRequest(callback, -1, null, null, '', 'abort');
};

var requestTimeout = function() {
// The response is always empty
// See https://xhr.spec.whatwg.org/#request-error-steps and https://fetch.spec.whatwg.org/#concept-network-error
completeRequest(callback, -1, null, null, '', 'timeout');
};

xhr.onerror = requestError;
xhr.onabort = requestError;
xhr.ontimeout = requestError;
xhr.onabort = requestAborted;
xhr.ontimeout = requestTimeout;

forEach(eventHandlers, function(value, key) {
xhr.addEventListener(key, value);
Expand Down Expand Up @@ -160,14 +171,14 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
}
}

function completeRequest(callback, status, response, headersString, statusText) {
function completeRequest(callback, status, response, headersString, statusText, xhrStatus) {
// cancel timeout and subsequent timeout promise resolution
if (isDefined(timeoutId)) {
$browserDefer.cancel(timeoutId);
}
jsonpDone = xhr = null;

callback(status, response, headersString, statusText);
callback(status, response, headersString, statusText, xhrStatus);
}
};

Expand Down
8 changes: 4 additions & 4 deletions src/ngMock/angular-mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -1354,8 +1354,8 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {

return function() {
return angular.isNumber(status)
? [status, data, headers, statusText]
: [200, status, data, headers];
? [status, data, headers, statusText, 'complete']
: [200, status, data, headers, 'complete'];
};
}

Expand Down Expand Up @@ -1391,14 +1391,14 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
var response = wrapped.response(method, url, data, headers, wrapped.params(url));
xhr.$$respHeaders = response[2];
callback(copy(response[0]), copy(response[1]), xhr.getAllResponseHeaders(),
copy(response[3] || ''));
copy(response[3] || ''), copy(response[4]));
}

function handleTimeout() {
for (var i = 0, ii = responses.length; i < ii; i++) {
if (responses[i] === handleResponse) {
responses.splice(i, 1);
callback(-1, undefined, '');
callback(-1, undefined, '', undefined, 'timeout');
break;
}
}
Expand Down
57 changes: 56 additions & 1 deletion test/ng/httpBackendSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,12 @@ describe('$httpBackend', function() {
});

it('should complete the request on timeout', function() {
callback.and.callFake(function(status, response, headers, statusText) {
callback.and.callFake(function(status, response, headers, statusText, xhrStatus) {
expect(status).toBe(-1);
expect(response).toBe(null);
expect(headers).toBe(null);
expect(statusText).toBe('');
expect(xhrStatus).toBe('timeout');
});
$backend('GET', '/url', null, callback, {});
xhr = MockXhr.$$lastInstance;
Expand All @@ -189,6 +190,60 @@ describe('$httpBackend', function() {
expect(callback).toHaveBeenCalledOnce();
});

it('should complete the request on abort', function() {
callback.and.callFake(function(status, response, headers, statusText, xhrStatus) {
expect(status).toBe(-1);
expect(response).toBe(null);
expect(headers).toBe(null);
expect(statusText).toBe('');
expect(xhrStatus).toBe('abort');
});
$backend('GET', '/url', null, callback, {});
xhr = MockXhr.$$lastInstance;

expect(callback).not.toHaveBeenCalled();

xhr.onabort();
expect(callback).toHaveBeenCalledOnce();
});

it('should complete the request on error', function() {
callback.and.callFake(function(status, response, headers, statusText, xhrStatus) {
expect(status).toBe(-1);
expect(response).toBe(null);
expect(headers).toBe(null);
expect(statusText).toBe('');
expect(xhrStatus).toBe('error');
});
$backend('GET', '/url', null, callback, {});
xhr = MockXhr.$$lastInstance;

expect(callback).not.toHaveBeenCalled();

xhr.onerror();
expect(callback).toHaveBeenCalledOnce();
});

it('should complete the request on success', function() {
callback.and.callFake(function(status, response, headers, statusText, xhrStatus) {
expect(status).toBe(200);
expect(response).toBe('response');
expect(headers).toBe('');
expect(statusText).toBe('');
expect(xhrStatus).toBe('complete');
});
$backend('GET', '/url', null, callback, {});
xhr = MockXhr.$$lastInstance;

expect(callback).not.toHaveBeenCalled();

xhr.statusText = '';
xhr.response = 'response';
xhr.status = 200;
xhr.onload();
expect(callback).toHaveBeenCalledOnce();
});

it('should abort request on timeout', function() {
callback.and.callFake(function(status, response) {
expect(status).toBe(-1);
Expand Down
34 changes: 34 additions & 0 deletions test/ng/httpSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,28 @@ describe('$http', function() {
expect(callback).toHaveBeenCalledOnce();
});

it('should pass xhrStatus in response object when a request is successful', function() {
$httpBackend.expect('GET', '/url').respond(200, 'SUCCESS', {}, 'OK');
$http({url: '/url', method: 'GET'}).then(function(response) {
expect(response.xhrStatus).toBe('complete');
callback();
});

$httpBackend.flush();
expect(callback).toHaveBeenCalledOnce();
});

it('should pass xhrStatus in response object when a request fails', function() {
$httpBackend.expect('GET', '/url').respond(404, 'ERROR', {}, 'Not Found');
$http({url: '/url', method: 'GET'}).then(null, function(response) {
expect(response.xhrStatus).toBe('complete');
callback();
});

$httpBackend.flush();
expect(callback).toHaveBeenCalledOnce();
});


it('should pass in the response object when a request failed', function() {
$httpBackend.expect('GET', '/url').respond(543, 'bad error', {'request-id': '123'});
Expand Down Expand Up @@ -1623,6 +1645,17 @@ describe('$http', function() {
expect(callback).toHaveBeenCalledOnce();
}));

it('should cache xhrStatus as well', inject(function($rootScope) {
doFirstCacheRequest('GET', 201, null);
callback.and.callFake(function(response) {
expect(response.xhrStatus).toBe('complete');
});

$http({method: 'get', url: '/url', cache: cache}).then(callback);
$rootScope.$digest();
expect(callback).toHaveBeenCalledOnce();
}));


it('should use cache even if second request was made before the first returned', function() {
$httpBackend.expect('GET', '/url').respond(201, 'fake-response');
Expand Down Expand Up @@ -1788,6 +1821,7 @@ describe('$http', function() {
function(response) {
expect(response.data).toBeUndefined();
expect(response.status).toBe(-1);
expect(response.xhrStatus).toBe('timeout');
expect(response.headers()).toEqual(Object.create(null));
expect(response.config.url).toBe('/some');
callback();
Expand Down
Loading

0 comments on commit e872f0e

Please sign in to comment.