fix($resource): don't allow using promises as timeout and log a war…

…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.
angular · Dec 8, 2015 · 4748652 · 4748652
1 parent 0292e6a
commit 4748652
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 1 deletion.
diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js
@@ -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,
@@ -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;
               }
             });
 

diff --git a/test/ngResource/resourceSpec.js b/test/ngResource/resourceSpec.js
@@ -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.');
+    })
+  );
 });