Skip to content

Commit

Permalink
feat(core): provide an error context when an exception happens in an …
Browse files Browse the repository at this point in the history
…error handler
  • Loading branch information
vsavkin committed Jul 28, 2015
1 parent 1d45029 commit 8543c34
Show file tree
Hide file tree
Showing 13 changed files with 175 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,9 @@ export class AbstractChangeDetector implements ChangeDetector {

throwError(proto: ProtoRecord, exception: any, stack: any): void {
var c = this.dispatcher.getDebugContext(proto.bindingRecord.elementIndex, proto.directiveIndex);
var context = new _Context(c["element"], c["componentElement"], c["directive"], c["context"],
c["locals"], c["injector"], proto.expressionAsString);
var context = isPresent(c) ? new _Context(c.element, c.componentElement, c.directive, c.context,
c.locals, c.injector, proto.expressionAsString) :
null;
throw new ChangeDetectionError(proto, exception, stack, context);
}
}
41 changes: 25 additions & 16 deletions modules/angular2/src/core/application_common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import {
BaseException,
assertionsEnabled,
print,
stringify
stringify,
isDart
} from 'angular2/src/facade/lang';
import {BrowserDomAdapter} from 'angular2/src/dom/browser_adapter';
import {DOM} from 'angular2/src/dom/dom_adapter';
Expand Down Expand Up @@ -128,7 +129,7 @@ function _injectorBindings(appComponentType): List<Type | Binding | List<any>> {
DirectiveResolver,
Parser,
Lexer,
bind(ExceptionHandler).toFactory(() => new ExceptionHandler(DOM), []),
bind(ExceptionHandler).toFactory(() => new ExceptionHandler(DOM, isDart ? false : true), []),
bind(XHR).toValue(new XHRImpl()),
ComponentUrlMapper,
UrlResolver,
Expand Down Expand Up @@ -280,8 +281,8 @@ export function commonBootstrap(
appComponentType: /*Type*/ any,
componentInjectableBindings: List<Type | Binding | List<any>> = null): Promise<ApplicationRef> {
BrowserDomAdapter.makeCurrent();
var bootstrapProcess: PromiseCompleter<any> = PromiseWrapper.completer();
var zone = createNgZone(new ExceptionHandler(DOM));
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.
Expand All @@ -290,19 +291,27 @@ export function commonBootstrap(
var exceptionHandler = appInjector.get(ExceptionHandler);
zone.overrideOnErrorHandler((e, s) => exceptionHandler.call(e, s));

var compRefToken: Promise<any> =
PromiseWrapper.wrap(() => appInjector.get(appComponentRefPromiseToken));
var tick = (componentRef) => {
var appChangeDetector = internalView(componentRef.hostView).changeDetector;
// retrieve life cycle: may have already been created if injected in root component
var lc = appInjector.get(LifeCycle);
lc.registerWith(zone, appChangeDetector);
lc.tick(); // the first tick that will bootstrap the app
try {
var compRefToken: Promise<any> = appInjector.get(appComponentRefPromiseToken);
var tick = (componentRef) => {
var appChangeDetector = internalView(componentRef.hostView).changeDetector;
// retrieve life cycle: may have already been created if injected in root component
var lc = appInjector.get(LifeCycle);
lc.registerWith(zone, appChangeDetector);
lc.tick(); // the first tick that will bootstrap the app

bootstrapProcess.resolve(new ApplicationRef(componentRef, appComponentType, appInjector));
};
PromiseWrapper.then(compRefToken, tick,
(err, stackTrace) => bootstrapProcess.reject(err, stackTrace));
bootstrapProcess.resolve(new ApplicationRef(componentRef, appComponentType, appInjector));
};

var tickResult = PromiseWrapper.then(compRefToken, tick);

PromiseWrapper.then(tickResult,
(_) => {}); // required for Dart to trigger the default error handler
PromiseWrapper.then(tickResult, null,
(err, stackTrace) => { bootstrapProcess.reject(err, stackTrace); });
} catch (e) {
bootstrapProcess.reject(e, e.stack);
}
});

return bootstrapProcess.promise;
Expand Down
2 changes: 1 addition & 1 deletion modules/angular2/src/core/compiler/element_injector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
var p = this._preBuiltObjects;
var index = p.elementRef.boundElementIndex - p.view.elementOffset;
var c = this._preBuiltObjects.view.getDebugContext(index, null);
return new _Context(c["element"], c["componentElement"], c["injector"]);
return isPresent(c) ? new _Context(c.element, c.componentElement, c.injector) : null;
}

