Skip to content

Commit 7e2e235

Browse files
NarretzNarretz
committed
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.
1 parent 4f4ad3c commit 7e2e235

File tree

6 files changed

+111
-20
lines changed

6 files changed

+111
-20
lines changed

‎src/ng/http.js

+5
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,11 @@ function $HttpProvider() {
845845
* See {@link $http#caching $http Caching} for more information.
846846
* - **timeout** – `{number|Promise}` – timeout in milliseconds, or {@link ng.$q promise}
847847
* that should abort the request when resolved.
848+
*
849+
* A numerical timeout or a promise returned from {@link ng.$timeout $timeout}, will set
850+
* the `xhrStatus` in the {@link $http#$http-returns response} to "timeout", and any other
851+
* resolved promise will set it to "abort", following standard XMLHttpRequest behavior.
852+
*
848853
* - **withCredentials** - `{boolean}` - whether to set the `withCredentials` flag on the
849854
* XHR object. See [requests with credentials](https://developer.mozilla.org/docs/Web/HTTP/Access_control_CORS#Requests_with_credentials)
850855
* for more information.

‎src/ng/httpBackend.js

+20-7
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
7070
} else {
7171

7272
var xhr = createXhr(method, url);
73+
var abortedByTimeout = false;
7374

7475
xhr.open(method, url, true);
7576
forEach(headers, function(value, key) {
@@ -110,7 +111,7 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
110111
};
111112

112113
var requestAborted = function() {
113-
completeRequest(callback, -1, null, null, '', 'abort');
114+
completeRequest(callback, -1, null, null, '', abortedByTimeout ? 'timeout' : 'abort');
114115
};
115116

116117
var requestTimeout = function() {
@@ -120,11 +121,11 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
120121
};
121122

122123
xhr.onerror = requestError;
123-
xhr.onabort = requestAborted;
124124
xhr.ontimeout = requestTimeout;
125+
xhr.onabort = requestAborted;
125126

126127
forEach(eventHandlers, function(value, key) {
127-
xhr.addEventListener(key, value);
128+
xhr.addEventListener(key, value);
128129
});
129130

130131
forEach(uploadEventHandlers, function(value, key) {
@@ -155,14 +156,26 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
155156
xhr.send(isUndefined(post) ? null : post);
156157
}
157158

159+
// Since we are using xhr.abort() when a request times out, we have to set a flag that
160+
// indicates to requestAborted if the request timed out or was aborted.
161+
//
162+
// http.timeout = numerical timeout timeout
163+
// http.timeout = $timeout timeout
164+
// http.timeout = promise abort
165+
// xhr.abort() abort (The xhr object is normally inaccessible, but
166+
// can be exposed with the xhrFactory)
158167
if (timeout > 0) {
159-
var timeoutId = $browserDefer(timeoutRequest, timeout);
168+
var timeoutId = $browserDefer(function() {
169+
timeoutRequest('timeout');
170+
}, timeout);
160171
} else if (isPromiseLike(timeout)) {
161-
timeout.then(timeoutRequest);
172+
timeout.then(function() {
173+
timeoutRequest(isDefined(timeout.$$timeoutId) ? 'timeout' : 'abort');
174+
});
162175
}
163176

164-
165-
function timeoutRequest() {
177+
function timeoutRequest(reason) {
178+
abortedByTimeout = reason === 'timeout';
166179
if (jsonpDone) {
167180
jsonpDone();
168181
}

‎src/ngMock/angular-mocks.js

+13-5
Original file line numberDiff line numberDiff line change
@@ -1378,9 +1378,13 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
13781378
function wrapResponse(wrapped) {
13791379
if (!$browser && timeout) {
13801380
if (timeout.then) {
1381-
timeout.then(handleTimeout);
1381+
timeout.then(function() {
1382+
handlePrematureEnd(angular.isDefined(timeout.$$timeoutId) ? 'timeout' : 'abort');
1383+
});
13821384
} else {
1383-
$timeout(handleTimeout, timeout);
1385+
$timeout(function() {
1386+
handlePrematureEnd('timeout');
1387+
}, timeout);
13841388
}
13851389
}
13861390

@@ -1394,11 +1398,11 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
13941398
copy(response[3] || ''), copy(response[4]));
13951399
}
13961400

1397-
function handleTimeout() {
1401+
function handlePrematureEnd(reason) {
13981402
for (var i = 0, ii = responses.length; i < ii; i++) {
13991403
if (responses[i] === handleResponse) {
14001404
responses.splice(i, 1);
1401-
callback(-1, undefined, '', undefined, 'timeout');
1405+
callback(-1, undefined, '', undefined, reason);
14021406
break;
14031407
}
14041408
}
@@ -2110,7 +2114,11 @@ function MockXhr() {
21102114
return lines.join('\n');
21112115
};
21122116

2113-
this.abort = angular.noop;
2117+
this.abort = function() {
2118+
if (isFunction(this.onabort)) {
2119+
this.onabort();
2120+
}
2121+
};
21142122

21152123
// This section simulates the events on a real XHR object (and the upload object)
21162124
// When we are testing $httpBackend (inside the AngularJS project) we make partial use of this

‎test/ng/httpBackendSpec.js

+38-3
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ describe('$httpBackend', function() {
244244
expect(callback).toHaveBeenCalledOnce();
245245
});
246246

247-
it('should abort request on timeout', function() {
247+
it('should abort request on numerical timeout', function() {
248248
callback.and.callFake(function(status, response) {
249249
expect(status).toBe(-1);
250250
});
@@ -264,9 +264,10 @@ describe('$httpBackend', function() {
264264
});
265265

266266

267-
it('should abort request on timeout promise resolution', inject(function($timeout) {
268-
callback.and.callFake(function(status, response) {
267+
it('should abort request on $timeout promise resolution', inject(function($timeout) {
268+
callback.and.callFake(function(status, response, headers, statusText, xhrStatus) {
269269
expect(status).toBe(-1);
270+
expect(xhrStatus).toBe('timeout');
270271
});
271272

272273
$backend('GET', '/url', null, callback, {}, $timeout(noop, 2000));
@@ -300,6 +301,24 @@ describe('$httpBackend', function() {
300301
}));
301302

302303

304+
it('should abort request on canceler promise resolution', inject(function($q, $browser) {
305+
var canceler = $q.defer();
306+
307+
callback.and.callFake(function(status, response, headers, statusText, xhrStatus) {
308+
expect(status).toBe(-1);
309+
expect(xhrStatus).toBe('abort');
310+
});
311+
312+
$backend('GET', '/url', null, callback, {}, canceler.promise);
313+
xhr = MockXhr.$$lastInstance;
314+
315+
canceler.resolve();
316+
$browser.defer.flush();
317+
318+
expect(callback).toHaveBeenCalledOnce();
319+
}));
320+
321+
303322
it('should cancel timeout on completion', function() {
304323
callback.and.callFake(function(status, response) {
305324
expect(status).toBe(200);
@@ -320,6 +339,22 @@ describe('$httpBackend', function() {
320339
});
321340

322341

342+
it('should call callback with xhrStatus "abort" on explicit xhr.abort() when $timeout is set', inject(function($timeout) {
343+
callback.and.callFake(function(status, response, headers, statusText, xhrStatus) {
344+
expect(status).toBe(-1);
345+
expect(xhrStatus).toBe('abort');
346+
});
347+
348+
$backend('GET', '/url', null, callback, {}, $timeout(noop, 2000));
349+
xhr = MockXhr.$$lastInstance;
350+
spyOn(xhr, 'abort').and.callThrough();
351+
352+
xhr.abort();
353+
354+
expect(callback).toHaveBeenCalledOnce();
355+
}));
356+
357+
323358
it('should set withCredentials', function() {
324359
$backend('GET', '/some.url', null, callback, {}, null, true);
325360
expect(MockXhr.$$lastInstance.withCredentials).toBe(true);

‎test/ng/httpSpec.js

+34-4
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
/* global MockXhr: false */
44

5+
// The http specs run against the mocked httpBackend
6+
57
describe('$http', function() {
68

79
var callback, mockedCookies;
@@ -1907,7 +1909,7 @@ describe('$http', function() {
19071909
function(response) {
19081910
expect(response.data).toBeUndefined();
19091911
expect(response.status).toBe(-1);
1910-
expect(response.xhrStatus).toBe('timeout');
1912+
expect(response.xhrStatus).toBe('abort');
19111913
expect(response.headers()).toEqual(Object.create(null));
19121914
expect(response.config.url).toBe('/some');
19131915
callback();
@@ -1923,17 +1925,45 @@ describe('$http', function() {
19231925
}));
19241926

19251927

1928+
it('should timeout request when numerical timeout is exceeded', inject(function($timeout) {
1929+
var onFulfilled = jasmine.createSpy('onFulfilled');
1930+
var onRejected = jasmine.createSpy('onRejected').and.callFake(function(response) {
1931+
expect(response.xhrStatus).toBe('timeout');
1932+
});
1933+
1934+
$httpBackend.expect('GET', '/some').respond(200);
1935+
1936+
$http({
1937+
method: 'GET',
1938+
url: '/some',
1939+
timeout: 10
1940+
}).then(onFulfilled, onRejected);
1941+
1942+
$timeout.flush(100);
1943+
1944+
expect(onFulfilled).not.toHaveBeenCalled();
1945+
expect(onRejected).toHaveBeenCalled();
1946+
}));
1947+
1948+
19261949
it('should reject promise when timeout promise resolves', inject(function($timeout) {
19271950
var onFulfilled = jasmine.createSpy('onFulfilled');
1928-
var onRejected = jasmine.createSpy('onRejected');
1951+
var onRejected = jasmine.createSpy('onRejected').and.callFake(function(response) {
1952+
expect(response.xhrStatus).toBe('timeout');
1953+
});
1954+
19291955
$httpBackend.expect('GET', '/some').respond(200);
19301956

1931-
$http({method: 'GET', url: '/some', timeout: $timeout(noop, 10)}).then(onFulfilled, onRejected);
1957+
$http({
1958+
method: 'GET',
1959+
url: '/some',
1960+
timeout: $timeout(noop, 10)
1961+
}).then(onFulfilled, onRejected);
19321962

19331963
$timeout.flush(100);
19341964

19351965
expect(onFulfilled).not.toHaveBeenCalled();
1936-
expect(onRejected).toHaveBeenCalledOnce();
1966+
expect(onRejected).toHaveBeenCalled();
19371967
}));
19381968
});
19391969

‎test/ngMock/angular-mocksSpec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1669,7 +1669,7 @@ describe('ngMock', function() {
16691669

16701670
canceler(); // simulate promise resolution
16711671

1672-
expect(callback).toHaveBeenCalledWith(-1, undefined, '', undefined, 'timeout');
1672+
expect(callback).toHaveBeenCalledWith(-1, undefined, '', undefined, 'abort');
16731673
hb.verifyNoOutstandingExpectation();
16741674
hb.verifyNoOutstandingRequest();
16751675
});

0 commit comments

Comments
 (0)