Skip to content

Commit

Permalink
fix(zone.js): only one listener should also re-throw an error correctly
Browse files Browse the repository at this point in the history
Close angular#41867

In the previous commit angular#41562 (comment),
the error thrown in the event listener will be caught and re-thrown, but there is a bug
in the commit, if there is only one listener for the specified event name, the error
will not be re-thrown, so this commit fixes the issue and make sure the error is re-thrown.
  • Loading branch information
JiaLiPassion committed Apr 29, 2021
1 parent 55d9713 commit bb8b696
Show file tree
Hide file tree
Showing 2 changed files with 148 additions and 10 deletions.
19 changes: 9 additions & 10 deletions packages/zone.js/lib/common/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,30 +146,29 @@ export function patchEventTarget(
const target: any = context || event.target || _global;
const tasks = target[zoneSymbolEventNames[event.type][isCapture ? TRUE_STR : FALSE_STR]];
if (tasks) {
const errors = [];
// invoke all tasks which attached to current target with given event.type and capture = false
// for performance concern, if task.length === 1, just invoke
if (tasks.length === 1) {
invokeTask(tasks[0], target, event);
errors.push(invokeTask(tasks[0], target, event));
} else {
// https://github.com/angular/zone.js/issues/836
// copy the tasks array before invoke, to avoid
// the callback will remove itself or other listener
const copyTasks = tasks.slice();
const errors = [];
for (let i = 0; i < copyTasks.length; i++) {
if (event && (event as any)[IMMEDIATE_PROPAGATION_SYMBOL] === true) {
break;
}
const err = invokeTask(copyTasks[i], target, event);
err && errors.push(err);
}
for (let i = 0; i < errors.length; i++) {
const err = errors[i];
api.nativeScheduleMicroTask(() => {
throw err;
});
errors.push(invokeTask(copyTasks[i], target, event));
}
}
for (let i = 0; i < errors.length; i++) {
const err = errors[i];
err && api.nativeScheduleMicroTask(() => {
throw err;
});
}
}
}

Expand Down
139 changes: 139 additions & 0 deletions packages/zone.js/test/browser/browser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2491,6 +2491,83 @@ describe('Zone', function() {
expect(logs).toEqual([]);
}));

it('should re-throw the error when the only listener throw error', function(done: DoneFn) {
// override global.onerror to prevent jasmine report error
let oriWindowOnError = window.onerror;
window.onerror = function() {};
try {
let logs: string[] = [];
const listener1 = function() {
throw new Error('test1');
};
button.addEventListener('click', listener1);

const mouseEvent = document.createEvent('MouseEvent');
mouseEvent.initEvent('click', true, true);

const unhandledRejection = (e: PromiseRejectionEvent) => {
logs.push(e.reason.message);
};
window.addEventListener('unhandledrejection', unhandledRejection);

button.dispatchEvent(mouseEvent);
expect(logs).toEqual([]);

setTimeout(() => {
expect(logs).toEqual(['test1']);
window.removeEventListener('unhandledrejection', unhandledRejection);
window.onerror = oriWindowOnError;
done()
});
} catch (e: any) {
window.onerror = oriWindowOnError;
}
});

it('should not re-throw the error when zone onHandleError handled the error and the only listener throw error',
function(done: DoneFn) {
// override global.onerror to prevent jasmine report error
let oriWindowOnError = window.onerror;
window.onerror = function() {};
try {
let logs: string[] = [];
const listener1 = function() {
throw new Error('test1');
};
const zone = Zone.current.fork({
name: 'error',
onHandleError: (delegate, curr, target, error) => {
logs.push('zone handled ' + target.name + ' ' + error.message);
return false;
}
});

zone.runGuarded(() => {
button.addEventListener('click', listener1);
});

const mouseEvent = document.createEvent('MouseEvent');
mouseEvent.initEvent('click', true, true);

const unhandledRejection = (e: PromiseRejectionEvent) => {
logs.push(e.reason.message);
};
window.addEventListener('unhandledrejection', unhandledRejection);

button.dispatchEvent(mouseEvent);
expect(logs).toEqual(['zone handled error test1']);

setTimeout(() => {
expect(logs).toEqual(['zone handled error test1']);
window.removeEventListener('unhandledrejection', unhandledRejection);
window.onerror = oriWindowOnError;
done();
});
} catch (e: any) {
window.onerror = oriWindowOnError;
}
});

it('should be able to continue to invoke remaining listeners even some listener throw error',
function(done: DoneFn) {
// override global.onerror to prevent jasmine report error
Expand Down Expand Up @@ -2540,6 +2617,68 @@ describe('Zone', function() {
}
});

it('should be able to continue to invoke remaining listeners even some listener throw error with onHandleError zone',
function(done: DoneFn) {
// override global.onerror to prevent jasmine report error
let oriWindowOnError = window.onerror;
window.onerror = function() {};
try {
const zone = Zone.current.fork({
name: 'error',
onHandleError: (delegate, curr, target, error) => {
logs.push('zone handled ' + target.name + ' ' + error.message);
return false;
}
});
let logs: string[] = [];
const listener1 = function() {
logs.push('listener1');
};
const listener2 = function() {
throw new Error('test1');
};
const listener3 = function() {
throw new Error('test2');
};
const listener4 = {
handleEvent: function() {
logs.push('listener2');
}
};

zone.runGuarded(() => {
button.addEventListener('click', listener1);
button.addEventListener('click', listener2);
button.addEventListener('click', listener3);
button.addEventListener('click', listener4);
});

const mouseEvent = document.createEvent('MouseEvent');
mouseEvent.initEvent('click', true, true);

const unhandledRejection = (e: PromiseRejectionEvent) => {
fail('should not be here');
};
window.addEventListener('unhandledrejection', unhandledRejection);

button.dispatchEvent(mouseEvent);
expect(logs).toEqual([
'listener1', 'zone handled error test1', 'zone handled error test2', 'listener2'
]);

setTimeout(() => {
expect(logs).toEqual([
'listener1', 'zone handled error test1', 'zone handled error test2', 'listener2'
]);
window.removeEventListener('unhandledrejection', unhandledRejection);
window.onerror = oriWindowOnError;
done();
});
} catch (e: any) {
window.onerror = oriWindowOnError;
}
});

it('should be able to continue to invoke remaining listeners even some listener throw error in the different zones',
function(done: DoneFn) {
// override global.onerror to prevent jasmine report error
Expand Down

0 comments on commit bb8b696

Please sign in to comment.