Skip to content

Commit c2bbda0

Browse files
vsavkinvsavkin
committed
feat(change_detection): provide error context for change detection errors
1 parent e744409 commit c2bbda0

File tree

17 files changed

+147
-36
lines changed

17 files changed

+147
-36
lines changed

‎modules/angular2/src/change_detection/abstract_change_detector.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,20 @@ import {ProtoRecord} from './proto_record';
77
import {Locals} from './parser/locals';
88
import {CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED, ON_PUSH} from './constants';
99

10+
class _Context {
11+
constructor(public element: any, public componentElement: any, public instance: any,
12+
public context: any, public locals: any, public injector: any,
13+
public expression: any) {}
14+
}
15+
1016
export class AbstractChangeDetector implements ChangeDetector {
1117
lightDomChildren: List<any> = [];
1218
shadowDomChildren: List<any> = [];
1319
parent: ChangeDetector;
1420
mode: string = null;
1521
ref: ChangeDetectorRef;
1622

17-
constructor(public id: string) { this.ref = new ChangeDetectorRef(this); }
23+
constructor(public id: string, public dispatcher: any) { this.ref = new ChangeDetectorRef(this); }
1824

1925
addChild(cd: ChangeDetector): void {
2026
this.lightDomChildren.push(cd);
@@ -83,6 +89,9 @@ export class AbstractChangeDetector implements ChangeDetector {
8389
}
8490

8591
throwError(proto: ProtoRecord, exception: any, stack: any): void {
86-
throw new ChangeDetectionError(proto, exception, stack);
92+
var c = this.dispatcher.getDebugContext(proto.bindingRecord.elementIndex, proto.directiveIndex);
93+
var context = new _Context(c["element"], c["componentElement"], c["directive"], c["context"],
94+
c["locals"], c["injector"], proto.expressionAsString);
95+
throw new ChangeDetectionError(proto, exception, stack, context);
8796
}
88-
}
97+
}

