fix(zone.js): should continue to executue listeners when throw error (#41562)

Close #41522

`zone.js` patches event listeners and run all event listeners together, if
one event handler throws error, the listeners afterward may not be invoked.

Reproduction:

```
export class AppComponent implements AfterViewInit {
  @ViewChild('btn') btn: ElementRef;
  title = 'event-error';

  constructor(private ngZone: NgZone) {}

  ngAfterViewInit() {
    this.ngZone.runOutsideAngular(() => {
      this.btn.nativeElement.addEventListener('click', () => {
        throw new Error('test1');
      });
      this.btn.nativeElement.addEventListener('click', () => {
        console.log('add eventlistener click');
      });
    });
  }
}
```

Until now no Angular users report this issue becuase in the `ngZone`, all
error will be caught and will not rethrow, so the event listeners afterward
will still continue to execute, but if the event handlers are outside of `ngZone`,
the error will break the execution.

This commit catch all errors, and after all event listeners finished invocation,
rethrow the errors in seperate `microTasks`, the reason I am using `microTask` here
is to handle multiple errors case.

PR Close #41562
This commit is contained in:
JiaLiPassion 2021-04-11 14:19:23 +08:00 committed by Jessica Janiuk
parent e7ab84051e
commit 008eaf3b7d
11 changed files with 154 additions and 64 deletions

View File

@ -66,7 +66,7 @@ Zone.__load_patch('EventTarget', (global: any, Zone: ZoneType, api: _ZonePrivate
// patch XMLHttpRequestEventTarget's addEventListener/removeEventListener // patch XMLHttpRequestEventTarget's addEventListener/removeEventListener
const XMLHttpRequestEventTarget = (global as any)['XMLHttpRequestEventTarget']; const XMLHttpRequestEventTarget = (global as any)['XMLHttpRequestEventTarget'];
if (XMLHttpRequestEventTarget && XMLHttpRequestEventTarget.prototype) { if (XMLHttpRequestEventTarget && XMLHttpRequestEventTarget.prototype) {
api.patchEventTarget(global, [XMLHttpRequestEventTarget.prototype]); api.patchEventTarget(global, api, [XMLHttpRequestEventTarget.prototype]);
} }
}); });

View File

@ -112,7 +112,7 @@ export function eventTargetLegacyPatch(_global: any, api: _ZonePrivate) {
} }
// vh is validateHandler to check event handler // vh is validateHandler to check event handler
// is valid or not(for security check) // is valid or not(for security check)
api.patchEventTarget(_global, apiTypes, { api.patchEventTarget(_global, api, apiTypes, {
vh: checkIEAndCrossContext, vh: checkIEAndCrossContext,
transferEventName: (eventName: string) => { transferEventName: (eventName: string) => {
const pointerEventName = pointerEventsMap[eventName]; const pointerEventName = pointerEventsMap[eventName];

View File

@ -29,7 +29,7 @@ export function eventTargetPatch(_global: any, api: _ZonePrivate) {
if (!EVENT_TARGET || !EVENT_TARGET.prototype) { if (!EVENT_TARGET || !EVENT_TARGET.prototype) {
return; return;
} }
api.patchEventTarget(_global, [EVENT_TARGET && EVENT_TARGET.prototype]); api.patchEventTarget(_global, api, [EVENT_TARGET && EVENT_TARGET.prototype]);
return true; return true;
} }

View File

@ -20,7 +20,7 @@ Zone.__load_patch('shadydom', (global: any, Zone: ZoneType, api: _ZonePrivate) =
if (proto && proto.hasOwnProperty('addEventListener')) { if (proto && proto.hasOwnProperty('addEventListener')) {
proto[Zone.__symbol__('addEventListener')] = null; proto[Zone.__symbol__('addEventListener')] = null;
proto[Zone.__symbol__('removeEventListener')] = null; proto[Zone.__symbol__('removeEventListener')] = null;
api.patchEventTarget(global, [proto]); api.patchEventTarget(global, api, [proto]);
} }
}); });
}); });

View File

