Skip to content

Commit

Permalink
fix(zone.js): cancel tasks only when they are scheduled or running
Browse files Browse the repository at this point in the history
Currently, there's no check if the task (that is being canceled) has the right state.
Only `scheduled` and `running` tasks can be canceled. If the task has a non-appropriate
state, then an error will be thrown. Cancelation should not throw an error on an already
canceled task, e.g. `clearTimeout` does not throw errors when it's called multiple times
on the same timer.

PR Close #45711
  • Loading branch information
arturovt committed Sep 20, 2022
1 parent 59aa2c0 commit 25f22c2
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 0 deletions.
5 changes: 5 additions & 0 deletions packages/zone.js/lib/zone.ts
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,11 @@ const Zone: ZoneType = (function(global: any) {
throw new Error(
'A task can only be cancelled in the zone of creation! (Creation: ' +
(task.zone || NO_ZONE).name + '; Execution: ' + this.name + ')');

if (task.state !== scheduled && task.state !== running) {
return;
}

(task as ZoneTask<any>)._transitionTo(canceling, scheduled, running);
try {
this._zoneDelegate.cancelTask(this, task);
Expand Down
22 changes: 22 additions & 0 deletions packages/zone.js/test/common/task.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -907,6 +907,28 @@ describe('task lifecycle', () => {
}));
});

// Test specific to https://github.com/angular/angular/issues/45711
it('should not throw an error when the task has been canceled previously and is attemped to be canceled again',
() => {
testFnWithLoggedTransitionTo(() => {
Zone.current.fork({name: 'testCancelZone'}).run(() => {
const task =
Zone.current.scheduleEventTask('testEventTask', noop, undefined, noop, noop);
Zone.current.cancelTask(task);
Zone.current.cancelTask(task);
});
expect(log.map(item => {
return {toState: item.toState, fromState: item.fromState};
}))
.toEqual([
{toState: 'scheduling', fromState: 'notScheduled'},
{toState: 'scheduled', fromState: 'scheduling'},
{toState: 'canceling', fromState: 'scheduled'},
{toState: 'notScheduled', fromState: 'canceling'}
]);
});
});

describe('reschedule zone', () => {
let callbackLogs: ({pos: string, method: string, zone: string, task: string}|HasTaskState)[];
const newZone = Zone.root.fork({
Expand Down

0 comments on commit 25f22c2

Please sign in to comment.