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

Commit

Permalink
fix($resource): don't allow using promises as timeout and log a war…
Browse files Browse the repository at this point in the history
…ning

Promises never worked correctly as values for `timeout` in `$resource`, because the same value has
to be re-used for multiple requests (and it is not possible to `angular.copy()` a promise).
Now (in addition to ignoring a non-numeric `timeout`), a warning is logged to the console using
`$log.debug()`.

Partly fixes #13393.

BREAKING CHANGE:

Possible breaking change for users who updated their code to provide a `timeout`
promise for a `$resource` request in version 1.4.8.

Up to v1.4.7 (included), using a promise as a timeout in `$resource`, would silently
fail (i.e. have no effect).

In v1.4.8, using a promise as timeout would have the (buggy) behaviour described
in #12657 (comment)
(i.e. it will work as expected for the first time you resolve the promise and will
cancel all subsequent requests after that - one has to re-create the resource
class. This is feature was not documented.)

With this change, using a promise as timeout in 1.4.9 onwsards is not allowed.
It will log a warning and ignore the timeout value.

If you need support for cancellable `$resource` actions, you should upgrade to
version 1.5 or higher.
  • Loading branch information
gkalpak authored and petebacondarwin committed Dec 8, 2015
1 parent 0292e6a commit 4748652
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 1 deletion.
12 changes: 11 additions & 1 deletion src/ngResource/resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ angular.module('ngResource', ['ng']).
}
};

this.$get = ['$http', '$q', function($http, $q) {
this.$get = ['$http', '$log', '$q', function($http, $log, $q) {

var noop = angular.noop,
forEach = angular.forEach,
Expand Down Expand Up @@ -579,6 +579,16 @@ angular.module('ngResource', ['ng']).
case 'isArray':
case 'interceptor':
break;
case 'timeout':
if (value && !angular.isNumber(value)) {
$log.debug('ngResource:\n' +
' Only numeric values are allowed as `timeout`.\n' +
' Promises are not supported in $resource, because the same value would ' +
'be used for multiple requests.\n' +
' If you need support for cancellable $resource actions, you should ' +
'upgrade to version 1.5 or higher.');
}
break;
}
});

Expand Down
42 changes: 42 additions & 0 deletions test/ngResource/resourceSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1355,6 +1355,48 @@ describe('resource', function() {
/^\[\$resource:badcfg\] Error in resource configuration for action `get`\. Expected response to contain an object but got an array \(Request: GET \/Customer\/123\)/
);
});
});

describe('resource with promises as `timeout`', function() {
var httpSpy;
var $httpBackend;
var $resource;

beforeEach(module('ngResource', function($provide) {
$provide.decorator('$http', function($delegate) {
httpSpy = jasmine.createSpy('$http').andCallFake($delegate);
return httpSpy;
});
}));

beforeEach(inject(function(_$httpBackend_, _$resource_) {
$httpBackend = _$httpBackend_;
$resource = _$resource_;
}));

it('should ignore non-numeric timeouts in actions and log a $debug message',
inject(function($log, $q) {
spyOn($log, 'debug');
$httpBackend.whenGET('/CreditCard').respond({});

var CreditCard = $resource('/CreditCard', {}, {
get: {
method: 'GET',
timeout: $q.defer().promise
}
});

CreditCard.get();
$httpBackend.flush();

expect(httpSpy).toHaveBeenCalledOnce();
expect(httpSpy.calls[0].args[0].timeout).toBeUndefined();
expect($log.debug).toHaveBeenCalledOnceWith('ngResource:\n' +
' Only numeric values are allowed as `timeout`.\n' +
' Promises are not supported in $resource, because the same value would ' +
'be used for multiple requests.\n' +
' If you need support for cancellable $resource actions, you should ' +
'upgrade to version 1.5 or higher.');
})
);
});

0 comments on commit 4748652

Please sign in to comment.