Skip to content

Commit

Permalink
fix(core): ErrorHandler should not rethrow an error by default (#15077)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mhevery committed Mar 16, 2017
1 parent 41f61b0 commit 31117a6
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 15 deletions.
10 changes: 2 additions & 8 deletions packages/core/src/error_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,9 @@ export class ErrorHandler {
_console: Console = console;

/**
* @internal
* @deprecated since v4.0 parameter no longer has an effect, as ErrorHandler will never rethrow.
*/
rethrowError: boolean;

constructor(rethrowError: boolean = true) { this.rethrowError = rethrowError; }
constructor(deprecatedArgument?: boolean) {}

handleError(error: any): void {
const originalError = this._findOriginalError(error);
Expand All @@ -63,10 +61,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 */
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/application_ref_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/error_handler_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions packages/platform-browser/test/browser/bootstrap_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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}
Expand All @@ -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 =
Expand All @@ -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 =
Expand Down

0 comments on commit 31117a6

Please sign in to comment.