fix(core): invoke error handler outside of the Angular Zone (#18269)

In Node.JS console.log/error/warn functions actually resuls in a socket
write which in turn is considered by Zone.js as an async task.

This means that if there is any exception during change detection in a platform-server
application the error handler will make the Angular Zone unstable which
in turn will cause change detection to run on next tick and cause an
infinite loop.

It is also better to run the error handler outside of the Angular Zone
in general on all platforms so that an error in the error handler itself doesn't cause an
infinite loop.

Fixes #17073, #7774.

PR Close #18269
This commit is contained in:
Vikram Subramanian 2017-07-20 22:34:19 -07:00 committed by Miško Hevery
parent abee785821
commit 7ae7573bc8
4 changed files with 22 additions and 9 deletions

View File

@ -231,12 +231,13 @@ export abstract class PlatformRef {
abstract get destroyed(): boolean; abstract get destroyed(): boolean;
} }
function _callAndReportToErrorHandler(errorHandler: ErrorHandler, callback: () => any): any { function _callAndReportToErrorHandler(
errorHandler: ErrorHandler, ngZone: NgZone, callback: () => any): any {
try { try {
const result = callback(); const result = callback();
if (isPromise(result)) { if (isPromise(result)) {
return result.catch((e: any) => { return result.catch((e: any) => {
errorHandler.handleError(e); ngZone.runOutsideAngular(() => errorHandler.handleError(e));
// rethrow as the exception handler might not do it // rethrow as the exception handler might not do it
throw e; throw e;
}); });
@ -244,7 +245,7 @@ function _callAndReportToErrorHandler(errorHandler: ErrorHandler, callback: () =
return result; return result;
} catch (e) { } catch (e) {
errorHandler.handleError(e); ngZone.runOutsideAngular(() => errorHandler.handleError(e));
// rethrow as the exception handler might not do it // rethrow as the exception handler might not do it
throw e; throw e;
} }
@ -299,8 +300,10 @@ export class PlatformRef_ extends PlatformRef {
throw new Error('No ErrorHandler. Is platform module (BrowserModule) included?'); throw new Error('No ErrorHandler. Is platform module (BrowserModule) included?');
} }
moduleRef.onDestroy(() => remove(this._modules, moduleRef)); moduleRef.onDestroy(() => remove(this._modules, moduleRef));
ngZone !.onError.subscribe({next: (error: any) => { exceptionHandler.handleError(error); }}); ngZone !.runOutsideAngular(
return _callAndReportToErrorHandler(exceptionHandler, () => { () => ngZone !.onError.subscribe(
{next: (error: any) => { exceptionHandler.handleError(error); }}));
return _callAndReportToErrorHandler(exceptionHandler, ngZone !, () => {
const initStatus: ApplicationInitStatus = moduleRef.injector.get(ApplicationInitStatus); const initStatus: ApplicationInitStatus = moduleRef.injector.get(ApplicationInitStatus);
initStatus.runInitializers(); initStatus.runInitializers();
return initStatus.donePromise.then(() => { return initStatus.donePromise.then(() => {
@ -561,7 +564,7 @@ export class ApplicationRef_ extends ApplicationRef {
} }
} catch (e) { } catch (e) {
// Attention: Don't rethrow as it could cancel subscriptions to Observables! // Attention: Don't rethrow as it could cancel subscriptions to Observables!
this._exceptionHandler.handleError(e); this._zone.runOutsideAngular(() => this._exceptionHandler.handleError(e));
} finally { } finally {
this._runningTick = false; this._runningTick = false;
wtfLeave(scope); wtfLeave(scope);

View File

@ -258,7 +258,7 @@ function forkInnerZoneWithAngularBehavior(zone: NgZonePrivate) {
onHandleError: (delegate: ZoneDelegate, current: Zone, target: Zone, error: any): boolean => { onHandleError: (delegate: ZoneDelegate, current: Zone, target: Zone, error: any): boolean => {
delegate.handleError(target, error); delegate.handleError(target, error);
zone.onError.emit(error); zone.runOutsideAngular(() => zone.onError.emit(error));
return false; return false;
} }
}); });

View File

@ -575,6 +575,14 @@ export function main() {
class MockConsole { class MockConsole {
res: any[][] = []; res: any[][] = [];
log(...args: any[]): void { this.res.push(args); } log(...args: any[]): void {
error(...args: any[]): void { this.res.push(args); } // Logging from ErrorHandler should run outside of the Angular Zone.
NgZone.assertNotInAngularZone();
this.res.push(args);
}
error(...args: any[]): void {
// Logging from ErrorHandler should run outside of the Angular Zone.
NgZone.assertNotInAngularZone();
this.res.push(args);
}
} }

View File

@ -31,6 +31,8 @@ const resolvedPromise = Promise.resolve(null);
function logOnError() { function logOnError() {
_zone.onError.subscribe({ _zone.onError.subscribe({
next: (error: any) => { next: (error: any) => {
// Error handler should run outside of the Angular zone.
NgZone.assertNotInAngularZone();
_errors.push(error); _errors.push(error);
_traces.push(error.stack); _traces.push(error.stack);
} }