‎modules/angular2/src/change_detection/change_detection_jit_generator.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,7 @@ export class ChangeDetectorJITGenerator {
6868
var typeName = _sanitizeName(`ChangeDetector_${this.id}`);
6969
var classDefinition = `
7070
var ${typeName} = function ${typeName}(dispatcher, protos, directiveRecords) {
71-
${ABSTRACT_CHANGE_DETECTOR}.call(this, ${JSON.stringify(this.id)});
72-
${DISPATCHER_ACCESSOR} = dispatcher;
71+
${ABSTRACT_CHANGE_DETECTOR}.call(this, ${JSON.stringify(this.id)}, dispatcher);
7372
${PROTOS_ACCESSOR} = protos;
7473
${DIRECTIVES_ACCESSOR} = directiveRecords;
7574
${LOCALS_ACCESSOR} = null;

‎modules/angular2/src/change_detection/change_detection_util.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export class ChangeDetectionUtil {
129129
}
130130

131131
static throwOnChange(proto: ProtoRecord, change) {
132-
throw new ExpressionChangedAfterItHasBeenChecked(proto, change);
132+
throw new ExpressionChangedAfterItHasBeenChecked(proto, change, null);
133133
}
134134

135135
static throwDehydrated() { throw new DehydratedException(); }

‎modules/angular2/src/change_detection/dynamic_change_detector.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
2020
alreadyChecked: boolean = false;
2121
private pipes: Pipes = null;
2222

23-
constructor(id: string, private changeControlStrategy: string, private dispatcher: any,
23+
constructor(id: string, private changeControlStrategy: string, dispatcher: any,
2424
private protos: List<ProtoRecord>, private directiveRecords: List<any>) {
25-
super(id);
25+
super(id, dispatcher);
2626
this.values = ListWrapper.createFixedSize(protos.length + 1);
2727
this.localPipes = ListWrapper.createFixedSize(protos.length + 1);
2828
this.prevContexts = ListWrapper.createFixedSize(protos.length + 1);

‎modules/angular2/src/change_detection/exceptions.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import {ProtoRecord} from './proto_record';
22
import {BaseException} from "angular2/src/facade/lang";
33

44
export class ExpressionChangedAfterItHasBeenChecked extends BaseException {
5-
constructor(proto: ProtoRecord, change: any) {
5+
constructor(proto: ProtoRecord, change: any, context: any) {
66
super(`Expression '${proto.expressionAsString}' has changed after it was checked. ` +
77
`Previous value: '${change.previousValue}'. Current value: '${change.currentValue}'`);
88
}
@@ -11,9 +11,9 @@ export class ExpressionChangedAfterItHasBeenChecked extends BaseException {
1111
export class ChangeDetectionError extends BaseException {
1212
location: string;
1313

14-
constructor(proto: ProtoRecord, originalException: any, originalStack: any) {
15-
super(`${originalException} in [${proto.expressionAsString}]`, originalException,
16-
originalStack);
14+
constructor(proto: ProtoRecord, originalException: any, originalStack: any, context: any) {
15+
super(`${originalException} in [${proto.expressionAsString}]`, originalException, originalStack,
16+
context);
1717
this.location = proto.expressionAsString;
1818
}
1919
}

‎modules/angular2/src/core/application_common.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ function _injectorBindings(appComponentType): List<Type | Binding | List<any>> {
128128
DirectiveResolver,
129129
Parser,
130130
Lexer,
131-
bind(ExceptionHandler).toFactory(() => new ExceptionHandler(DOM)),
131+
bind(ExceptionHandler).toFactory(() => new ExceptionHandler(DOM), []),
132132
bind(XHR).toValue(new XHRImpl()),
133133
ComponentUrlMapper,
134134
UrlResolver,

‎modules/angular2/src/core/compiler/element_injector.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -496,10 +496,9 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
496496

497497
private _debugContext(): any {
498498
var p = this._preBuiltObjects;
499-
var element = isPresent(p.elementRef) ? p.elementRef.nativeElement : null;
500-
var hostRef = p.view.getHostElement();
501-
var componentElement = isPresent(hostRef) ? hostRef.nativeElement : null;
502-
return new _Context(element, componentElement, this._injector);
499+
var index = p.elementRef.boundElementIndex - p.view.elementOffset;
500+
var c = this._preBuiltObjects.view.getDebugContext(index, null);
501+
return new _Context(c["element"], c["componentElement"], c["injector"]);
503502
}
504503

505504
private _reattachInjectors(imperativelyCreatedInjector: Injector): void {
@@ -573,6 +572,8 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
573572

574573
getComponent(): any { return this._strategy.getComponent(); }
575574

575+
getInjector(): Injector { return this._injector; }
576+
576577
getElementRef(): ElementRef { return this._preBuiltObjects.elementRef; }
577578

578579
getViewContainerRef(): ViewContainerRef {

‎modules/angular2/src/core/compiler/view.ts

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
1-
import {ListWrapper, MapWrapper, Map, StringMapWrapper, List} from 'angular2/src/facade/collection';
1+
import {
2+
ListWrapper,
3+
MapWrapper,
4+
Map,
5+
StringMapWrapper,
6+
List,
7+
StringMap
8+
} from 'angular2/src/facade/collection';
29
import {
310
AST,
411
Locals,
@@ -202,7 +209,36 @@ export class AppView implements ChangeDispatcher, RenderEventDispatcher {
202209

203210
getHostElement(): ElementRef {
204211
var boundElementIndex = this.mainMergeMapping.hostElementIndicesByViewIndex[this.viewOffset];
205-
return this.elementRefs[boundElementIndex];
212+
return isPresent(boundElementIndex) ? this.elementRefs[boundElementIndex] : null;
213+
}
214+
215+
getDebugContext(elementIndex: number, directiveIndex: DirectiveIndex): StringMap<string, any> {
216+
try {
217+
var offsettedIndex = this.elementOffset + elementIndex;
218+
var hasRefForIndex = offsettedIndex < this.elementRefs.length;
219+
220+
var elementRef = hasRefForIndex ? this.elementRefs[this.elementOffset + elementIndex] : null;
221+
var host = this.getHostElement();
222+
var ei = hasRefForIndex ? this.elementInjectors[this.elementOffset + elementIndex] : null;
223+
224+
var element = isPresent(elementRef) ? elementRef.nativeElement : null;
225+
var componentElement = isPresent(host) ? host.nativeElement : null;
226+
var directive = isPresent(directiveIndex) ? this.getDirectiveFor(directiveIndex) : null;
227+
var injector = isPresent(ei) ? ei.getInjector() : null;
228+
229+
return {
230+
element: element,
231+
componentElement: componentElement,
232+
directive: directive,
233+
context: this.context,
234+
locals: _localsToStringMap(this.locals),
235+
injector: injector
236+
};
237+
} catch (e) {
238+
// TODO: vsavkin log the exception once we have a good way to log errors and warnings
239+
// if an error happens during getting the debug context, we return an empty map.
240+
return {};
241+
}
206242
}
207243

208244
getDetectorFor(directive: DirectiveIndex): any {
@@ -252,6 +288,16 @@ export class AppView implements ChangeDispatcher, RenderEventDispatcher {
252288
}
253289
}
254290

291+
function _localsToStringMap(locals: Locals): StringMap<string, any> {
292+
var res = {};
293+
var c = locals;
294+
while (isPresent(c)) {
295+
res = StringMapWrapper.merge(res, MapWrapper.toStringMap(c.current));
296+
c = c.parent;
297+
}
298+
return res;
299+
}
300+
255301
/**
256302
*
257303
*/

‎modules/angular2/src/core/exception_handler.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export class ExceptionHandler {
5050

5151
if (isPresent(stackTrace) && isBlank(originalStack)) {
5252
this.logger.log("STACKTRACE:");
53-
this.logger.log(this._longStackTrace(stackTrace))
53+
this.logger.log(this._longStackTrace(stackTrace));
5454
}
5555

5656
if (isPresent(reason)) {

‎modules/angular2/src/di/exceptions.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,7 @@ export class InstantiationError extends AbstractBindingError {
105105
constructor(injector: Injector, originalException, originalStack, key: Key) {
106106
super(injector, key, function(keys: List<any>) {
107107
var first = stringify(ListWrapper.first(keys).token);
108-
return `Error during instantiation of ${first}!${constructResolvingPath(keys)}.` +
109-
`\n\n ORIGINAL ERROR: ${originalException}` +
110-
`\n\n ORIGINAL STACK: ${originalStack} \n`;
108+
return `Error during instantiation of ${first}!${constructResolvingPath(keys)}.`;
111109
}, originalException, originalStack);
112110

113111
this.causeKey = key;

‎modules/angular2/src/facade/collection.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,13 @@ class IterableMap extends IterableBase<List> {
3434

3535
class MapWrapper {
3636
static Map clone(Map m) => new Map.from(m);
37+
38+
// in opposite to JS, Dart does not create a new map
3739
static Map createFromStringMap(Map m) => m;
40+
41+
// in opposite to JS, Dart does not create a new map
42+
static Map toStringMap(Map m) => m;
43+
3844
static Map createFromPairs(List pairs) => pairs.fold({}, (m, p) {
3945
m[p[0]] = p[1];
4046
return m;

‎modules/angular2/src/facade/collection.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,11 @@ export class MapWrapper {
6262
}
6363
return result;
6464
}
65+
static toStringMap<T>(m: Map<string, T>): StringMap<string, T> {
66+
var r = {};
67+
m.forEach((v, k) => r[k] = v);
68+
return r;
69+
}
6570
static createFromPairs(pairs: List<any>): Map<any, any> { return createMapFromPairs(pairs); }
6671
static forEach<K, V>(m: Map<K, V>, fn: /*(V, K) => void*/ Function) { m.forEach(<any>fn); }
6772
static get<K, V>(map: Map<K, V>, key: K): V { return map.get(key); }

‎modules/angular2/src/transform/template_compiler/change_detector_codegen.dart

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,6 @@ class _CodegenState {
124124
void _writeToBuf(StringBuffer buf) {
125125
buf.write('''\n
126126
class $_changeDetectorTypeName extends $_BASE_CLASS {
127-
final dynamic $_DISPATCHER_ACCESSOR;
128127
$_GEN_PREFIX.Pipes $_PIPES_ACCESSOR;
129128
final $_GEN_PREFIX.List<$_GEN_PREFIX.ProtoRecord> $_PROTOS_ACCESSOR;
130129
final $_GEN_PREFIX.List<$_GEN_PREFIX.DirectiveRecord>
@@ -140,9 +139,9 @@ class _CodegenState {
140139
}).join('')}
141140
142141
$_changeDetectorTypeName(
143-
this.$_DISPATCHER_ACCESSOR,
142+
dynamic $_DISPATCHER_ACCESSOR,
144143
this.$_PROTOS_ACCESSOR,
145-
this.$_DIRECTIVES_ACCESSOR) : super(${_encodeValue(_changeDetectorDefId)});
144+
this.$_DIRECTIVES_ACCESSOR) : super(${_encodeValue(_changeDetectorDefId)}, $_DISPATCHER_ACCESSOR);
146145
147146
void detectChangesInRecords(throwOnChange) {
148147
if (!hydrated()) {
@@ -536,7 +535,7 @@ const _CHANGES_LOCAL = 'changes';
536535
const _CONTEXT_ACCESSOR = '_context';
537536
const _CURRENT_PROTO = 'currentProto';
538537
const _DIRECTIVES_ACCESSOR = '_directiveRecords';
539-
const _DISPATCHER_ACCESSOR = '_dispatcher';
538+
const _DISPATCHER_ACCESSOR = 'dispatcher';
540539
const _GEN_PREFIX = '_gen';
541540
const _GEN_RECORDS_METHOD_NAME = '_createRecords';
542541
const _IDENTICAL_CHECK_FN = '$_GEN_PREFIX.looseIdentical';

‎modules/angular2/src/web-workers/worker/application_common.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ function _injectorBindings(appComponentType, bus: WorkerMessageBus,
123123
DirectiveResolver,
124124
Parser,
125125
Lexer,
126-
bind(ExceptionHandler).toFactory(() => {new ExceptionHandler(new PrintLogger())}),
126+
bind(ExceptionHandler).toFactory(() => new ExceptionHandler(new PrintLogger()), []),
127127
bind(XHR).toValue(new XHRImpl()),
128128
ComponentUrlMapper,
129129
UrlResolver,

‎modules/angular2/test/change_detection/change_detector_spec.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1043,6 +1043,10 @@ class TestDispatcher implements ChangeDispatcher {
10431043

10441044
notifyOnAllChangesDone() { this.onAllChangesDoneCalled = true; }
10451045

1046+
getDebugContext(a, b) {
1047+
return {}
1048+
}
1049+
10461050
_asString(value) { return (isBlank(value) ? 'null' : value.toString()); }
10471051
}
10481052

‎modules/angular2/test/core/compiler/integration_spec.ts

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,7 +1165,7 @@ export function main() {
11651165
});
11661166
}));
11671167

1168-
it('should provide an error context when an error happens in the DI',
1168+
it('should provide an error context when an error happens in DI',
11691169
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
11701170

11711171
tcb = tcb.overrideView(MyComp, new viewAnn.View({
@@ -1174,13 +1174,58 @@ export function main() {
11741174
}));
11751175

11761176
PromiseWrapper.catchError(tcb.createAsync(MyComp), (e) => {
1177-
expect(DOM.nodeName(e.context.element).toUpperCase())
1178-
.toEqual("DIRECTIVE-THROWING-ERROR");
1177+
var c = e.context;
1178+
expect(DOM.nodeName(c.element).toUpperCase()).toEqual("DIRECTIVE-THROWING-ERROR");
1179+
expect(DOM.nodeName(c.componentElement).toUpperCase()).toEqual("DIV");
1180+
expect(c.injector).toBeAnInstanceOf(Injector);
11791181
async.done();
11801182
return null;
11811183
});
11821184
}));
11831185

1186+
it('should provide an error context when an error happens in change detection',
1187+
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
1188+
1189+
tcb = tcb.overrideView(
1190+
MyComp, new viewAnn.View({template: `<input [value]="one.two.three" #local>`}));
1191+
1192+
tcb.createAsync(MyComp).then(rootTC => {
1193+
try {
1194+
rootTC.detectChanges();
1195+
throw "Should throw";
1196+
} catch (e) {
1197+
var c = e.context;
1198+
expect(DOM.nodeName(c.element).toUpperCase()).toEqual("INPUT");
1199+
expect(DOM.nodeName(c.componentElement).toUpperCase()).toEqual("DIV");
1200+
expect(c.injector).toBeAnInstanceOf(Injector);
1201+
expect(c.expression).toContain("one.two.three");
1202+
expect(c.context).toBe(rootTC.componentInstance);
1203+
expect(c.locals["local"]).not.toBeNull();
1204+
}
1205+
1206+
async.done();
1207+
});
1208+
}));
1209+
1210+
it('should provide an error context when an error happens in change detection (text node)',
1211+
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
1212+
1213+
tcb = tcb.overrideView(MyComp, new viewAnn.View({template: `{{one.two.three}}`}));
1214+
1215+
tcb.createAsync(MyComp).then(rootTC => {
1216+
try {
1217+
rootTC.detectChanges();
1218+
throw "Should throw";
1219+
} catch (e) {
1220+
var c = e.context;
1221+
expect(c.element).toBeNull();
1222+
expect(c.injector).toBeNull();
1223+
}
1224+
1225+
async.done();
1226+
});
1227+
}));
1228+
11841229
if (!IS_DARTIUM) {
11851230
it('should report a meaningful error when a directive is undefined',
11861231
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder,

‎modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ void initReflector() {
2424
_MyComponent_ChangeDetector0.newProtoChangeDetector;
2525
}
2626
class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector {
27-
final dynamic _dispatcher;
2827
_gen.Pipes _pipes;
2928
final _gen.List<_gen.ProtoRecord> _protos;
3029
final _gen.List<_gen.DirectiveRecord> _directiveRecords;
@@ -36,8 +35,8 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector {
3635
dynamic _interpolate1 = _gen.ChangeDetectionUtil.uninitialized();
3736

3837
_MyComponent_ChangeDetector0(
39-
this._dispatcher, this._protos, this._directiveRecords)
40-
: super("MyComponent_comp_0");
38+
dynamic dispatcher, this._protos, this._directiveRecords)
39+
: super("MyComponent_comp_0", dispatcher);
4140

4241
void detectChangesInRecords(throwOnChange) {
4342
if (!hydrated()) {
@@ -80,7 +79,7 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector {
8079
_interpolate1, interpolate1));
8180
}
8281

83-
_dispatcher.notifyOnBinding(currentProto.bindingRecord, interpolate1);
82+
dispatcher.notifyOnBinding(currentProto.bindingRecord, interpolate1);
8483

8584
_interpolate1 = interpolate1;
8685
}
@@ -95,7 +94,7 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector {
9594
}
9695

9796
void callOnAllChangesDone() {
98-
_dispatcher.notifyOnAllChangesDone();
97+
dispatcher.notifyOnAllChangesDone();
9998
}
10099

101100
void hydrate(MyComponent context, locals, directives, pipes) {

0 commit comments

Comments
 (0)