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
This commit is contained in:
Miško Hevery 2017-03-16 12:58:41 -07:00 committed by Chuck Jazdzewski
parent 492153a986
commit 77fd91d615
5 changed files with 15 additions and 18 deletions

View File

@ -42,12 +42,12 @@ export class ErrorHandler {
*/ */
_console: Console = console; _console: Console = console;
constructor(
/** /**
* @internal * @deprecated since v4.0 parameter no longer has an effect, as ErrorHandler will never
* rethrow.
*/ */
rethrowError: boolean; deprecatedParameter?: boolean) {}
constructor(rethrowError: boolean = true) { this.rethrowError = rethrowError; }
handleError(error: any): void { handleError(error: any): void {
const originalError = this._findOriginalError(error); const originalError = this._findOriginalError(error);
@ -63,10 +63,6 @@ export class ErrorHandler {
if (context) { if (context) {
errorLogger(this._console, 'ERROR CONTEXT', 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 */ /** @internal */

View File

@ -52,7 +52,7 @@ export function main() {
} else { } else {
options = providersOrOptions || {}; options = providersOrOptions || {};
} }
const errorHandler = new ErrorHandler(false); const errorHandler = new ErrorHandler();
errorHandler._console = mockConsole as any; errorHandler._console = mockConsole as any;
const platformModule = getDOM().supportsDOMEvents() ? BrowserModule : ServerModule; const platformModule = getDOM().supportsDOMEvents() ? BrowserModule : ServerModule;

View File

@ -18,7 +18,7 @@ class MockConsole {
export function main() { export function main() {
function errorToString(error: any) { function errorToString(error: any) {
const logger = new MockConsole(); const logger = new MockConsole();
const errorHandler = new ErrorHandler(false); const errorHandler = new ErrorHandler();
errorHandler._console = logger as any; errorHandler._console = logger as any;
errorHandler.handleError(error); errorHandler.handleError(error);
return logger.res.map(line => line.join('#')).join('\n'); 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', () => { it('should use the error logger on the error', () => {
const err = new Error('test'); const err = new Error('test');
const console = new MockConsole(); const console = new MockConsole();
const errorHandler = new ErrorHandler(false); const errorHandler = new ErrorHandler();
errorHandler._console = console as any; errorHandler._console = console as any;
const logger = jasmine.createSpy('logger'); const logger = jasmine.createSpy('logger');
(err as any)[ERROR_LOGGER] = logger; (err as any)[ERROR_LOGGER] = logger;

View File

@ -159,7 +159,7 @@ export function main() {
it('should throw if bootstrapped Directive is not a Component', it('should throw if bootstrapped Directive is not a Component',
inject([AsyncTestCompleter], (done: AsyncTestCompleter) => { inject([AsyncTestCompleter], (done: AsyncTestCompleter) => {
const logger = new MockConsole(); const logger = new MockConsole();
const errorHandler = new ErrorHandler(false); const errorHandler = new ErrorHandler();
errorHandler._console = logger as any; errorHandler._console = logger as any;
expect( expect(
() => bootstrap( () => bootstrap(
@ -171,7 +171,7 @@ export function main() {
it('should throw if no element is found', it('should throw if no element is found',
inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { inject([AsyncTestCompleter], (async: AsyncTestCompleter) => {
const logger = new MockConsole(); const logger = new MockConsole();
const errorHandler = new ErrorHandler(false); const errorHandler = new ErrorHandler();
errorHandler._console = logger as any; errorHandler._console = logger as any;
bootstrap(NonExistentComp, [ bootstrap(NonExistentComp, [
{provide: ErrorHandler, useValue: errorHandler} {provide: ErrorHandler, useValue: errorHandler}
@ -187,7 +187,7 @@ export function main() {
it('should forward the error to promise when bootstrap fails', it('should forward the error to promise when bootstrap fails',
inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { inject([AsyncTestCompleter], (async: AsyncTestCompleter) => {
const logger = new MockConsole(); const logger = new MockConsole();
const errorHandler = new ErrorHandler(false); const errorHandler = new ErrorHandler();
errorHandler._console = logger as any; errorHandler._console = logger as any;
const refPromise = const refPromise =
@ -202,7 +202,7 @@ export function main() {
it('should invoke the default exception handler when bootstrap fails', it('should invoke the default exception handler when bootstrap fails',
inject([AsyncTestCompleter], (async: AsyncTestCompleter) => { inject([AsyncTestCompleter], (async: AsyncTestCompleter) => {
const logger = new MockConsole(); const logger = new MockConsole();
const errorHandler = new ErrorHandler(false); const errorHandler = new ErrorHandler();
errorHandler._console = logger as any; errorHandler._console = logger as any;
const refPromise = const refPromise =

View File

@ -383,7 +383,8 @@ export declare function enableProdMode(): void;
/** @stable */ /** @stable */
export declare class ErrorHandler { export declare class ErrorHandler {
constructor(rethrowError?: boolean); constructor(
deprecatedParameter?: boolean);
handleError(error: any): void; handleError(error: any): void;
} }