@ -22,5 +22,5 @@ Zone.__load_patch('RTCPeerConnection', (global: any, Zone: ZoneType, api: _ZoneP
RTCPeerConnection.prototype[addSymbol] = null; RTCPeerConnection.prototype[addSymbol] = null;
RTCPeerConnection.prototype[removeSymbol] = null; RTCPeerConnection.prototype[removeSymbol] = null;
api.patchEventTarget(global, [RTCPeerConnection.prototype], {useG: false}); api.patchEventTarget(global, api, [RTCPeerConnection.prototype], {useG: false});
}); });

View File

@ -13,7 +13,7 @@ export function apply(api: _ZonePrivate, _global: any) {
// On Safari window.EventTarget doesn't exist so need to patch WS add/removeEventListener // On Safari window.EventTarget doesn't exist so need to patch WS add/removeEventListener
// On older Chrome, no need since EventTarget was already patched // On older Chrome, no need since EventTarget was already patched
if (!(<any>_global).EventTarget) { if (!(<any>_global).EventTarget) {
api.patchEventTarget(_global, [WS.prototype]); api.patchEventTarget(_global, api, [WS.prototype]);
} }
(<any>_global).WebSocket = function(x: any, y: any) { (<any>_global).WebSocket = function(x: any, y: any) {
const socket = arguments.length > 1 ? new WS(x, y) : new WS(x); const socket = arguments.length > 1 ? new WS(x, y) : new WS(x);

View File

@ -86,7 +86,7 @@ export interface PatchEventTargetOptions {
} }
export function patchEventTarget( export function patchEventTarget(
_global: any, apis: any[], patchOptions?: PatchEventTargetOptions) { _global: any, api: _ZonePrivate, apis: any[], patchOptions?: PatchEventTargetOptions) {
const ADD_EVENT_LISTENER = (patchOptions && patchOptions.add) || ADD_EVENT_LISTENER_STR; const ADD_EVENT_LISTENER = (patchOptions && patchOptions.add) || ADD_EVENT_LISTENER_STR;
const REMOVE_EVENT_LISTENER = (patchOptions && patchOptions.rm) || REMOVE_EVENT_LISTENER_STR; const REMOVE_EVENT_LISTENER = (patchOptions && patchOptions.rm) || REMOVE_EVENT_LISTENER_STR;
@ -114,7 +114,15 @@ export function patchEventTarget(
task.originalDelegate = delegate; task.originalDelegate = delegate;
} }
// invoke static task.invoke // invoke static task.invoke
task.invoke(task, target, [event]); // need to try/catch error here, otherwise, the error in one event listener
// will break the executions of the other event listeners. Also error will
// not remove the event listener when `once` options is true.
let error;
try {
task.invoke(task, target, [event]);
} catch (err) {
error = err;
}
const options = task.options; const options = task.options;
if (options && typeof options === 'object' && options.once) { if (options && typeof options === 'object' && options.once) {
// if options.once is true, after invoke once remove listener here // if options.once is true, after invoke once remove listener here
@ -123,10 +131,10 @@ export function patchEventTarget(
const delegate = task.originalDelegate ? task.originalDelegate : task.callback; const delegate = task.originalDelegate ? task.originalDelegate : task.callback;
target[REMOVE_EVENT_LISTENER].call(target, event.type, delegate, options); target[REMOVE_EVENT_LISTENER].call(target, event.type, delegate, options);
} }
return error;
}; };
// global shared zoneAwareCallback to handle all event callback with capture = false function globalCallback(context: unknown, event: Event, isCapture: boolean) {
const globalZoneAwareCallback = function(this: unknown, event: Event) {
// https://github.com/angular/zone.js/issues/911, in IE, sometimes // https://github.com/angular/zone.js/issues/911, in IE, sometimes
// event will be undefined, so we need to use window.event // event will be undefined, so we need to use window.event
event = event || _global.event; event = event || _global.event;
@ -135,8 +143,8 @@ export function patchEventTarget(
} }
// event.target is needed for Samsung TV and SourceBuffer // event.target is needed for Samsung TV and SourceBuffer
// || global is needed https://github.com/angular/zone.js/issues/190 // || global is needed https://github.com/angular/zone.js/issues/190
const target: any = this || event.target || _global; const target: any = context || event.target || _global;
const tasks = target[zoneSymbolEventNames[event.type][FALSE_STR]]; const tasks = target[zoneSymbolEventNames[event.type][isCapture ? TRUE_STR : FALSE_STR]];
if (tasks) { if (tasks) {
// invoke all tasks which attached to current target with given event.type and capture = false // invoke all tasks which attached to current target with given event.type and capture = false
// for performance concern, if task.length === 1, just invoke // for performance concern, if task.length === 1, just invoke
@ -147,46 +155,32 @@ export function patchEventTarget(
// copy the tasks array before invoke, to avoid // copy the tasks array before invoke, to avoid
// the callback will remove itself or other listener // the callback will remove itself or other listener
const copyTasks = tasks.slice(); const copyTasks = tasks.slice();
const errors = [];
for (let i = 0; i < copyTasks.length; i++) { for (let i = 0; i < copyTasks.length; i++) {
if (event && (event as any)[IMMEDIATE_PROPAGATION_SYMBOL] === true) { if (event && (event as any)[IMMEDIATE_PROPAGATION_SYMBOL] === true) {
break; break;
} }
invokeTask(copyTasks[i], target, event); 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;
});
} }
} }
} }
}
// global shared zoneAwareCallback to handle all event callback with capture = false
const globalZoneAwareCallback = function(this: unknown, event: Event) {
return globalCallback(this, event, false);
}; };
// global shared zoneAwareCallback to handle all event callback with capture = true // global shared zoneAwareCallback to handle all event callback with capture = true
const globalZoneAwareCaptureCallback = function(this: unknown, event: Event) { const globalZoneAwareCaptureCallback = function(this: unknown, event: Event) {
// https://github.com/angular/zone.js/issues/911, in IE, sometimes return globalCallback(this, event, true);
// event will be undefined, so we need to use window.event
event = event || _global.event;
if (!event) {
return;
}
// event.target is needed for Samsung TV and SourceBuffer
// || global is needed https://github.com/angular/zone.js/issues/190
const target: any = this || event.target || _global;
const tasks = target[zoneSymbolEventNames[event.type][TRUE_STR]];
if (tasks) {
// 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);
} 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();
for (let i = 0; i < copyTasks.length; i++) {
if (event && (event as any)[IMMEDIATE_PROPAGATION_SYMBOL] === true) {
break;
}
invokeTask(copyTasks[i], target, event);
}
}
}
}; };
function patchEventTargetMethods(obj: any, patchOptions?: PatchEventTargetOptions) { function patchEventTargetMethods(obj: any, patchOptions?: PatchEventTargetOptions) {

View File

@ -8,7 +8,7 @@
Zone.__load_patch('socketio', (global: any, Zone: ZoneType, api: _ZonePrivate) => { Zone.__load_patch('socketio', (global: any, Zone: ZoneType, api: _ZonePrivate) => {
(Zone as any)[Zone.__symbol__('socketio')] = function patchSocketIO(io: any) { (Zone as any)[Zone.__symbol__('socketio')] = function patchSocketIO(io: any) {
// patch io.Socket.prototype event listener related method // patch io.Socket.prototype event listener related method
api.patchEventTarget(global, [io.Socket.prototype], { api.patchEventTarget(global, api, [io.Socket.prototype], {
useG: false, useG: false,
chkDup: false, chkDup: false,
rt: true, rt: true,

View File

@ -8,7 +8,7 @@
import {patchEventTarget} from '../common/events'; import {patchEventTarget} from '../common/events';
Zone.__load_patch('EventEmitter', (global: any) => { Zone.__load_patch('EventEmitter', (global: any, Zone: ZoneType, api: _ZonePrivate) => {
// For EventEmitter // For EventEmitter
const EE_ADD_LISTENER = 'addListener'; const EE_ADD_LISTENER = 'addListener';
const EE_PREPEND_LISTENER = 'prependListener'; const EE_PREPEND_LISTENER = 'prependListener';
@ -34,7 +34,7 @@ Zone.__load_patch('EventEmitter', (global: any) => {
}; };
function patchEventEmitterMethods(obj: any) { function patchEventEmitterMethods(obj: any) {
const result = patchEventTarget(global, [obj], { const result = patchEventTarget(global, api, [obj], {
useG: false, useG: false,
add: EE_ADD_LISTENER, add: EE_ADD_LISTENER,
rm: EE_REMOVE_LISTENER, rm: EE_REMOVE_LISTENER,

View File

@ -337,7 +337,7 @@ interface _ZonePrivate {
onUnhandledError: (error: Error) => void; onUnhandledError: (error: Error) => void;
microtaskDrainDone: () => void; microtaskDrainDone: () => void;
showUncaughtError: () => boolean; showUncaughtError: () => boolean;
patchEventTarget: (global: any, apis: any[], options?: any) => boolean[]; patchEventTarget: (global: any, api: _ZonePrivate, apis: any[], options?: any) => boolean[];
patchOnProperties: (obj: any, properties: string[]|null, prototype?: any) => void; patchOnProperties: (obj: any, properties: string[]|null, prototype?: any) => void;
patchThen: (ctro: Function) => void; patchThen: (ctro: Function) => void;
patchMethod: patchMethod:
@ -359,6 +359,7 @@ interface _ZonePrivate {
filterProperties: (target: any, onProperties: string[], ignoreProperties: any[]) => string[]; filterProperties: (target: any, onProperties: string[], ignoreProperties: any[]) => string[];
attachOriginToPatched: (target: any, origin: any) => void; attachOriginToPatched: (target: any, origin: any) => void;
_redefineProperty: (target: any, callback: string, desc: any) => void; _redefineProperty: (target: any, callback: string, desc: any) => void;
nativeScheduleMicroTask: (func: Function) => void;
patchCallbacks: patchCallbacks:
(api: _ZonePrivate, target: any, targetName: string, method: string, (api: _ZonePrivate, target: any, targetName: string, method: string,
callbacks: string[]) => void; callbacks: string[]) => void;
@ -1343,27 +1344,31 @@ const Zone: ZoneType = (function(global: any) {
let _isDrainingMicrotaskQueue: boolean = false; let _isDrainingMicrotaskQueue: boolean = false;
let nativeMicroTaskQueuePromise: any; let nativeMicroTaskQueuePromise: any;
function nativeScheduleMicroTask(func: Function) {
if (!nativeMicroTaskQueuePromise) {
if (global[symbolPromise]) {
nativeMicroTaskQueuePromise = global[symbolPromise].resolve(0);
}
}
if (nativeMicroTaskQueuePromise) {
let nativeThen = nativeMicroTaskQueuePromise[symbolThen];
if (!nativeThen) {
// native Promise is not patchable, we need to use `then` directly
// issue 1078
nativeThen = nativeMicroTaskQueuePromise['then'];
}
nativeThen.call(nativeMicroTaskQueuePromise, func);
} else {
global[symbolSetTimeout](func, 0);
}
}
function scheduleMicroTask(task?: MicroTask) { function scheduleMicroTask(task?: MicroTask) {
// if we are not running in any task, and there has not been anything scheduled // if we are not running in any task, and there has not been anything scheduled
// we must bootstrap the initial task creation by manually scheduling the drain // we must bootstrap the initial task creation by manually scheduling the drain
if (_numberOfNestedTaskFrames === 0 && _microTaskQueue.length === 0) { if (_numberOfNestedTaskFrames === 0 && _microTaskQueue.length === 0) {
// We are not running in Task, so we need to kickstart the microtask queue. // We are not running in Task, so we need to kickstart the microtask queue.
if (!nativeMicroTaskQueuePromise) { nativeScheduleMicroTask(drainMicroTaskQueue);
if (global[symbolPromise]) {
nativeMicroTaskQueuePromise = global[symbolPromise].resolve(0);
}
}
if (nativeMicroTaskQueuePromise) {
let nativeThen = nativeMicroTaskQueuePromise[symbolThen];
if (!nativeThen) {
// native Promise is not patchable, we need to use `then` directly
// issue 1078
nativeThen = nativeMicroTaskQueuePromise['then'];
}
nativeThen.call(nativeMicroTaskQueuePromise, drainMicroTaskQueue);
} else {
global[symbolSetTimeout](drainMicroTaskQueue, 0);
}
} }
task && _microTaskQueue.push(task); task && _microTaskQueue.push(task);
} }
@ -1428,7 +1433,8 @@ const Zone: ZoneType = (function(global: any) {
filterProperties: () => [], filterProperties: () => [],
attachOriginToPatched: () => noop, attachOriginToPatched: () => noop,
_redefineProperty: () => noop, _redefineProperty: () => noop,
patchCallbacks: () => noop patchCallbacks: () => noop,
nativeScheduleMicroTask: nativeScheduleMicroTask
}; };
let _currentZoneFrame: _ZoneFrame = {parent: null, zone: new Zone(null, null)}; let _currentZoneFrame: _ZoneFrame = {parent: null, zone: new Zone(null, null)};
let _currentTask: Task|null = null; let _currentTask: Task|null = null;

View File

@ -348,7 +348,7 @@ describe('Zone', function() {
(HTMLSpanElement.prototype as any)[zoneSymbol('addEventListener')] = null; (HTMLSpanElement.prototype as any)[zoneSymbol('addEventListener')] = null;
patchEventTarget(window, [HTMLSpanElement.prototype]); patchEventTarget(window, null as any, [HTMLSpanElement.prototype]);
const span = document.createElement('span'); const span = document.createElement('span');
document.body.appendChild(span); document.body.appendChild(span);
@ -1037,7 +1037,7 @@ describe('Zone', function() {
})); }));
it('should change options to boolean if not support passive', () => { it('should change options to boolean if not support passive', () => {
patchEventTarget(window, [TestEventListener.prototype]); patchEventTarget(window, null as any, [TestEventListener.prototype]);
const testEventListener = new TestEventListener(); const testEventListener = new TestEventListener();
const listener = function() {}; const listener = function() {};
@ -2490,6 +2490,96 @@ describe('Zone', function() {
expect(hookSpy).not.toHaveBeenCalled(); expect(hookSpy).not.toHaveBeenCalled();
expect(logs).toEqual([]); expect(logs).toEqual([]);
})); }));
it('should be able to continue to invoke remaining listeners even some listener throw error',
function(done: DoneFn) {
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');
}
};
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) => {
logs.push(e.reason.message);
};
window.addEventListener('unhandledrejection', unhandledRejection);
button.dispatchEvent(mouseEvent);
expect(logs).toEqual(['listener1', 'listener2']);
setTimeout(() => {
expect(logs).toEqual(['listener1', 'listener2', 'test1', 'test2']);
window.removeEventListener('unhandledrejection', unhandledRejection);
done()
});
});
it('should be able to continue to invoke remaining listeners even some listener throw error in the different zones',
function(done: DoneFn) {
let logs: string[] = [];
const zone1 = Zone.current.fork({
name: 'zone1',
onHandleError: (delegate, curr, target, error) => {
logs.push(error.message);
return false;
}
});
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');
}
};
button.addEventListener('click', listener1);
zone1.run(() => {
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) => {
logs.push(e.reason.message);
};
window.addEventListener('unhandledrejection', unhandledRejection);
button.dispatchEvent(mouseEvent);
expect(logs).toEqual(['listener1', 'test1', 'listener2']);
setTimeout(() => {
expect(logs).toEqual(['listener1', 'test1', 'listener2', 'test2']);
done()
});
});
}); });
// TODO: Re-enable via https://github.com/angular/angular/pull/41526 // TODO: Re-enable via https://github.com/angular/angular/pull/41526