feat(change_detection): provide error context for change detection errors

This commit is contained in:
vsavkin 2015-07-23 18:01:34 -07:00
parent e744409cb9
commit c2bbda02a1
17 changed files with 147 additions and 36 deletions

View File

@ -7,6 +7,12 @@ import {ProtoRecord} from './proto_record';
import {Locals} from './parser/locals'; import {Locals} from './parser/locals';
import {CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED, ON_PUSH} from './constants'; import {CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED, ON_PUSH} from './constants';
class _Context {
constructor(public element: any, public componentElement: any, public instance: any,
public context: any, public locals: any, public injector: any,
public expression: any) {}
}
export class AbstractChangeDetector implements ChangeDetector { export class AbstractChangeDetector implements ChangeDetector {
lightDomChildren: List<any> = []; lightDomChildren: List<any> = [];
shadowDomChildren: List<any> = []; shadowDomChildren: List<any> = [];
@ -14,7 +20,7 @@ export class AbstractChangeDetector implements ChangeDetector {
mode: string = null; mode: string = null;
ref: ChangeDetectorRef; ref: ChangeDetectorRef;
constructor(public id: string) { this.ref = new ChangeDetectorRef(this); } constructor(public id: string, public dispatcher: any) { this.ref = new ChangeDetectorRef(this); }
addChild(cd: ChangeDetector): void { addChild(cd: ChangeDetector): void {
this.lightDomChildren.push(cd); this.lightDomChildren.push(cd);
@ -83,6 +89,9 @@ export class AbstractChangeDetector implements ChangeDetector {
} }
throwError(proto: ProtoRecord, exception: any, stack: any): void { throwError(proto: ProtoRecord, exception: any, stack: any): void {
throw new ChangeDetectionError(proto, exception, stack); 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);
throw new ChangeDetectionError(proto, exception, stack, context);
} }
} }

View File

@ -68,8 +68,7 @@ export class ChangeDetectorJITGenerator {
var typeName = _sanitizeName(`ChangeDetector_${this.id}`); var typeName = _sanitizeName(`ChangeDetector_${this.id}`);
var classDefinition = ` var classDefinition = `
var ${typeName} = function ${typeName}(dispatcher, protos, directiveRecords) { var ${typeName} = function ${typeName}(dispatcher, protos, directiveRecords) {
${ABSTRACT_CHANGE_DETECTOR}.call(this, ${JSON.stringify(this.id)}); ${ABSTRACT_CHANGE_DETECTOR}.call(this, ${JSON.stringify(this.id)}, dispatcher);
${DISPATCHER_ACCESSOR} = dispatcher;
${PROTOS_ACCESSOR} = protos; ${PROTOS_ACCESSOR} = protos;
${DIRECTIVES_ACCESSOR} = directiveRecords; ${DIRECTIVES_ACCESSOR} = directiveRecords;
${LOCALS_ACCESSOR} = null; ${LOCALS_ACCESSOR} = null;

View File

@ -129,7 +129,7 @@ export class ChangeDetectionUtil {
} }
static throwOnChange(proto: ProtoRecord, change) { static throwOnChange(proto: ProtoRecord, change) {
throw new ExpressionChangedAfterItHasBeenChecked(proto, change); throw new ExpressionChangedAfterItHasBeenChecked(proto, change, null);
} }
static throwDehydrated() { throw new DehydratedException(); } static throwDehydrated() { throw new DehydratedException(); }

View File

@ -20,9 +20,9 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
alreadyChecked: boolean = false; alreadyChecked: boolean = false;
private pipes: Pipes = null; private pipes: Pipes = null;
constructor(id: string, private changeControlStrategy: string, private dispatcher: any, constructor(id: string, private changeControlStrategy: string, dispatcher: any,
private protos: List<ProtoRecord>, private directiveRecords: List<any>) { private protos: List<ProtoRecord>, private directiveRecords: List<any>) {
super(id); super(id, dispatcher);
this.values = ListWrapper.createFixedSize(protos.length + 1); this.values = ListWrapper.createFixedSize(protos.length + 1);
this.localPipes = ListWrapper.createFixedSize(protos.length + 1); this.localPipes = ListWrapper.createFixedSize(protos.length + 1);
this.prevContexts = ListWrapper.createFixedSize(protos.length + 1); this.prevContexts = ListWrapper.createFixedSize(protos.length + 1);

View File

@ -2,7 +2,7 @@ import {ProtoRecord} from './proto_record';
import {BaseException} from "angular2/src/facade/lang"; import {BaseException} from "angular2/src/facade/lang";
export class ExpressionChangedAfterItHasBeenChecked extends BaseException { export class ExpressionChangedAfterItHasBeenChecked extends BaseException {
constructor(proto: ProtoRecord, change: any) { constructor(proto: ProtoRecord, change: any, context: any) {
super(`Expression '${proto.expressionAsString}' has changed after it was checked. ` + super(`Expression '${proto.expressionAsString}' has changed after it was checked. ` +
`Previous value: '${change.previousValue}'. Current value: '${change.currentValue}'`); `Previous value: '${change.previousValue}'. Current value: '${change.currentValue}'`);
} }
@ -11,9 +11,9 @@ export class ExpressionChangedAfterItHasBeenChecked extends BaseException {
export class ChangeDetectionError extends BaseException { export class ChangeDetectionError extends BaseException {
location: string; location: string;
constructor(proto: ProtoRecord, originalException: any, originalStack: any) { constructor(proto: ProtoRecord, originalException: any, originalStack: any, context: any) {
super(`${originalException} in [${proto.expressionAsString}]`, originalException, super(`${originalException} in [${proto.expressionAsString}]`, originalException, originalStack,
originalStack); context);
this.location = proto.expressionAsString; this.location = proto.expressionAsString;
} }
} }

View File

@ -128,7 +128,7 @@ function _injectorBindings(appComponentType): List<Type | Binding | List<any>> {
DirectiveResolver, DirectiveResolver,
Parser, Parser,
Lexer, Lexer,
bind(ExceptionHandler).toFactory(() => new ExceptionHandler(DOM)), bind(ExceptionHandler).toFactory(() => new ExceptionHandler(DOM), []),
bind(XHR).toValue(new XHRImpl()), bind(XHR).toValue(new XHRImpl()),
ComponentUrlMapper, ComponentUrlMapper,
UrlResolver, UrlResolver,

View File

@ -496,10 +496,9 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
private _debugContext(): any { private _debugContext(): any {
var p = this._preBuiltObjects; var p = this._preBuiltObjects;
var element = isPresent(p.elementRef) ? p.elementRef.nativeElement : null; var index = p.elementRef.boundElementIndex - p.view.elementOffset;
var hostRef = p.view.getHostElement(); var c = this._preBuiltObjects.view.getDebugContext(index, null);
var componentElement = isPresent(hostRef) ? hostRef.nativeElement : null; return new _Context(c["element"], c["componentElement"], c["injector"]);
return new _Context(element, componentElement, this._injector);
} }
private _reattachInjectors(imperativelyCreatedInjector: Injector): void { private _reattachInjectors(imperativelyCreatedInjector: Injector): void {
@ -573,6 +572,8 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
getComponent(): any { return this._strategy.getComponent(); } getComponent(): any { return this._strategy.getComponent(); }
getInjector(): Injector { return this._injector; }
getElementRef(): ElementRef { return this._preBuiltObjects.elementRef; } getElementRef(): ElementRef { return this._preBuiltObjects.elementRef; }
getViewContainerRef(): ViewContainerRef { getViewContainerRef(): ViewContainerRef {

View File

@ -1,4 +1,11 @@
import {ListWrapper, MapWrapper, Map, StringMapWrapper, List} from 'angular2/src/facade/collection'; import {
ListWrapper,
MapWrapper,
Map,
StringMapWrapper,
List,
StringMap
} from 'angular2/src/facade/collection';
import { import {
AST, AST,
Locals, Locals,
@ -202,7 +209,36 @@ export class AppView implements ChangeDispatcher, RenderEventDispatcher {
getHostElement(): ElementRef { getHostElement(): ElementRef {
var boundElementIndex = this.mainMergeMapping.hostElementIndicesByViewIndex[this.viewOffset]; var boundElementIndex = this.mainMergeMapping.hostElementIndicesByViewIndex[this.viewOffset];
return this.elementRefs[boundElementIndex]; return isPresent(boundElementIndex) ? this.elementRefs[boundElementIndex] : null;
}
getDebugContext(elementIndex: number, directiveIndex: DirectiveIndex): StringMap<string, any> {
try {
var offsettedIndex = this.elementOffset + elementIndex;
var hasRefForIndex = offsettedIndex < this.elementRefs.length;
var elementRef = hasRefForIndex ? this.elementRefs[this.elementOffset + elementIndex] : null;
var host = this.getHostElement();
var ei = hasRefForIndex ? this.elementInjectors[this.elementOffset + elementIndex] : null;
var element = isPresent(elementRef) ? elementRef.nativeElement : null;
var componentElement = isPresent(host) ? host.nativeElement : null;
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
};
} 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 {};
}
} }
getDetectorFor(directive: DirectiveIndex): any { getDetectorFor(directive: DirectiveIndex): any {
@ -252,6 +288,16 @@ export class AppView implements ChangeDispatcher, RenderEventDispatcher {
} }
} }
function _localsToStringMap(locals: Locals): StringMap<string, any> {
var res = {};
var c = locals;
while (isPresent(c)) {
res = StringMapWrapper.merge(res, MapWrapper.toStringMap(c.current));
c = c.parent;
}
return res;
}
/** /**
* *
*/ */

View File

@ -50,7 +50,7 @@ export class ExceptionHandler {
if (isPresent(stackTrace) && isBlank(originalStack)) { if (isPresent(stackTrace) && isBlank(originalStack)) {
this.logger.log("STACKTRACE:"); this.logger.log("STACKTRACE:");
this.logger.log(this._longStackTrace(stackTrace)) this.logger.log(this._longStackTrace(stackTrace));
} }
if (isPresent(reason)) { if (isPresent(reason)) {

View File

@ -105,9 +105,7 @@ export class InstantiationError extends AbstractBindingError {
constructor(injector: Injector, originalException, originalStack, key: Key) { constructor(injector: Injector, originalException, originalStack, key: Key) {
super(injector, key, function(keys: List<any>) { super(injector, key, function(keys: List<any>) {
var first = stringify(ListWrapper.first(keys).token); var first = stringify(ListWrapper.first(keys).token);
return `Error during instantiation of ${first}!${constructResolvingPath(keys)}.` + return `Error during instantiation of ${first}!${constructResolvingPath(keys)}.`;
`\n\n ORIGINAL ERROR: ${originalException}` +
`\n\n ORIGINAL STACK: ${originalStack} \n`;
}, originalException, originalStack); }, originalException, originalStack);
this.causeKey = key; this.causeKey = key;

View File

@ -34,7 +34,13 @@ class IterableMap extends IterableBase<List> {
class MapWrapper { class MapWrapper {
static Map clone(Map m) => new Map.from(m); static Map clone(Map m) => new Map.from(m);
// in opposite to JS, Dart does not create a new map
static Map createFromStringMap(Map m) => m; static Map createFromStringMap(Map m) => m;
// in opposite to JS, Dart does not create a new map
static Map toStringMap(Map m) => m;
static Map createFromPairs(List pairs) => pairs.fold({}, (m, p) { static Map createFromPairs(List pairs) => pairs.fold({}, (m, p) {
m[p[0]] = p[1]; m[p[0]] = p[1];
return m; return m;

View File

@ -62,6 +62,11 @@ export class MapWrapper {
} }
return result; return result;
} }
static toStringMap<T>(m: Map<string, T>): StringMap<string, T> {
var r = {};
m.forEach((v, k) => r[k] = v);
return r;
}
static createFromPairs(pairs: List<any>): Map<any, any> { return createMapFromPairs(pairs); } static createFromPairs(pairs: List<any>): Map<any, any> { return createMapFromPairs(pairs); }
static forEach<K, V>(m: Map<K, V>, fn: /*(V, K) => void*/ Function) { m.forEach(<any>fn); } static forEach<K, V>(m: Map<K, V>, fn: /*(V, K) => void*/ Function) { m.forEach(<any>fn); }
static get<K, V>(map: Map<K, V>, key: K): V { return map.get(key); } static get<K, V>(map: Map<K, V>, key: K): V { return map.get(key); }

View File

@ -124,7 +124,6 @@ class _CodegenState {
void _writeToBuf(StringBuffer buf) { void _writeToBuf(StringBuffer buf) {
buf.write('''\n buf.write('''\n
class $_changeDetectorTypeName extends $_BASE_CLASS { class $_changeDetectorTypeName extends $_BASE_CLASS {
final dynamic $_DISPATCHER_ACCESSOR;
$_GEN_PREFIX.Pipes $_PIPES_ACCESSOR; $_GEN_PREFIX.Pipes $_PIPES_ACCESSOR;
final $_GEN_PREFIX.List<$_GEN_PREFIX.ProtoRecord> $_PROTOS_ACCESSOR; final $_GEN_PREFIX.List<$_GEN_PREFIX.ProtoRecord> $_PROTOS_ACCESSOR;
final $_GEN_PREFIX.List<$_GEN_PREFIX.DirectiveRecord> final $_GEN_PREFIX.List<$_GEN_PREFIX.DirectiveRecord>
@ -140,9 +139,9 @@ class _CodegenState {
}).join('')} }).join('')}
$_changeDetectorTypeName( $_changeDetectorTypeName(
this.$_DISPATCHER_ACCESSOR, dynamic $_DISPATCHER_ACCESSOR,
this.$_PROTOS_ACCESSOR, this.$_PROTOS_ACCESSOR,
this.$_DIRECTIVES_ACCESSOR) : super(${_encodeValue(_changeDetectorDefId)}); this.$_DIRECTIVES_ACCESSOR) : super(${_encodeValue(_changeDetectorDefId)}, $_DISPATCHER_ACCESSOR);
void detectChangesInRecords(throwOnChange) { void detectChangesInRecords(throwOnChange) {
if (!hydrated()) { if (!hydrated()) {
@ -536,7 +535,7 @@ const _CHANGES_LOCAL = 'changes';
const _CONTEXT_ACCESSOR = '_context'; const _CONTEXT_ACCESSOR = '_context';
const _CURRENT_PROTO = 'currentProto'; const _CURRENT_PROTO = 'currentProto';
const _DIRECTIVES_ACCESSOR = '_directiveRecords'; const _DIRECTIVES_ACCESSOR = '_directiveRecords';
const _DISPATCHER_ACCESSOR = '_dispatcher'; const _DISPATCHER_ACCESSOR = 'dispatcher';
const _GEN_PREFIX = '_gen'; const _GEN_PREFIX = '_gen';
const _GEN_RECORDS_METHOD_NAME = '_createRecords'; const _GEN_RECORDS_METHOD_NAME = '_createRecords';
const _IDENTICAL_CHECK_FN = '$_GEN_PREFIX.looseIdentical'; const _IDENTICAL_CHECK_FN = '$_GEN_PREFIX.looseIdentical';

View File

@ -123,7 +123,7 @@ function _injectorBindings(appComponentType, bus: WorkerMessageBus,
DirectiveResolver, DirectiveResolver,
Parser, Parser,
Lexer, Lexer,
bind(ExceptionHandler).toFactory(() => {new ExceptionHandler(new PrintLogger())}), bind(ExceptionHandler).toFactory(() => new ExceptionHandler(new PrintLogger()), []),
bind(XHR).toValue(new XHRImpl()), bind(XHR).toValue(new XHRImpl()),
ComponentUrlMapper, ComponentUrlMapper,
UrlResolver, UrlResolver,

View File

@ -1043,6 +1043,10 @@ class TestDispatcher implements ChangeDispatcher {
notifyOnAllChangesDone() { this.onAllChangesDoneCalled = true; } notifyOnAllChangesDone() { this.onAllChangesDoneCalled = true; }
getDebugContext(a, b) {
return {}
}
_asString(value) { return (isBlank(value) ? 'null' : value.toString()); } _asString(value) { return (isBlank(value) ? 'null' : value.toString()); }
} }

View File

@ -1165,7 +1165,7 @@ export function main() {
}); });
})); }));
it('should provide an error context when an error happens in the DI', it('should provide an error context when an error happens in DI',
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
tcb = tcb.overrideView(MyComp, new viewAnn.View({ tcb = tcb.overrideView(MyComp, new viewAnn.View({
@ -1174,13 +1174,58 @@ export function main() {
})); }));
PromiseWrapper.catchError(tcb.createAsync(MyComp), (e) => { PromiseWrapper.catchError(tcb.createAsync(MyComp), (e) => {
expect(DOM.nodeName(e.context.element).toUpperCase()) var c = e.context;
.toEqual("DIRECTIVE-THROWING-ERROR"); expect(DOM.nodeName(c.element).toUpperCase()).toEqual("DIRECTIVE-THROWING-ERROR");
expect(DOM.nodeName(c.componentElement).toUpperCase()).toEqual("DIV");
expect(c.injector).toBeAnInstanceOf(Injector);
async.done(); async.done();
return null; return null;
}); });
})); }));
it('should provide an error context when an error happens in change detection',
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
tcb = tcb.overrideView(
MyComp, new viewAnn.View({template: `<input [value]="one.two.three" #local>`}));
tcb.createAsync(MyComp).then(rootTC => {
try {
rootTC.detectChanges();
throw "Should throw";
} catch (e) {
var c = e.context;
expect(DOM.nodeName(c.element).toUpperCase()).toEqual("INPUT");
expect(DOM.nodeName(c.componentElement).toUpperCase()).toEqual("DIV");
expect(c.injector).toBeAnInstanceOf(Injector);
expect(c.expression).toContain("one.two.three");
expect(c.context).toBe(rootTC.componentInstance);
expect(c.locals["local"]).not.toBeNull();
}
async.done();
});
}));
it('should provide an error context when an error happens in change detection (text node)',
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
tcb = tcb.overrideView(MyComp, new viewAnn.View({template: `{{one.two.three}}`}));
tcb.createAsync(MyComp).then(rootTC => {
try {
rootTC.detectChanges();
throw "Should throw";
} catch (e) {
var c = e.context;
expect(c.element).toBeNull();
expect(c.injector).toBeNull();
}
async.done();
});
}));
if (!IS_DARTIUM) { if (!IS_DARTIUM) {
it('should report a meaningful error when a directive is undefined', it('should report a meaningful error when a directive is undefined',
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder,

View File

@ -24,7 +24,6 @@ void initReflector() {
_MyComponent_ChangeDetector0.newProtoChangeDetector; _MyComponent_ChangeDetector0.newProtoChangeDetector;
} }
class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector { class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector {
final dynamic _dispatcher;
_gen.Pipes _pipes; _gen.Pipes _pipes;
final _gen.List<_gen.ProtoRecord> _protos; final _gen.List<_gen.ProtoRecord> _protos;
final _gen.List<_gen.DirectiveRecord> _directiveRecords; final _gen.List<_gen.DirectiveRecord> _directiveRecords;
@ -36,8 +35,8 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector {
dynamic _interpolate1 = _gen.ChangeDetectionUtil.uninitialized(); dynamic _interpolate1 = _gen.ChangeDetectionUtil.uninitialized();
_MyComponent_ChangeDetector0( _MyComponent_ChangeDetector0(
this._dispatcher, this._protos, this._directiveRecords) dynamic dispatcher, this._protos, this._directiveRecords)
: super("MyComponent_comp_0"); : super("MyComponent_comp_0", dispatcher);
void detectChangesInRecords(throwOnChange) { void detectChangesInRecords(throwOnChange) {
if (!hydrated()) { if (!hydrated()) {
@ -80,7 +79,7 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector {
_interpolate1, interpolate1)); _interpolate1, interpolate1));
} }
_dispatcher.notifyOnBinding(currentProto.bindingRecord, interpolate1); dispatcher.notifyOnBinding(currentProto.bindingRecord, interpolate1);
_interpolate1 = interpolate1; _interpolate1 = interpolate1;
} }
@ -95,7 +94,7 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector {
} }
void callOnAllChangesDone() { void callOnAllChangesDone() {
_dispatcher.notifyOnAllChangesDone(); dispatcher.notifyOnAllChangesDone();
} }
void hydrate(MyComponent context, locals, directives, pipes) { void hydrate(MyComponent context, locals, directives, pipes) {