Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

feat($http,$resource): statusText resolved in timeout promise #14021

Closed
wants to merge 3 commits into from
Closed

feat($http,$resource): statusText resolved in timeout promise #14021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eaf0cd7
feat($http,$resource): statusText resolved in timeout promise
mrac Feb 12, 2016
ebbb85c
fixup: add spies in unit tests
mrac Feb 16, 2016
d8b9f29
fixup: simplified callback call
mrac Feb 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/ng/httpBackend.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,12 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
statusText);
};

var timeoutStatusText;

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, timeoutStatusText || '');
};

xhr.onerror = requestError;
Expand Down Expand Up @@ -145,7 +147,8 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
}


function timeoutRequest() {
function timeoutRequest(statusText) {
timeoutStatusText = statusText;
jsonpDone && jsonpDone();
xhr && xhr.abort();
}
Expand All @@ -155,7 +158,7 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
if (isDefined(timeoutId)) {
$browserDefer.cancel(timeoutId);
}
jsonpDone = xhr = null;
jsonpDone = xhr = timeoutStatusText = null;

callback(status, response, headersString, statusText);
$browser.$$completeOutstandingRequest(noop);
Expand Down
4 changes: 2 additions & 2 deletions src/ngMock/angular-mocks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1346,11 +1346,11 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
copy(response[3] || ''));
}

function handleTimeout() {
function handleTimeout(statusText) {
for (var i = 0, ii = responses.length; i < ii; i++) {
if (responses[i] === handleResponse) {
responses.splice(i, 1);
callback(-1, undefined, '');
callback(-1, undefined, '', statusText);
break;
}
}
Expand Down
11 changes: 9 additions & 2 deletions src/ngResource/resource.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,9 @@ angular.module('ngResource', ['ng']).
httpConfig.timeout = timeoutDeferred.promise;

if (numericTimeout) {
numericTimeoutPromise = $timeout(timeoutDeferred.resolve, numericTimeout);
numericTimeoutPromise = $timeout(function() {
timeoutDeferred.resolve("timeout");
}, numericTimeout);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be also written as:

numericTimeoutPromise = $timeout(timeoutDeferred.resolve, numericTimeout, false, 'timeout');
// OR
numericTimeoutPromise = $timeout(timeoutDeferred.resolve.bind(null, 'timeout'), numericTimeout);
// OR
numericTimeoutPromise = $timeout(angular.bind(timeoutDeferred, timeoutDeferred.resolve, 'timeout'), numericTimeout);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But...you know...it's better to stay as is 😥

}
}

Expand Down Expand Up @@ -729,7 +731,12 @@ angular.module('ngResource', ['ng']).
// - return the instance / collection
value.$promise = promise;
value.$resolved = false;
if (cancellable) value.$cancelRequest = timeoutDeferred.resolve;

if (cancellable) {
value.$cancelRequest = function() {
timeoutDeferred.resolve("cancelled");
};
}

return value;
}
Expand Down
25 changes: 24 additions & 1 deletion test/ng/httpBackendSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,9 @@ describe('$httpBackend', function() {


it('should abort request on timeout promise resolution', inject(function($timeout) {
callback.andCallFake(function(status, response) {
callback.andCallFake(function(status, response, headers, statusText) {
expect(status).toBe(-1);
expect(statusText).toBe('');
});

$backend('GET', '/url', null, callback, {}, $timeout(noop, 2000));
Expand All @@ -190,6 +191,28 @@ describe('$httpBackend', function() {
}));


it('should abort request on timeout promise resolution with statusText', inject(function($timeout) {
callback.andCallFake(function(status, response, headers, statusText) {
expect(status).toBe(-1);
expect(statusText).toBe('whatever');
});

$backend('GET', '/url', null, callback, {}, $timeout(function() {
return "whatever";
}, 2000));

xhr = MockXhr.$$lastInstance;
spyOn(xhr, 'abort');

$timeout.flush();
expect(xhr.abort).toHaveBeenCalledOnce();

xhr.status = 0;
xhr.onabort();
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);
Expand Down
22 changes: 20 additions & 2 deletions test/ngMock/angular-mocksSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1393,7 +1393,25 @@ describe('ngMock', function() {

canceler(); // simulate promise resolution

expect(callback).toHaveBeenCalledWith(-1, undefined, '');
expect(callback).toHaveBeenCalledWith(-1, undefined, '', undefined);
hb.verifyNoOutstandingExpectation();
hb.verifyNoOutstandingRequest();
});


it('should abort requests when timeout promise resolves with statusText', 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('whatever'); // simulate promise resolution

expect(callback).toHaveBeenCalledWith(-1, undefined, '', 'whatever');
hb.verifyNoOutstandingExpectation();
hb.verifyNoOutstandingRequest();
});
Expand All @@ -1405,7 +1423,7 @@ describe('ngMock', function() {
hb('GET', '/url1', null, callback, null, 200);
$timeout.flush(300);

expect(callback).toHaveBeenCalledWith(-1, undefined, '');
expect(callback).toHaveBeenCalledWith(-1, undefined, '', undefined);
hb.verifyNoOutstandingExpectation();
hb.verifyNoOutstandingRequest();
}));
Expand Down
42 changes: 38 additions & 4 deletions test/ngResource/resourceSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1544,13 +1544,23 @@ describe('cancelling requests', function() {
}
});

CreditCard.get();
var errorCB = jasmine.createSpy('error').andCallFake(function(err) {
expect(err.status).toEqual(-1);
expect(err.statusText).toEqual("timeout");
});

var creditCard = CreditCard.get();

creditCard.$promise.catch(errorCB);

$timeout.flush();
expect($httpBackend.flush).toThrow(new Error('No pending request to flush !'));
expect(errorCB).toHaveBeenCalledOnce();
errorCB.reset();

CreditCard.get();
expect($httpBackend.flush).not.toThrow();

expect(errorCB).not.toHaveBeenCalled();
});

it('should cancel the request (if cancellable), when calling `$cancelRequest`', function() {
Expand All @@ -1563,11 +1573,23 @@ describe('cancelling requests', function() {
}
});

CreditCard.get().$cancelRequest();
var errorCB = jasmine.createSpy('error').andCallFake(function(err) {
expect(err.status).toEqual(-1);
expect(err.statusText).toEqual("cancelled");
});

var creditCard = CreditCard.get();

creditCard.$promise.catch(errorCB);

creditCard.$cancelRequest();
expect($httpBackend.flush).toThrow(new Error('No pending request to flush !'));
expect(errorCB).toHaveBeenCalledOnce();
errorCB.reset();

CreditCard.get();
expect($httpBackend.flush).not.toThrow();
expect(errorCB).not.toHaveBeenCalled();
});

it('should cancel the request, when calling `$cancelRequest` in cancellable actions with timeout defined', function() {
Expand All @@ -1581,11 +1603,23 @@ describe('cancelling requests', function() {
}
});

CreditCard.get().$cancelRequest();
var errorCB = jasmine.createSpy('error').andCallFake(function(err) {
expect(err.status).toEqual(-1);
expect(err.statusText).toEqual("cancelled");
});

var creditCard = CreditCard.get();

creditCard.$promise.catch(errorCB);

creditCard.$cancelRequest();
expect($httpBackend.flush).toThrow(new Error('No pending request to flush !'));
expect(errorCB).toHaveBeenCalledOnce();
errorCB.reset();

CreditCard.get();
expect($httpBackend.flush).not.toThrow();
expect(errorCB).not.toHaveBeenCalled();
});

it('should reset `$cancelRequest` after the response arrives', function() {
Expand Down