fix(exception_handler): log errors that are thrown by the compiler

vsavkin · vsavkin · commit 07b9be798c1c · 2015-08-06T15:26:24.000-07:00
diff --git a/modules/angular2/src/core/application_common.ts b/modules/angular2/src/core/application_common.ts
@@ -158,13 +158,8 @@ function _injectorBindings(appComponentType): List<Type | Binding | List<any>> {
   ];
 }
 
-export function createNgZone(handler: ExceptionHandler): NgZone {
-  // bootstrapErrorReporter is needed because we cannot use custom exception handler
-  // configured via DI until the root Injector has been created.
-  var bootstrapErrorReporter = (exception, stackTrace) => handler.call(exception, stackTrace);
-  var zone = new NgZone({enableLongStackTrace: assertionsEnabled()});
-  zone.overrideOnErrorHandler(bootstrapErrorReporter);
-  return zone;
+export function createNgZone(): NgZone {
+  return new NgZone({enableLongStackTrace: assertionsEnabled()});
 }
 
 /**
@@ -299,16 +294,16 @@ export function commonBootstrap(
   BrowserDomAdapter.makeCurrent();
   wtfInit();
   var bootstrapProcess = PromiseWrapper.completer();
-  var zone = createNgZone(new ExceptionHandler(DOM, isDart ? false : true));
-  zone.run(() => {
-    // TODO(rado): prepopulate template cache, so applications with only
-    // index.html and main.js are possible.
+  var zone = createNgZone();
 
-    var appInjector = _createAppInjector(appComponentType, componentInjectableBindings, zone);
-    var exceptionHandler = appInjector.get(ExceptionHandler);
-    zone.overrideOnErrorHandler((e, s) => exceptionHandler.call(e, s));
+  zone.run(() => {
+    var exceptionHandler;
 
     try {
+      var appInjector = _createAppInjector(appComponentType, componentInjectableBindings, zone);
+      exceptionHandler = appInjector.get(ExceptionHandler);
+      zone.overrideOnErrorHandler((e, s) => exceptionHandler.call(e, s));
+
       var compRefToken: Promise<any> = appInjector.get(appComponentRefPromiseToken);
       var tick = (componentRef) => {
         var appChangeDetector = internalView(componentRef.hostView).changeDetector;
@@ -327,10 +322,17 @@ export function commonBootstrap(
       PromiseWrapper.then(tickResult, null,
                           (err, stackTrace) => { bootstrapProcess.reject(err, stackTrace); });
     } catch (e) {
+      if (isPresent(exceptionHandler)) {
+        exceptionHandler.call(e, e.stack);
+      } else {
+        // The error happened during the creation of an injector, most likely because of a bug in
+        // DI.
+        // We cannot use the provided exception handler, so we default to writing to the DOM.
+        DOM.logError(e);
+      }
       bootstrapProcess.reject(e, e.stack);
     }
   });
-
   return bootstrapProcess.promise;
 }
 
diff --git a/modules/angular2/src/web-workers/ui/impl.ts b/modules/angular2/src/web-workers/ui/impl.ts
@@ -29,10 +29,8 @@ import {
 import {createNgZone} from 'angular2/src/core/application_common';
 import {WorkerElementRef} from 'angular2/src/web-workers/shared/api';
 import {AnchorBasedAppRootUrl} from 'angular2/src/services/anchor_based_app_root_url';
-import {ExceptionHandler} from 'angular2/src/core/exception_handler';
 import {Injectable} from 'angular2/di';
 import {BrowserDomAdapter} from 'angular2/src/dom/browser_adapter';
-import {DOM} from 'angular2/src/dom/dom_adapter';
 import {
   serializeMouseEvent,
   serializeKeyboardEvent,
@@ -46,7 +44,7 @@ import {
  */
 export function bootstrapUICommon(bus: MessageBus) {
   BrowserDomAdapter.makeCurrent();
-  var zone = createNgZone(new ExceptionHandler(DOM));
+  var zone = createNgZone();
   zone.run(() => {
     var injector = createInjector(zone);
     var webWorkerMain = injector.get(WebWorkerMain);
diff --git a/modules/angular2/src/web-workers/worker/application_common.ts b/modules/angular2/src/web-workers/worker/application_common.ts
@@ -139,7 +139,7 @@ export function bootstrapWebworkerCommon(
     componentInjectableBindings: List<Type | Binding | List<any>> = null): Promise<ApplicationRef> {
   var bootstrapProcess: PromiseCompleter<any> = PromiseWrapper.completer();
 
-  var zone = createNgZone(new ExceptionHandler(new PrintLogger()));
+  var zone = createNgZone();
   zone.run(() => {
     // TODO(rado): prepopulate template cache, so applications with only
     // index.html and main.js are possible.
diff --git a/modules/angular2/test/core/application_spec.ts b/modules/angular2/test/core/application_spec.ts
@@ -92,11 +92,16 @@ export function main() {
 
     it('should throw if bootstrapped Directive is not a Component',
        inject([AsyncTestCompleter], (async) => {
-         var refPromise = bootstrap(HelloRootDirectiveIsNotCmp, [testBindings]);
+         var logger = new _ArrayLogger();
+         var exceptionHandler = new ExceptionHandler(logger, false);
+         var refPromise =
+             bootstrap(HelloRootDirectiveIsNotCmp,
+                       [testBindings, bind(ExceptionHandler).toValue(exceptionHandler)]);
 
          PromiseWrapper.then(refPromise, null, (exception) => {
            expect(exception).toContainError(
                `Could not load '${stringify(HelloRootDirectiveIsNotCmp)}' because it is not a component.`);
+           expect(logger.res.join("")).toContain("Could not load");
            async.done();
            return null;
          });
