From 7ae7573bc8ff782833b6d1974b4dc4ff0dc6b69f Mon Sep 17 00:00:00 2001 From: Vikram Subramanian Date: Thu, 20 Jul 2017 22:34:19 -0700 Subject: [PATCH] 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 --- packages/core/src/application_ref.ts | 15 +++++++++------ packages/core/src/zone/ng_zone.ts | 2 +- packages/core/test/application_ref_spec.ts | 12 ++++++++++-- packages/core/test/zone/ng_zone_spec.ts | 2 ++ 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/packages/core/src/application_ref.ts b/packages/core/src/application_ref.ts index 9cf06ea52e..ebb6a28b2d 100644 --- a/packages/core/src/application_ref.ts +++ b/packages/core/src/application_ref.ts @@ -231,12 +231,13 @@ export abstract class PlatformRef { abstract get destroyed(): boolean; } -function _callAndReportToErrorHandler(errorHandler: ErrorHandler, callback: () => any): any { +function _callAndReportToErrorHandler( + errorHandler: ErrorHandler, ngZone: NgZone, callback: () => any): any { try { const result = callback(); if (isPromise(result)) { return result.catch((e: any) => { - errorHandler.handleError(e); + ngZone.runOutsideAngular(() => errorHandler.handleError(e)); // rethrow as the exception handler might not do it throw e; }); @@ -244,7 +245,7 @@ function _callAndReportToErrorHandler(errorHandler: ErrorHandler, callback: () = return result; } catch (e) { - errorHandler.handleError(e); + ngZone.runOutsideAngular(() => errorHandler.handleError(e)); // rethrow as the exception handler might not do it throw e; } @@ -299,8 +300,10 @@ export class PlatformRef_ extends PlatformRef { throw new Error('No ErrorHandler. Is platform module (BrowserModule) included?'); } moduleRef.onDestroy(() => remove(this._modules, moduleRef)); - ngZone !.onError.subscribe({next: (error: any) => { exceptionHandler.handleError(error); }}); - return _callAndReportToErrorHandler(exceptionHandler, () => { + ngZone !.runOutsideAngular( + () => ngZone !.onError.subscribe( + {next: (error: any) => { exceptionHandler.handleError(error); }})); + return _callAndReportToErrorHandler(exceptionHandler, ngZone !, () => { const initStatus: ApplicationInitStatus = moduleRef.injector.get(ApplicationInitStatus); initStatus.runInitializers(); return initStatus.donePromise.then(() => { @@ -561,7 +564,7 @@ export class ApplicationRef_ extends ApplicationRef { } } catch (e) { // Attention: Don't rethrow as it could cancel subscriptions to Observables! - this._exceptionHandler.handleError(e); + this._zone.runOutsideAngular(() => this._exceptionHandler.handleError(e)); } finally { this._runningTick = false; wtfLeave(scope); diff --git a/packages/core/src/zone/ng_zone.ts b/packages/core/src/zone/ng_zone.ts index 1b32aafea5..8e77320e2b 100644 --- a/packages/core/src/zone/ng_zone.ts +++ b/packages/core/src/zone/ng_zone.ts @@ -258,7 +258,7 @@ function forkInnerZoneWithAngularBehavior(zone: NgZonePrivate) { onHandleError: (delegate: ZoneDelegate, current: Zone, target: Zone, error: any): boolean => { delegate.handleError(target, error); - zone.onError.emit(error); + zone.runOutsideAngular(() => zone.onError.emit(error)); return false; } }); diff --git a/packages/core/test/application_ref_spec.ts b/packages/core/test/application_ref_spec.ts index e3f4a3fdff..5e9977cb63 100644 --- a/packages/core/test/application_ref_spec.ts +++ b/packages/core/test/application_ref_spec.ts @@ -575,6 +575,14 @@ export function main() { class MockConsole { res: any[][] = []; - log(...args: any[]): void { this.res.push(args); } - error(...args: any[]): void { this.res.push(args); } + log(...args: any[]): void { + // 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); + } } diff --git a/packages/core/test/zone/ng_zone_spec.ts b/packages/core/test/zone/ng_zone_spec.ts index f44d0cee5a..cfe2dd29f6 100644 --- a/packages/core/test/zone/ng_zone_spec.ts +++ b/packages/core/test/zone/ng_zone_spec.ts @@ -31,6 +31,8 @@ const resolvedPromise = Promise.resolve(null); function logOnError() { _zone.onError.subscribe({ next: (error: any) => { + // Error handler should run outside of the Angular zone. + NgZone.assertNotInAngularZone(); _errors.push(error); _traces.push(error.stack); }