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

Commit 3cb3ac0

Browse files
gkalpakgkalpak
committed
fix($interval): throw when trying to cancel non-$interval promise
Previously, calling `$interval.cancel()` with a promise that was not generated by a call to `$interval()` would do nothing. This could, for example, happen when calling `.then()`/`.catch()` on the returned promise, which creates a new promise, and passing that to `$interval.cancel()`. With this commit, `$interval.cancel()` will throw an error if called with a non-$interval promise, thus surfacing errors that would otherwise go unnoticed. Related to #16424. BREAKING CHNAGE: `$interval.cancel()` will throw an error if called with a promise that was not generated by `$interval()`. Previously, it would silently do nothing. Before: ```js var promise = $interval(doSomething, 1000, 5).then(doSomethingElse); $interval.cancel(promise); // No error; interval NOT canceled. ``` After: ```js var promise = $interval(doSomething, 1000, 5).then(doSomethingElse); $interval.cancel(promise); // Throws error. ``` Correct usage: ```js var promise = $interval(doSomething, 1000, 5); var newPromise = promise.then(doSomethingElse); $interval.cancel(promise); // Interval canceled. ```
1 parent 2465749 commit 3cb3ac0

File tree

3 files changed

+188
-146
lines changed

3 files changed

+188
-146
lines changed