private _reattachInjectors(imperativelyCreatedInjector: Injector): void {
Expand Down
90 changes: 58 additions & 32 deletions modules/angular2/src/core/compiler/view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ export class AppView implements ChangeDispatcher, RenderEventDispatcher {
return isPresent(boundElementIndex) ? this.elementRefs[boundElementIndex] : null;
}

getDebugContext(elementIndex: number, directiveIndex: DirectiveIndex): StringMap<string, any> {
getDebugContext(elementIndex: number, directiveIndex: DirectiveIndex): DebugContext {
try {
var offsettedIndex = this.elementOffset + elementIndex;
var hasRefForIndex = offsettedIndex < this.elementRefs.length;
Expand All @@ -226,18 +226,13 @@ export class AppView implements ChangeDispatcher, RenderEventDispatcher {
var directive = isPresent(directiveIndex) ? this.getDirectiveFor(directiveIndex) : null;
var injector = isPresent(ei) ? ei.getInjector() : null;

return {
element: element,
componentElement: componentElement,
directive: directive,
context: this.context,
locals: _localsToStringMap(this.locals),
injector: injector
};
return new DebugContext(element, componentElement, directive, this.context,
_localsToStringMap(this.locals), injector);

} catch (e) {
// TODO: vsavkin log the exception once we have a good way to log errors and warnings
// if an error happens during getting the debug context, we return an empty map.
return {};
return null;
}
}

Expand All @@ -262,29 +257,37 @@ export class AppView implements ChangeDispatcher, RenderEventDispatcher {

// returns false if preventDefault must be applied to the DOM event
dispatchEvent(boundElementIndex: number, eventName: string, locals: Map<string, any>): boolean {
// Most of the time the event will be fired only when the view is in the live document.
// However, in a rare circumstance the view might get dehydrated, in between the event
// queuing up and firing.
var allowDefaultBehavior = true;
if (this.hydrated()) {
var elBinder = this.proto.elementBinders[boundElementIndex - this.elementOffset];
if (isBlank(elBinder.hostListeners)) return allowDefaultBehavior;
var eventMap = elBinder.hostListeners[eventName];
if (isBlank(eventMap)) return allowDefaultBehavior;
MapWrapper.forEach(eventMap, (expr, directiveIndex) => {
var context;
if (directiveIndex === -1) {
context = this.context;
} else {
context = this.elementInjectors[boundElementIndex].getDirectiveAtIndex(directiveIndex);
}
var result = expr.eval(context, new Locals(this.locals, locals));
if (isPresent(result)) {
allowDefaultBehavior = allowDefaultBehavior && result == true;
}
});
try {
// Most of the time the event will be fired only when the view is in the live document.
// However, in a rare circumstance the view might get dehydrated, in between the event
// queuing up and firing.
var allowDefaultBehavior = true;
if (this.hydrated()) {
var elBinder = this.proto.elementBinders[boundElementIndex - this.elementOffset];
if (isBlank(elBinder.hostListeners)) return allowDefaultBehavior;
var eventMap = elBinder.hostListeners[eventName];
if (isBlank(eventMap)) return allowDefaultBehavior;
MapWrapper.forEach(eventMap, (expr, directiveIndex) => {
var context;
if (directiveIndex === -1) {
context = this.context;
} else {
context = this.elementInjectors[boundElementIndex].getDirectiveAtIndex(directiveIndex);
}
var result = expr.eval(context, new Locals(this.locals, locals));
if (isPresent(result)) {
allowDefaultBehavior = allowDefaultBehavior && result == true;
}
});
}
return allowDefaultBehavior;
} catch (e) {
var c = this.getDebugContext(boundElementIndex - this.elementOffset, null);
var context = isPresent(c) ? new _Context(c.element, c.componentElement, c.context, c.locals,
c.injector) :
null;
throw new EventEvaluationError(eventName, e, e.stack, context);
}
return allowDefaultBehavior;
}
}

Expand All @@ -298,6 +301,29 @@ function _localsToStringMap(locals: Locals): StringMap<string, any> {
return res;
}

export class DebugContext {
constructor(public element: any, public componentElement: any, public directive: any,
public context: any, public locals: any, public injector: any) {}
}

/**
* Error context included when an event handler throws an exception.
*/
class _Context {
constructor(public element: any, public componentElement: any, public context: any,
public locals: any, public injector: any) {}
}

/**
* Wraps an exception thrown by an event handler.
*/
class EventEvaluationError extends BaseException {
constructor(eventName: string, originalException: any, originalStack: any, context: any) {
super(`Error during evaluation of "${eventName}"`, originalException, originalStack, context);
}
}


/**
*
*/
Expand Down
2 changes: 1 addition & 1 deletion modules/angular2/src/core/exception_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export class ExceptionHandler {

_longStackTrace(stackTrace: any): any {
return isListLikeIterable(stackTrace) ? (<any>stackTrace).join("\n\n-----async gap-----\n") :
stackTrace;
stackTrace.toString();
}

_findContext(exception: any): any {
Expand Down
2 changes: 2 additions & 0 deletions modules/angular2/src/facade/lang.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import 'dart:math' as math;
import 'dart:convert' as convert;
import 'dart:async' show Future;

bool isDart = true;

String getTypeNameForDebugging(Type type) => type.toString();

class Math {
Expand Down
2 changes: 2 additions & 0 deletions modules/angular2/src/facade/lang.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ export function getTypeNameForDebugging(type: Type): string {
return type['name'];
}

export var isDart = false;

export class BaseException extends Error {
stack;
constructor(public message?: string, private _originalException?, private _originalStack?,
Expand Down
7 changes: 7 additions & 0 deletions modules/angular2/src/test_lib/fake_async.dart
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@ void tick([int millis = 0]) {
_fakeAsync.elapse(duration);
}

/**
* This is not needed in Dart. Because quiver correctly removes a timer when
* it throws an exception.
*/
void clearPendingTimers() {
}

/**
* Flush any pending microtasks.
*/
Expand Down
12 changes: 9 additions & 3 deletions modules/angular2/src/test_lib/fake_async.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ export function fakeAsync(fn: Function): Function {
return function(...args) {
// TODO(tbosch): This class should already be part of the jasmine typings but it is not...
_scheduler = new (<any>jasmine).DelayedFunctionScheduler();
ListWrapper.clear(_microtasks);
ListWrapper.clear(_pendingPeriodicTimers);
ListWrapper.clear(_pendingTimers);
clearPendingTimers();

let res = fakeAsyncZone.run(() => {
let res = fn(...args);
Expand All @@ -67,6 +65,14 @@ export function fakeAsync(fn: Function): Function {
}
}

// TODO we should fix tick to dequeue the failed timer instead of relying on clearPendingTimers
export function clearPendingTimers() {
ListWrapper.clear(_microtasks);
ListWrapper.clear(_pendingPeriodicTimers);
ListWrapper.clear(_pendingTimers);
}


/**
* Simulates the asynchronous passage of time for the timers in the fakeAsync zone.
*
Expand Down
1 change: 1 addition & 0 deletions modules/angular2/src/test_lib/test_lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export interface NgMatchers extends jasmine.Matchers {

export var expect: (actual: any) => NgMatchers = <any>_global.expect;

// TODO vsavkin: remove it and use lang/isDart instead
export var IS_DARTIUM = false;

export class AsyncTestCompleter {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1043,9 +1043,7 @@ class TestDispatcher implements ChangeDispatcher {

notifyOnAllChangesDone() { this.onAllChangesDoneCalled = true; }

getDebugContext(a, b) {
return {}
}
getDebugContext(a, b) { return null; }

_asString(value) { return (isBlank(value) ? 'null' : value.toString()); }
}
Expand Down
34 changes: 27 additions & 7 deletions modules/angular2/test/core/application_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,14 @@ class HelloRootMissingTemplate {
class HelloRootDirectiveIsNotCmp {
}

class _NilLogger {
log(s: any): void {}
logGroup(s: any): void {}
class _ArrayLogger {
res: any[] = [];
log(s: any): void { this.res.push(s); }
logGroup(s: any): void { this.res.push(s); }
logGroupEnd(){};
}


export function main() {
var fakeDoc, el, el2, testBindings, lightDom;

Expand All @@ -90,7 +92,7 @@ export function main() {

it('should throw if bootstrapped Directive is not a Component',
inject([AsyncTestCompleter], (async) => {
var refPromise = bootstrap(HelloRootDirectiveIsNotCmp, testBindings);
var refPromise = bootstrap(HelloRootDirectiveIsNotCmp, [testBindings]);

PromiseWrapper.then(refPromise, null, (exception) => {
expect(exception).toContainError(
Expand All @@ -101,17 +103,35 @@ export function main() {
}));

it('should throw if no element is found', inject([AsyncTestCompleter], (async) => {
// do not print errors to the console
var e = new ExceptionHandler(new _NilLogger());
var logger = new _ArrayLogger();
var exceptionHandler = new ExceptionHandler(logger, IS_DARTIUM ? false : true);

var refPromise = bootstrap(HelloRootCmp, [bind(ExceptionHandler).toValue(e)]);
var refPromise =
bootstrap(HelloRootCmp, [bind(ExceptionHandler).toValue(exceptionHandler)]);
PromiseWrapper.then(refPromise, null, (reason) => {
expect(reason.message).toContain('The selector "hello-app" did not match any elements');
async.done();
return null;
});
}));

if (DOM.supportsDOMEvents()) {
it('should invoke the default exception handler when bootstrap fails',
inject([AsyncTestCompleter], (async) => {
var logger = new _ArrayLogger();
var exceptionHandler = new ExceptionHandler(logger, IS_DARTIUM ? false : true);

var refPromise =
bootstrap(HelloRootCmp, [bind(ExceptionHandler).toValue(exceptionHandler)]);
PromiseWrapper.then(refPromise, null, (reason) => {
expect(logger.res.join(""))
.toContain('The selector "hello-app" did not match any elements');
async.done();
return null;
});
}));
}

it('should create an injector promise', () => {
var refPromise = bootstrap(HelloRootCmp, testBindings);
expect(refPromise).not.toBe(null);
Expand Down
Loading

0 comments on commit 8543c34

Please sign in to comment.