fix(errors): require passing stack traces explicitly in ng2 own code

Author: yjbanov

modules/angular2/src/core/application.js

@@ -259,8 +259,8 @@ export function bootstrap(appComponentType: Type,
         bootstrapProcess.resolve(new ApplicationRef(componentRef, appComponentType, appInjector));
       },
 
-      (err) => {
-        bootstrapProcess.reject(err)
+      (err, stackTrace) => {
+        bootstrapProcess.reject(err, stackTrace)
       });
   });
 

modules/angular2/src/di/injector.ts

@@ -338,17 +338,17 @@ class _AsyncInjectorStrategy {
     var deps = this.injector._resolveDependencies(key, binding, true);
     var depsPromise = PromiseWrapper.all(deps);
 
-    var promise = PromiseWrapper.then(depsPromise, null, (e) => this._errorHandler(key, e))
+    var promise = PromiseWrapper.then(depsPromise, null, (e, s) => this._errorHandler(key, e, s))
                       .then(deps => this._findOrCreate(key, binding, deps))
                       .then(instance => this._cacheInstance(key, instance));
 
     this.injector._setInstance(key, new _Waiting(promise));
     return promise;
   }
 
-  _errorHandler(key: Key, e): Promise<any> {
+  _errorHandler(key: Key, e, stack): Promise<any> {
     if (e instanceof AbstractBindingError) e.addKey(key);
-    return PromiseWrapper.reject(e);
+    return PromiseWrapper.reject(e, stack);
   }
 
   _findOrCreate(key: Key, binding: ResolvedBinding, deps: List<any>) {

modules/angular2/src/facade/async.dart

@@ -6,9 +6,13 @@ export 'dart:async' show Future, Stream, StreamController, StreamSubscription;
 class PromiseWrapper {
   static Future resolve(obj) => new Future.value(obj);
 
-  static Future reject(obj) => new Future.error(
+  static Future reject(obj, stackTrace) => new Future.error(
       obj,
-      obj is Error ? obj.stackTrace : null);
+      stackTrace != null
+        ? stackTrace
+        : obj is Error
+          ? obj.stackTrace
+          : null);
 
   static Future<List> all(List<Future> promises) => Future.wait(promises);
 
@@ -23,7 +27,7 @@ class PromiseWrapper {
     return promise.catchError(onError);
   }
 
-  static _Completer completer() => new _Completer(new Completer());
+  static CompleterWrapper completer() => new CompleterWrapper(new Completer());
 
   // TODO(vic): create a TimerWrapper
   static Timer setTimeout(fn(), int millis)
@@ -99,22 +103,21 @@ class EventEmitter extends Stream {
   }
 }
 
-class _Completer {
+class CompleterWrapper {
   final Completer c;
 
-  _Completer(this.c);
+  CompleterWrapper(this.c);
 
   Future get promise => c.future;
 
   void resolve(v) {
     c.complete(v);
   }
 
-  void reject(v) {
-    var stack = null;
-    if (v is Error) {
-      stack = v.stackTrace;
+  void reject(error, stack) {
+    if (stack == null && error is Error) {
+      stack = error.stackTrace;
     }
-    c.completeError(v, stack);
+    c.completeError(error, stack);
   }
 }

modules/angular2/src/facade/async.ts

@@ -15,7 +15,7 @@ export var Promise = (<any>global).Promise;
 export class PromiseWrapper {
   static resolve(obj): Promise<any> { return Promise.resolve(obj); }
 
-  static reject(obj): Promise<any> { return Promise.reject(obj); }
+  static reject(obj, _): Promise<any> { return Promise.reject(obj); }
 
   // Note: We can't rename this method into `catch`, as this is not a valid
   // method name in Dart.
@@ -29,7 +29,7 @@ export class PromiseWrapper {
   }
 
   static then<T>(promise: Promise<T>, success: (value: any) => T | Thenable<T>,
-                 rejection: (error: any) => T | Thenable<T>): Promise<T> {
+                 rejection: (error: any, stack?: any) => T | Thenable<T>): Promise<T> {
     return promise.then(success, rejection);
   }
 

modules/angular2/src/mock/xhr_mock.js

@@ -88,7 +88,7 @@ class _PendingRequest {
 
   complete(response: string) {
     if (isBlank(response)) {
-      this.completer.reject(`Failed to load ${this.url}`);
+      this.completer.reject(`Failed to load ${this.url}`, null);
     } else {
       this.completer.resolve(response);
     }

modules/angular2/test/core/compiler/integration_dart_spec.dart

@@ -7,6 +7,7 @@ import 'package:angular2/src/test_lib/test_bed.dart';
 import 'package:angular2/test_lib.dart';
 
 class MockException implements Error { var message; var stackTrace; }
+class NonError { var message; }
 
 void functionThatThrows() {
   try { throw new MockException(); }
@@ -19,6 +20,16 @@ void functionThatThrows() {
   }
 }
 
+void functionThatThrowsNonError() {
+  try { throw new NonError(); }
+  catch(e, stack) {
+    // If we lose the stack trace the message will no longer match
+    // the first line in the stack
+    e.message = stack.toString().split('\n')[0];
+    rethrow;
+  }
+}
+
 main() {
   describe('TypeLiteral', () {
     it('should publish via appInjector',
@@ -37,7 +48,7 @@ main() {
   });
 
   describe('Error handling', () {
-    it('should preserve stack traces throws from components',
+    it('should preserve Error stack traces thrown from components',
         inject([TestBed, AsyncTestCompleter], (tb, async) {
       tb.overrideView(Dummy, new View(
         template: '<throwing-component></throwing-component>',
@@ -49,6 +60,19 @@ main() {
         async.done();
       });
     }));
+
+    it('should preserve non-Error stack traces thrown from components',
+        inject([TestBed, AsyncTestCompleter], (tb, async) {
+      tb.overrideView(Dummy, new View(
+        template: '<throwing-component2></throwing-component2>',
+        directives: [ThrowingComponent2]
+      ));
+
+      tb.createView(Dummy).catchError((e, stack) {
+        expect(stack.toString().split('\n')[0]).toEqual(e.message);
+        async.done();
+      });
+    }));
   });
 }
 
@@ -81,3 +105,13 @@ class ThrowingComponent {
     functionThatThrows();
   }
 }
+
+@Component(
+  selector: 'throwing-component2'
+)
+@View(template: '')
+class ThrowingComponent2 {
+  ThrowingComponent2() {
+    functionThatThrowsNonError();
+  }
+}

modules/angular2/test/facade/async_dart_spec.dart

@@ -5,6 +5,7 @@ import 'package:angular2/test_lib.dart';
 import 'package:angular2/src/facade/async.dart';
 
 class MockException implements Error { var message; var stackTrace; }
+class NonError { var message; }
 
 void functionThatThrows() {
   try { throw new MockException(); }
@@ -18,7 +19,13 @@ void functionThatThrows() {
 }
 
 void functionThatThrowsNonError() {
-  throw 'this is an error';
+  try { throw new NonError(); }
+  catch(e, stack) {
+    // If we lose the stack trace the message will no longer match
+    // the first line in the stack
+    e.message = stack.toString().split('\n')[0];
+    rethrow;
+  }
 }
 
 void expectFunctionThatThrowsWithStackTrace(
@@ -29,72 +36,65 @@ void expectFunctionThatThrowsWithStackTrace(
   });
 }
 
-void expectFunctionThatThrowsWithoutStackTrace(Future future,
-    AsyncTestCompleter async) {
-  PromiseWrapper.catchError(future, (err, StackTrace stack) {
-    expect(stack).toBe(null);
-    async.done();
-  });
-}
-
 main() {
-  describe('Completer', () {
-
-    it('should preserve error stack traces',
-        inject([AsyncTestCompleter], (async) {
-      var c = PromiseWrapper.completer();
-
-      expectFunctionThatThrowsWithStackTrace(c.promise, async);
-
-      try {
-        functionThatThrows();
-      } catch(e) {
-        c.reject(e);
-      }
-    }));
+  describe('async facade', () {
+    describe('Completer', () {
 
-    // TODO: We might fix this one day; for now testing it to be explicit
-    it('CANNOT preserve error stack traces for non-Errors',
-        inject([AsyncTestCompleter], (async) {
-      var c = PromiseWrapper.completer();
-
-      expectFunctionThatThrowsWithoutStackTrace(c.promise, async);
-
-      try {
-        functionThatThrowsNonError();
-      } catch(e) {
-        c.reject(e);
-      }
-    }));
-
-  });
-
-  describe('PromiseWrapper', () {
+      it('should preserve Error stack traces',
+          inject([AsyncTestCompleter], (async) {
+        var c = PromiseWrapper.completer();
 
-    describe('reject', () {
+        expectFunctionThatThrowsWithStackTrace(c.promise, async);
 
-      it('should preserve error stack traces',
-          inject([AsyncTestCompleter], (async) {
         try {
           functionThatThrows();
         } catch(e) {
-          var rejectedFuture = PromiseWrapper.reject(e);
-          expectFunctionThatThrowsWithStackTrace(rejectedFuture, async);
+          c.reject(e, null);
         }
       }));
 
-      // TODO: We might fix this one day; for now testing it to be explicit
-      it('CANNOT preserve stack traces for non-Errors',
+      it('should preserve error stack traces for non-Errors',
           inject([AsyncTestCompleter], (async) {
+        var c = PromiseWrapper.completer();
+
+        expectFunctionThatThrowsWithStackTrace(c.promise, async);
+
         try {
           functionThatThrowsNonError();
-        } catch(e) {
-          var rejectedFuture = PromiseWrapper.reject(e);
-          expectFunctionThatThrowsWithoutStackTrace(rejectedFuture, async);
+        } catch(e, s) {
+          c.reject(e, s);
         }
       }));
 
     });
 
+    describe('PromiseWrapper', () {
+
+      describe('reject', () {
+
+        it('should preserve Error stack traces',
+            inject([AsyncTestCompleter], (async) {
+          try {
+            functionThatThrows();
+          } catch(e) {
+            var rejectedFuture = PromiseWrapper.reject(e, null);
+            expectFunctionThatThrowsWithStackTrace(rejectedFuture, async);
+          }
+        }));
+
+        it('should preserve stack traces for non-Errors',
+            inject([AsyncTestCompleter], (async) {
+          try {
+            functionThatThrowsNonError();
+          } catch(e, s) {
+            var rejectedFuture = PromiseWrapper.reject(e, s);
+            expectFunctionThatThrowsWithStackTrace(rejectedFuture, async);
+          }
+        }));
+
+      });
+
+    });
+
   });
 }

modules/angular2/test/render/dom/compiler/compiler_common_tests.js

@@ -219,7 +219,7 @@ class FakeTemplateLoader extends TemplateLoader {
       }
     }
 
-    return PromiseWrapper.reject('Load failed');
+    return PromiseWrapper.reject('Load failed', null);
   }
 }
 

modules/angular2/test/render/dom/shadow_dom/emulated_scoped_shadow_dom_strategy_spec.js

@@ -131,7 +131,7 @@ class FakeXHR extends XHR {
   get(url: string): Promise<string> {
     var response = MapWrapper.get(this._responses, url);
     if (isBlank(response)) {
-      return PromiseWrapper.reject('xhr error');
+      return PromiseWrapper.reject('xhr error', null);
     }
 
     return PromiseWrapper.resolve(response);

modules/angular2/test/render/dom/shadow_dom/style_inliner_spec.js

@@ -223,7 +223,7 @@ class FakeXHR extends XHR {
   get(url: string): Promise<string> {
     var response = MapWrapper.get(this._responses, url);
     if (isBlank(response)) {
-      return PromiseWrapper.reject('xhr error');
+      return PromiseWrapper.reject('xhr error', null);
     }
 
     return PromiseWrapper.resolve(response);