‎docs/content/error/$interval/badprom.ngdoc

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
@ngdoc error
2+
@name $interval:badprom
3+
@fullName Non-$interval promise
4+
@description
5+
6+
This error occurs when calling {@link ng.$interval#cancel $interval.cancel()} with a promise that
7+
was not generated by the {@link ng.$interval $interval} service. This can, for example, happen when
8+
calling {@link ng.$q#the-promise-api then()/catch()} on the returned promise, which creates a new
9+
promise, and pass that new promise to {@link ng.$interval#cancel $interval.cancel()}.
10+
11+
Example of incorrect usage that leads to this error:
12+
13+
```js
14+
var promise = $interval(doSomething, 1000, 5).then(doSomethingElse);
15+
$interval.cancel(promise);
16+
```
17+
18+
To fix the example above, keep a reference to the promise returned by
19+
{@link ng.$interval $interval()} and pass that to {@link ng.$interval#cancel $interval.cancel()}:
20+
21+
```js
22+
var promise = $interval(doSomething, 1000, 5);
23+
var newPromise = promise.then(doSomethingElse);
24+
$interval.cancel(promise);
25+
```

‎src/ng/interval.js

+156-144
Original file line numberDiff line numberDiff line change
@@ -1,138 +1,140 @@
11
'use strict';
22

3+
var $intervalMinErr = minErr('$interval');
4+
35
/** @this */
46
function $IntervalProvider() {
57
this.$get = ['$rootScope', '$window', '$q', '$$q', '$browser',
68
function($rootScope, $window, $q, $$q, $browser) {
79
var intervals = {};
810

911

10-
/**
11-
* @ngdoc service
12-
* @name $interval
13-
*
14-
* @description
15-
* AngularJS's wrapper for `window.setInterval`. The `fn` function is executed every `delay`
16-
* milliseconds.
17-
*
18-
* The return value of registering an interval function is a promise. This promise will be
19-
* notified upon each tick of the interval, and will be resolved after `count` iterations, or
20-
* run indefinitely if `count` is not defined. The value of the notification will be the
21-
* number of iterations that have run.
22-
* To cancel an interval, call `$interval.cancel(promise)`.
23-
*
24-
* In tests you can use {@link ngMock.$interval#flush `$interval.flush(millis)`} to
25-
* move forward by `millis` milliseconds and trigger any functions scheduled to run in that
26-
* time.
27-
*
28-
* <div class="alert alert-warning">
29-
* **Note**: Intervals created by this service must be explicitly destroyed when you are finished
30-
* with them. In particular they are not automatically destroyed when a controller's scope or a
31-
* directive's element are destroyed.
32-
* You should take this into consideration and make sure to always cancel the interval at the
33-
* appropriate moment. See the example below for more details on how and when to do this.
34-
* </div>
35-
*
36-
* @param {function()} fn A function that should be called repeatedly. If no additional arguments
37-
* are passed (see below), the function is called with the current iteration count.
38-
* @param {number} delay Number of milliseconds between each function call.
39-
* @param {number=} [count=0] Number of times to repeat. If not set, or 0, will repeat
40-
* indefinitely.
41-
* @param {boolean=} [invokeApply=true] If set to `false` skips model dirty checking, otherwise
42-
* will invoke `fn` within the {@link ng.$rootScope.Scope#$apply $apply} block.
43-
* @param {...*=} Pass additional parameters to the executed function.
44-
* @returns {promise} A promise which will be notified on each iteration. It will resolve once all iterations of the interval complete.
45-
*
46-
* @example
47-
* <example module="intervalExample" name="interval-service">
48-
* <file name="index.html">
49-
* <script>
50-
* angular.module('intervalExample', [])
51-
* .controller('ExampleController', ['$scope', '$interval',
52-
* function($scope, $interval) {
53-
* $scope.format = 'M/d/yy h:mm:ss a';
54-
* $scope.blood_1 = 100;
55-
* $scope.blood_2 = 120;
56-
*
57-
* var stop;
58-
* $scope.fight = function() {
59-
* // Don't start a new fight if we are already fighting
60-
* if ( angular.isDefined(stop) ) return;
61-
*
62-
* stop = $interval(function() {
63-
* if ($scope.blood_1 > 0 && $scope.blood_2 > 0) {
64-
* $scope.blood_1 = $scope.blood_1 - 3;
65-
* $scope.blood_2 = $scope.blood_2 - 4;
66-
* } else {
67-
* $scope.stopFight();
68-
* }
69-
* }, 100);
70-
* };
71-
*
72-
* $scope.stopFight = function() {
73-
* if (angular.isDefined(stop)) {
74-
* $interval.cancel(stop);
75-
* stop = undefined;
76-
* }
77-
* };
78-
*
79-
* $scope.resetFight = function() {
80-
* $scope.blood_1 = 100;
81-
* $scope.blood_2 = 120;
82-
* };
83-
*
84-
* $scope.$on('$destroy', function() {
85-
* // Make sure that the interval is destroyed too
86-
* $scope.stopFight();
87-
* });
88-
* }])
89-
* // Register the 'myCurrentTime' directive factory method.
90-
* // We inject $interval and dateFilter service since the factory method is DI.
91-
* .directive('myCurrentTime', ['$interval', 'dateFilter',
92-
* function($interval, dateFilter) {
93-
* // return the directive link function. (compile function not needed)
94-
* return function(scope, element, attrs) {
95-
* var format, // date format
96-
* stopTime; // so that we can cancel the time updates
97-
*
98-
* // used to update the UI
99-
* function updateTime() {
100-
* element.text(dateFilter(new Date(), format));
101-
* }
102-
*
103-
* // watch the expression, and update the UI on change.
104-
* scope.$watch(attrs.myCurrentTime, function(value) {
105-
* format = value;
106-
* updateTime();
107-
* });
108-
*
109-
* stopTime = $interval(updateTime, 1000);
110-
*
111-
* // listen on DOM destroy (removal) event, and cancel the next UI update
112-
* // to prevent updating time after the DOM element was removed.
113-
* element.on('$destroy', function() {
114-
* $interval.cancel(stopTime);
115-
* });
116-
* }
117-
* }]);
118-
* </script>
119-
*
120-
* <div>
121-
* <div ng-controller="ExampleController">
122-
* <label>Date format: <input ng-model="format"></label> <hr/>
123-
* Current time is: <span my-current-time="format"></span>
124-
* <hr/>
125-
* Blood 1 : <font color='red'>{{blood_1}}</font>
126-
* Blood 2 : <font color='red'>{{blood_2}}</font>
127-
* <button type="button" data-ng-click="fight()">Fight</button>
128-
* <button type="button" data-ng-click="stopFight()">StopFight</button>
129-
* <button type="button" data-ng-click="resetFight()">resetFight</button>
130-
* </div>
131-
* </div>
132-
*
133-
* </file>
134-
* </example>
135-
*/
12+
/**
13+
* @ngdoc service
14+
* @name $interval
15+
*
16+
* @description
17+
* AngularJS's wrapper for `window.setInterval`. The `fn` function is executed every `delay`
18+
* milliseconds.
19+
*
20+
* The return value of registering an interval function is a promise. This promise will be
21+
* notified upon each tick of the interval, and will be resolved after `count` iterations, or
22+
* run indefinitely if `count` is not defined. The value of the notification will be the
23+
* number of iterations that have run.
24+
* To cancel an interval, call `$interval.cancel(promise)`.
25+
*
26+
* In tests you can use {@link ngMock.$interval#flush `$interval.flush(millis)`} to
27+
* move forward by `millis` milliseconds and trigger any functions scheduled to run in that
28+
* time.
29+
*
30+
* <div class="alert alert-warning">
31+
* **Note**: Intervals created by this service must be explicitly destroyed when you are finished
32+
* with them. In particular they are not automatically destroyed when a controller's scope or a
33+
* directive's element are destroyed.
34+
* You should take this into consideration and make sure to always cancel the interval at the
35+
* appropriate moment. See the example below for more details on how and when to do this.
36+
* </div>
37+
*
38+
* @param {function()} fn A function that should be called repeatedly. If no additional arguments
39+
* are passed (see below), the function is called with the current iteration count.
40+
* @param {number} delay Number of milliseconds between each function call.
41+
* @param {number=} [count=0] Number of times to repeat. If not set, or 0, will repeat
42+
* indefinitely.
43+
* @param {boolean=} [invokeApply=true] If set to `false` skips model dirty checking, otherwise
44+
* will invoke `fn` within the {@link ng.$rootScope.Scope#$apply $apply} block.
45+
* @param {...*=} Pass additional parameters to the executed function.
46+
* @returns {promise} A promise which will be notified on each iteration. It will resolve once all iterations of the interval complete.
47+
*
48+
* @example
49+
* <example module="intervalExample" name="interval-service">
50+
* <file name="index.html">
51+
* <script>
52+
* angular.module('intervalExample', [])
53+
* .controller('ExampleController', ['$scope', '$interval',
54+
* function($scope, $interval) {
55+
* $scope.format = 'M/d/yy h:mm:ss a';
56+
* $scope.blood_1 = 100;
57+
* $scope.blood_2 = 120;
58+
*
59+
* var stop;
60+
* $scope.fight = function() {
61+
* // Don't start a new fight if we are already fighting
62+
* if ( angular.isDefined(stop) ) return;
63+
*
64+
* stop = $interval(function() {
65+
* if ($scope.blood_1 > 0 && $scope.blood_2 > 0) {
66+
* $scope.blood_1 = $scope.blood_1 - 3;
67+
* $scope.blood_2 = $scope.blood_2 - 4;
68+
* } else {
69+
* $scope.stopFight();
70+
* }
71+
* }, 100);
72+
* };
73+
*
74+
* $scope.stopFight = function() {
75+
* if (angular.isDefined(stop)) {
76+
* $interval.cancel(stop);
77+
* stop = undefined;
78+
* }
79+
* };
80+
*
81+
* $scope.resetFight = function() {
82+
* $scope.blood_1 = 100;
83+
* $scope.blood_2 = 120;
84+
* };
85+
*
86+
* $scope.$on('$destroy', function() {
87+
* // Make sure that the interval is destroyed too
88+
* $scope.stopFight();
89+
* });
90+
* }])
91+
* // Register the 'myCurrentTime' directive factory method.
92+
* // We inject $interval and dateFilter service since the factory method is DI.
93+
* .directive('myCurrentTime', ['$interval', 'dateFilter',
94+
* function($interval, dateFilter) {
95+
* // return the directive link function. (compile function not needed)
96+
* return function(scope, element, attrs) {
97+
* var format, // date format
98+
* stopTime; // so that we can cancel the time updates
99+
*
100+
* // used to update the UI
101+
* function updateTime() {
102+
* element.text(dateFilter(new Date(), format));
103+
* }
104+
*
105+
* // watch the expression, and update the UI on change.
106+
* scope.$watch(attrs.myCurrentTime, function(value) {
107+
* format = value;
108+
* updateTime();
109+
* });
110+
*
111+
* stopTime = $interval(updateTime, 1000);
112+
*
113+
* // listen on DOM destroy (removal) event, and cancel the next UI update
114+
* // to prevent updating time after the DOM element was removed.
115+
* element.on('$destroy', function() {
116+
* $interval.cancel(stopTime);
117+
* });
118+
* }
119+
* }]);
120+
* </script>
121+
*
122+
* <div>
123+
* <div ng-controller="ExampleController">
124+
* <label>Date format: <input ng-model="format"></label> <hr/>
125+
* Current time is: <span my-current-time="format"></span>
126+
* <hr/>
127+
* Blood 1 : <font color='red'>{{blood_1}}</font>
128+
* Blood 2 : <font color='red'>{{blood_2}}</font>
129+
* <button type="button" data-ng-click="fight()">Fight</button>
130+
* <button type="button" data-ng-click="stopFight()">StopFight</button>
131+
* <button type="button" data-ng-click="resetFight()">resetFight</button>
132+
* </div>
133+
* </div>
134+
*
135+
* </file>
136+
* </example>
137+
*/
136138
function interval(fn, delay, count, invokeApply) {
137139
var hasParams = arguments.length > 4,
138140
args = hasParams ? sliceArgs(arguments, 4) : [],
@@ -177,26 +179,36 @@ function $IntervalProvider() {
177179
}
178180

179181

180-
/**
181-
* @ngdoc method
182-
* @name $interval#cancel
183-
*
184-
* @description
185-
* Cancels a task associated with the `promise`.
186-
*
187-
* @param {Promise=} promise returned by the `$interval` function.
188-
* @returns {boolean} Returns `true` if the task was successfully canceled.
189-
*/
182+
/**
183+
* @ngdoc method
184+
* @name $interval#cancel
185+
*
186+
* @description
187+
* Cancels a task associated with the `promise`.
188+
*
189+
* @param {Promise=} promise returned by the `$interval` function.
190+
* @returns {boolean} Returns `true` if the task was successfully canceled.
191+
*/
190192
interval.cancel = function(promise) {
191-
if (promise && promise.$$intervalId in intervals) {
192-
// Interval cancels should not report as unhandled promise.
193-
markQExceptionHandled(intervals[promise.$$intervalId].promise);
194-
intervals[promise.$$intervalId].reject('canceled');
195-
$window.clearInterval(promise.$$intervalId);
196-
delete intervals[promise.$$intervalId];
197-
return true;
193+
if (!promise) return false;
194+
195+
if (!promise.hasOwnProperty('$$intervalId')) {
196+
throw $intervalMinErr('badprom',
197+
'`$interval.cancel()` called with a promise that was not generated by `$interval()`.');
198198
}
199-
return false;
199+
200+
if (!intervals.hasOwnProperty(promise.$$intervalId)) return false;
201+
202+
var id = promise.$$intervalId;
203+
var deferred = intervals[id];
204+
205+
// Interval cancels should not report an unhandled promise.
206+
markQExceptionHandled(deferred.promise);
207+
deferred.reject('canceled');
208+
$window.clearInterval(id);
209+
delete intervals[id];
210+
211+
return true;
200212
};
201213

202214
return interval;

‎test/ng/intervalSpec.js

+7-2
Original file line numberDiff line numberDiff line change
@@ -335,12 +335,17 @@ describe('$interval', function() {
335335
}));
336336

337337

338-
it('should not throw a runtime exception when given an undefined promise',
339-
inject(function($interval) {
338+
it('should not throw an error when given an undefined promise', inject(function($interval) {
340339
expect($interval.cancel()).toBe(false);
341340
}));
342341

343342

343+
it('should throw an error when given a non-$interval promise', inject(function($interval) {
344+
var promise = $interval(noop).then(noop);
345+
expect(function() { $interval.cancel(promise); }).toThrowMinErr('$interval', 'badprom');
346+
}));
347+
348+
344349
it('should not trigger digest when cancelled', inject(function($interval, $rootScope, $browser) {
345350
var watchSpy = jasmine.createSpy('watchSpy');
346351
$rootScope.$watch(watchSpy);

0 commit comments

Comments
 (0)