From 77fd91d615c937857e0959a40c713d92727f641f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mi=C5=A1ko=20Hevery?= Date: Thu, 16 Mar 2017 12:58:41 -0700 Subject: [PATCH] fix(core): ErrorHandler should not rethrow an error by default (#15077) (#15208) ErrorHandler can not throw errors because it will unsubscribe itself from the error stream. Zones captures errors and feed it into NgZone, which than has a Rx Observable to feed it into ErrorHandler. If the ErroHandler throws, then Rx will teardown the observable which in essence causes the ErrorHandler to be removed from the error handling. This implies that the ErrorHandler can never throw errors. Closes #14949 Closes #15182 Closes #14316 --- packages/core/src/error_handler.ts | 16 ++++++---------- packages/core/test/application_ref_spec.ts | 2 +- packages/core/test/error_handler_spec.ts | 4 ++-- .../test/browser/bootstrap_spec.ts | 8 ++++---- tools/public_api_guard/core/typings/core.d.ts | 3 ++- 5 files changed, 15 insertions(+), 18 deletions(-) diff --git a/packages/core/src/error_handler.ts b/packages/core/src/error_handler.ts index e4438ef315..decb71692a 100644 --- a/packages/core/src/error_handler.ts +++ b/packages/core/src/error_handler.ts @@ -42,12 +42,12 @@ export class ErrorHandler { */ _console: Console = console; - /** - * @internal - */ - rethrowError: boolean; - - constructor(rethrowError: boolean = true) { this.rethrowError = rethrowError; } + constructor( + /** + * @deprecated since v4.0 parameter no longer has an effect, as ErrorHandler will never + * rethrow. + */ + deprecatedParameter?: boolean) {} handleError(error: any): void { const originalError = this._findOriginalError(error); @@ -63,10 +63,6 @@ export class ErrorHandler { if (context) { errorLogger(this._console, 'ERROR CONTEXT', context); } - - // We rethrow exceptions, so operations like 'bootstrap' will result in an error - // when an error happens. If we do not rethrow, bootstrap will always succeed. - if (this.rethrowError) throw error; } /** @internal */ diff --git a/packages/core/test/application_ref_spec.ts b/packages/core/test/application_ref_spec.ts index 7fdfaf8442..5c137fc546 100644 --- a/packages/core/test/application_ref_spec.ts +++ b/packages/core/test/application_ref_spec.ts @@ -52,7 +52,7 @@ export function main() { } else { options = providersOrOptions || {}; } - const errorHandler = new ErrorHandler(false); + const errorHandler = new ErrorHandler(); errorHandler._console = mockConsole as any; const platformModule = getDOM().supportsDOMEvents() ? BrowserModule : ServerModule; diff --git a/packages/core/test/error_handler_spec.ts b/packages/core/test/error_handler_spec.ts index d63366884a..3d296c8af6 100644 --- a/packages/core/test/error_handler_spec.ts +++ b/packages/core/test/error_handler_spec.ts @@ -18,7 +18,7 @@ class MockConsole { export function main() { function errorToString(error: any) { const logger = new MockConsole(); - const errorHandler = new ErrorHandler(false); + const errorHandler = new ErrorHandler(); errorHandler._console = logger as any; errorHandler.handleError(error); return logger.res.map(line => line.join('#')).join('\n'); @@ -61,7 +61,7 @@ ERROR CONTEXT#Context`); it('should use the error logger on the error', () => { const err = new Error('test'); const console = new MockConsole(); - const errorHandler = new ErrorHandler(false); + const errorHandler = new ErrorHandler(); errorHandler._console = console as any; const logger = jasmine.createSpy('logger'); (err as any)[ERROR_LOGGER] = logger; diff --git a/packages/platform-browser/test/browser/bootstrap_spec.ts b/packages/platform-browser/test/browser/bootstrap_spec.ts index 059cb997c7..ad2851092c 100644 --- a/packages/platform-browser/test/browser/bootstrap_spec.ts +++ b/packages/platform-browser/test/browser/bootstrap_spec.ts @@ -159,7 +159,7 @@ export function main() { it('should throw if bootstrapped Directive is not a Component', inject([AsyncTestCompleter], (done: AsyncTestCompleter) => { const logger = new MockConsole(); - const errorHandler = new ErrorHandler(false); + const errorHandler = new ErrorHandler(); errorHandler._console = logger as any; expect( () => bootstrap( @@ -171,7 +171,7 @@ export function main() { it('should throw if no element is found', inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { const logger = new MockConsole(); - const errorHandler = new ErrorHandler(false); + const errorHandler = new ErrorHandler(); errorHandler._console = logger as any; bootstrap(NonExistentComp, [ {provide: ErrorHandler, useValue: errorHandler} @@ -187,7 +187,7 @@ export function main() { it('should forward the error to promise when bootstrap fails', inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { const logger = new MockConsole(); - const errorHandler = new ErrorHandler(false); + const errorHandler = new ErrorHandler(); errorHandler._console = logger as any; const refPromise = @@ -202,7 +202,7 @@ export function main() { it('should invoke the default exception handler when bootstrap fails', inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { const logger = new MockConsole(); - const errorHandler = new ErrorHandler(false); + const errorHandler = new ErrorHandler(); errorHandler._console = logger as any; const refPromise = diff --git a/tools/public_api_guard/core/typings/core.d.ts b/tools/public_api_guard/core/typings/core.d.ts index 1868d6001c..b9336c75c2 100644 --- a/tools/public_api_guard/core/typings/core.d.ts +++ b/tools/public_api_guard/core/typings/core.d.ts @@ -383,7 +383,8 @@ export declare function enableProdMode(): void; /** @stable */ export declare class ErrorHandler { - constructor(rethrowError?: boolean); + constructor( + deprecatedParameter?: boolean); handleError(error: any): void; }