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

This commit is contained in:
Yegor Jbanov 2015-05-19 12:48:00 -07:00
parent 5c88f662cd
commit 8ab773538b
11 changed files with 112 additions and 75 deletions

View File

@ -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)
});
});

View File

@ -338,7 +338,7 @@ 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));
@ -346,9 +346,9 @@ class _AsyncInjectorStrategy {
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>) {

View File

@ -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,10 +103,10 @@ class EventEmitter extends Stream {
}
}
class _Completer {
class CompleterWrapper {
final Completer c;
_Completer(this.c);
CompleterWrapper(this.c);
Future get promise => c.future;
@ -110,11 +114,10 @@ class _Completer {
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);
}
}

View File

@ -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);
}

View File

@ -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);
}

View File

@ -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();
}
}

View File

@ -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,18 +36,11 @@ void expectFunctionThatThrowsWithStackTrace(
});
}
void expectFunctionThatThrowsWithoutStackTrace(Future future,
AsyncTestCompleter async) {
PromiseWrapper.catchError(future, (err, StackTrace stack) {
expect(stack).toBe(null);
async.done();
});
}
main() {
describe('async facade', () {
describe('Completer', () {
it('should preserve error stack traces',
it('should preserve Error stack traces',
inject([AsyncTestCompleter], (async) {
var c = PromiseWrapper.completer();
@ -49,21 +49,20 @@ main() {
try {
functionThatThrows();
} catch(e) {
c.reject(e);
c.reject(e, null);
}
}));
// TODO: We might fix this one day; for now testing it to be explicit
it('CANNOT preserve error stack traces for non-Errors',
it('should preserve error stack traces for non-Errors',
inject([AsyncTestCompleter], (async) {
var c = PromiseWrapper.completer();
expectFunctionThatThrowsWithoutStackTrace(c.promise, async);
expectFunctionThatThrowsWithStackTrace(c.promise, async);
try {
functionThatThrowsNonError();
} catch(e) {
c.reject(e);
} catch(e, s) {
c.reject(e, s);
}
}));
@ -73,28 +72,29 @@ main() {
describe('reject', () {
it('should preserve error stack traces',
it('should preserve Error stack traces',
inject([AsyncTestCompleter], (async) {
try {
functionThatThrows();
} catch(e) {
var rejectedFuture = PromiseWrapper.reject(e);
var rejectedFuture = PromiseWrapper.reject(e, null);
expectFunctionThatThrowsWithStackTrace(rejectedFuture, async);
}
}));
// 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 stack traces for non-Errors',
inject([AsyncTestCompleter], (async) {
try {
functionThatThrowsNonError();
} catch(e) {
var rejectedFuture = PromiseWrapper.reject(e);
expectFunctionThatThrowsWithoutStackTrace(rejectedFuture, async);
} catch(e, s) {
var rejectedFuture = PromiseWrapper.reject(e, s);
expectFunctionThatThrowsWithStackTrace(rejectedFuture, async);
}
}));
});
});
});
}

View File

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

View File

@ -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);

View File

@ -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);

View File

@ -76,8 +76,8 @@ module.exports = function(gulp, plugins, config) {
return;
}
}
// TODO(yjbanov): fix ng2 code and remove the check below
if (line.match(/_stack/)) {
// TODO: https://github.com/angular/ts2dart/issues/168
if (line.match(/_stack' is not used/)) {
return;
}