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
…46435)

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

PR Close #46435
  • Loading branch information
arturovt authored and thePunderWoman committed Oct 11, 2022
1 parent 96b7fe9 commit b618b5a
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
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
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 b618b5a

Please sign in to comment.