Skip to content

Commit 5c48cd3

Browse files
JiaLiPassionAndrewKushnir
JiaLiPassion
authored and
AndrewKushnir
committed
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
1 parent b64b6b3 commit 5c48cd3

File tree

11 files changed

+154
-64
lines changed

11 files changed

+154
-64
lines changed

‎packages/zone.js/lib/browser/browser.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ Zone.__load_patch('EventTarget', (global: any, Zone: ZoneType, api: _ZonePrivate
6666
// patch XMLHttpRequestEventTarget's addEventListener/removeEventListener
6767
const XMLHttpRequestEventTarget = (global as any)['XMLHttpRequestEventTarget'];
6868
if (XMLHttpRequestEventTarget && XMLHttpRequestEventTarget.prototype) {
69-
api.patchEventTarget(global, [XMLHttpRequestEventTarget.prototype]);
69+
api.patchEventTarget(global, api, [XMLHttpRequestEventTarget.prototype]);
7070
}
7171
});
7272

‎packages/zone.js/lib/browser/event-target-legacy.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ export function eventTargetLegacyPatch(_global: any, api: _ZonePrivate) {
112112
}
113113
// vh is validateHandler to check event handler
114114
// is valid or not(for security check)
115-
api.patchEventTarget(_global, apiTypes, {
115+
api.patchEventTarget(_global, api, apiTypes, {
116116
vh: checkIEAndCrossContext,
117117
transferEventName: (eventName: string) => {
118118
const pointerEventName = pointerEventsMap[eventName];

‎packages/zone.js/lib/browser/event-target.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export function eventTargetPatch(_global: any, api: _ZonePrivate) {
2929
if (!EVENT_TARGET || !EVENT_TARGET.prototype) {
3030
return;
3131
}
32-
api.patchEventTarget(_global, [EVENT_TARGET && EVENT_TARGET.prototype]);
32+
api.patchEventTarget(_global, api, [EVENT_TARGET && EVENT_TARGET.prototype]);
3333

3434
return true;
3535
}

‎packages/zone.js/lib/browser/shadydom.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ Zone.__load_patch('shadydom', (global: any, Zone: ZoneType, api: _ZonePrivate) =
2020
if (proto && proto.hasOwnProperty('addEventListener')) {
2121
proto[Zone.__symbol__('addEventListener')] = null;
2222
proto[Zone.__symbol__('removeEventListener')] = null;
23-
api.patchEventTarget(global, [proto]);
23+
api.patchEventTarget(global, api, [proto]);
2424
}
2525
});
2626
});

‎packages/zone.js/lib/browser/webapis-rtc-peer-connection.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,5 @@ Zone.__load_patch('RTCPeerConnection', (global: any, Zone: ZoneType, api: _ZoneP
2222
RTCPeerConnection.prototype[addSymbol] = null;
2323
RTCPeerConnection.prototype[removeSymbol] = null;
2424

25-
api.patchEventTarget(global, [RTCPeerConnection.prototype], {useG: false});
25+
api.patchEventTarget(global, api, [RTCPeerConnection.prototype], {useG: false});
2626
});

‎packages/zone.js/lib/browser/websocket.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export function apply(api: _ZonePrivate, _global: any) {
1313
// On Safari window.EventTarget doesn't exist so need to patch WS add/removeEventListener
1414
// On older Chrome, no need since EventTarget was already patched
1515
if (!(<any>_global).EventTarget) {
16-
api.patchEventTarget(_global, [WS.prototype]);
16+
api.patchEventTarget(_global, api, [WS.prototype]);
1717
}
1818
(<any>_global).WebSocket = function(x: any, y: any) {
1919
const socket = arguments.length > 1 ? new WS(x, y) : new WS(x);

‎packages/zone.js/lib/common/events.ts‎

Lines changed: 29 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ export interface PatchEventTargetOptions {
8686
}
8787

8888
export function patchEventTarget(
89-
_global: any, apis: any[], patchOptions?: PatchEventTargetOptions) {
89+
_global: any, api: _ZonePrivate, apis: any[], patchOptions?: PatchEventTargetOptions) {
9090
const ADD_EVENT_LISTENER = (patchOptions && patchOptions.add) || ADD_EVENT_LISTENER_STR;
9191
const REMOVE_EVENT_LISTENER = (patchOptions && patchOptions.rm) || REMOVE_EVENT_LISTENER_STR;
9292

@@ -114,7 +114,15 @@ export function patchEventTarget(
114114
task.originalDelegate = delegate;
115115
}
116116
// invoke static task.invoke
117-
task.invoke(task, target, [event]);
117+
// need to try/catch error here, otherwise, the error in one event listener
118+
// will break the executions of the other event listeners. Also error will
119+
// not remove the event listener when `once` options is true.
120+
let error;
121+
try {
122+
task.invoke(task, target, [event]);
123+
} catch (err) {
124+
error = err;
125+
}
118126
const options = task.options;
119127
if (options && typeof options === 'object' && options.once) {
120128
// if options.once is true, after invoke once remove listener here
@@ -123,10 +131,10 @@ export function patchEventTarget(
123131
const delegate = task.originalDelegate ? task.originalDelegate : task.callback;
124132
target[REMOVE_EVENT_LISTENER].call(target, event.type, delegate, options);
125133
}
134+
return error;
126135
};
127136

128-
// global shared zoneAwareCallback to handle all event callback with capture = false
129-
const globalZoneAwareCallback = function(this: unknown, event: Event) {
137+
function globalCallback(context: unknown, event: Event, isCapture: boolean) {
130138
// https://github.com/angular/zone.js/issues/911, in IE, sometimes
131139
// event will be undefined, so we need to use window.event
132140
event = event || _global.event;
@@ -135,8 +143,8 @@ export function patchEventTarget(
135143
}
136144
// event.target is needed for Samsung TV and SourceBuffer
137145
// || global is needed https://github.com/angular/zone.js/issues/190
138-
const target: any = this || event.target || _global;
139-
const tasks = target[zoneSymbolEventNames[event.type][FALSE_STR]];
146+
const target: any = context || event.target || _global;
147+
const tasks = target[zoneSymbolEventNames[event.type][isCapture ? TRUE_STR : FALSE_STR]];
140148
if (tasks) {
141149
// invoke all tasks which attached to current target with given event.type and capture = false
142150
// for performance concern, if task.length === 1, just invoke
@@ -147,46 +155,32 @@ export function patchEventTarget(
147155
// copy the tasks array before invoke, to avoid
148156
// the callback will remove itself or other listener
149157
const copyTasks = tasks.slice();
158+
const errors = [];
150159
for (let i = 0; i < copyTasks.length; i++) {
151160
if (event && (event as any)[IMMEDIATE_PROPAGATION_SYMBOL] === true) {
152161
break;
153162
}
154-
invokeTask(copyTasks[i], target, event);
163+
const err = invokeTask(copyTasks[i], target, event);
164+
err && errors.push(err);
165+
}
166+
for (let i = 0; i < errors.length; i++) {
167+
const err = errors[i];
168+
api.nativeScheduleMicroTask(() => {
169+
throw err;
170+
});
155171
}
156172
}
157173
}
174+
}
175+
176+
// global shared zoneAwareCallback to handle all event callback with capture = false
177+
const globalZoneAwareCallback = function(this: unknown, event: Event) {
178+
return globalCallback(this, event, false);
158179
};
159180

160181
// global shared zoneAwareCallback to handle all event callback with capture = true
161182
const globalZoneAwareCaptureCallback = function(this: unknown, event: Event) {
162-
// https://github.com/angular/zone.js/issues/911, in IE, sometimes
163-
// event will be undefined, so we need to use window.event
164-
event = event || _global.event;
165-
if (!event) {
166-
return;
167-
}
168-
// event.target is needed for Samsung TV and SourceBuffer
169-
// || global is needed https://github.com/angular/zone.js/issues/190
170-
const target: any = this || event.target || _global;
171-
const tasks = target[zoneSymbolEventNames[event.type][TRUE_STR]];
172-
if (tasks) {
173-
// invoke all tasks which attached to current target with given event.type and capture = false
174-
// for performance concern, if task.length === 1, just invoke
175-
if (tasks.length === 1) {
176-
invokeTask(tasks[0], target, event);
177-
} else {
178-
// https://github.com/angular/zone.js/issues/836
179-
// copy the tasks array before invoke, to avoid
180-
// the callback will remove itself or other listener
181-
const copyTasks = tasks.slice();
182-
for (let i = 0; i < copyTasks.length; i++) {
183-
if (event && (event as any)[IMMEDIATE_PROPAGATION_SYMBOL] === true) {
184-
break;
185-
}
186-
invokeTask(copyTasks[i], target, event);
187-
}
188-
}
189-
}
183+
return globalCallback(this, event, true);
190184
};
191185

192186
function patchEventTargetMethods(obj: any, patchOptions?: PatchEventTargetOptions) {

‎packages/zone.js/lib/extra/socket-io.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
Zone.__load_patch('socketio', (global: any, Zone: ZoneType, api: _ZonePrivate) => {
99
(Zone as any)[Zone.__symbol__('socketio')] = function patchSocketIO(io: any) {
1010
// patch io.Socket.prototype event listener related method
11-
api.patchEventTarget(global, [io.Socket.prototype], {
11+
api.patchEventTarget(global, api, [io.Socket.prototype], {
1212
useG: false,
1313
chkDup: false,
1414
rt: true,

‎packages/zone.js/lib/node/events.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import {patchEventTarget} from '../common/events';
1010

11-
Zone.__load_patch('EventEmitter', (global: any) => {
11+
Zone.__load_patch('EventEmitter', (global: any, Zone: ZoneType, api: _ZonePrivate) => {
1212
// For EventEmitter
1313
const EE_ADD_LISTENER = 'addListener';
1414
const EE_PREPEND_LISTENER = 'prependListener';
@@ -34,7 +34,7 @@ Zone.__load_patch('EventEmitter', (global: any) => {
3434
};
3535

3636
function patchEventEmitterMethods(obj: any) {
37-
const result = patchEventTarget(global, [obj], {
37+
const result = patchEventTarget(global, api, [obj], {
3838
useG: false,
3939
add: EE_ADD_LISTENER,
4040
rm: EE_REMOVE_LISTENER,

‎packages/zone.js/lib/zone.ts‎

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ interface _ZonePrivate {
337337
onUnhandledError: (error: Error) => void;
338338
microtaskDrainDone: () => void;
339339
showUncaughtError: () => boolean;
340-
patchEventTarget: (global: any, apis: any[], options?: any) => boolean[];
340+
patchEventTarget: (global: any, api: _ZonePrivate, apis: any[], options?: any) => boolean[];
341341
patchOnProperties: (obj: any, properties: string[]|null, prototype?: any) => void;
342342
patchThen: (ctro: Function) => void;
343343
patchMethod:
@@ -359,6 +359,7 @@ interface _ZonePrivate {
359359
filterProperties: (target: any, onProperties: string[], ignoreProperties: any[]) => string[];
360360
attachOriginToPatched: (target: any, origin: any) => void;
361361
_redefineProperty: (target: any, callback: string, desc: any) => void;
362+
nativeScheduleMicroTask: (func: Function) => void;
362363
patchCallbacks:
363364
(api: _ZonePrivate, target: any, targetName: string, method: string,
364365
callbacks: string[]) => void;
@@ -1343,27 +1344,31 @@ const Zone: ZoneType = (function(global: any) {
13431344
let _isDrainingMicrotaskQueue: boolean = false;
13441345
let nativeMicroTaskQueuePromise: any;
13451346

1347+
function nativeScheduleMicroTask(func: Function) {
1348+
if (!nativeMicroTaskQueuePromise) {
1349+
if (global[symbolPromise]) {
1350+
nativeMicroTaskQueuePromise = global[symbolPromise].resolve(0);
1351+
}
1352+
}
1353+
if (nativeMicroTaskQueuePromise) {
1354+
let nativeThen = nativeMicroTaskQueuePromise[symbolThen];
1355+
if (!nativeThen) {
1356+
// native Promise is not patchable, we need to use `then` directly
1357+
// issue 1078
1358+
nativeThen = nativeMicroTaskQueuePromise['then'];
1359+
}
1360+
nativeThen.call(nativeMicroTaskQueuePromise, func);
1361+
} else {
1362+
global[symbolSetTimeout](func, 0);
1363+
}
1364+
}
1365+
13461366
function scheduleMicroTask(task?: MicroTask) {
13471367
// if we are not running in any task, and there has not been anything scheduled
13481368
// we must bootstrap the initial task creation by manually scheduling the drain
13491369
if (_numberOfNestedTaskFrames === 0 && _microTaskQueue.length === 0) {
13501370
// We are not running in Task, so we need to kickstart the microtask queue.
1351-
if (!nativeMicroTaskQueuePromise) {
1352-
if (global[symbolPromise]) {
1353-
nativeMicroTaskQueuePromise = global[symbolPromise].resolve(0);
1354-
}
1355-
}
1356-
if (nativeMicroTaskQueuePromise) {
1357-
let nativeThen = nativeMicroTaskQueuePromise[symbolThen];
1358-
if (!nativeThen) {
1359-
// native Promise is not patchable, we need to use `then` directly
1360-
// issue 1078
1361-
nativeThen = nativeMicroTaskQueuePromise['then'];
1362-
}
1363-
nativeThen.call(nativeMicroTaskQueuePromise, drainMicroTaskQueue);
1364-
} else {
1365-
global[symbolSetTimeout](drainMicroTaskQueue, 0);
1366-
}
1371+
nativeScheduleMicroTask(drainMicroTaskQueue);
13671372
}
13681373
task && _microTaskQueue.push(task);
13691374
}
@@ -1428,7 +1433,8 @@ const Zone: ZoneType = (function(global: any) {
14281433
filterProperties: () => [],
14291434
attachOriginToPatched: () => noop,
14301435
_redefineProperty: () => noop,
1431-
patchCallbacks: () => noop
1436+
patchCallbacks: () => noop,
1437+
nativeScheduleMicroTask: nativeScheduleMicroTask
14321438
};
14331439
let _currentZoneFrame: _ZoneFrame = {parent: null, zone: new Zone(null, null)};
14341440
let _currentTask: Task|null = null;

0 commit comments

Comments
 (0)