X Tutup
Skip to content

Commit 8543c34

Browse files
vsavkinvsavkin
committed
feat(core): provide an error context when an exception happens in an error handler
1 parent 1d45029 commit 8543c34

File tree

13 files changed

+175
-67
lines changed

13 files changed

+175
-67
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,9 @@ export class AbstractChangeDetector implements ChangeDetector {
9090

9191
throwError(proto: ProtoRecord, exception: any, stack: any): void {
9292
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);
93+
var context = isPresent(c) ? new _Context(c.element, c.componentElement, c.directive, c.context,
94+
c.locals, c.injector, proto.expressionAsString) :
95+
null;
9596
throw new ChangeDetectionError(proto, exception, stack, context);
9697
}
9798
}

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

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ import {
77
BaseException,
88
assertionsEnabled,
99
print,
10-
stringify
10+
stringify,
11+
isDart
1112
} from 'angular2/src/facade/lang';
1213
import {BrowserDomAdapter} from 'angular2/src/dom/browser_adapter';
1314
import {DOM} from 'angular2/src/dom/dom_adapter';
@@ -128,7 +129,7 @@ function _injectorBindings(appComponentType): List<Type | Binding | List<any>> {
128129
DirectiveResolver,
129130
Parser,
130131
Lexer,
131-
bind(ExceptionHandler).toFactory(() => new ExceptionHandler(DOM), []),
132+
bind(ExceptionHandler).toFactory(() => new ExceptionHandler(DOM, isDart ? false : true), []),
132133
bind(XHR).toValue(new XHRImpl()),
133134
ComponentUrlMapper,
134135
UrlResolver,
@@ -280,8 +281,8 @@ export function commonBootstrap(
280281
appComponentType: /*Type*/ any,
281282
componentInjectableBindings: List<Type | Binding | List<any>> = null): Promise<ApplicationRef> {
282283
BrowserDomAdapter.makeCurrent();
283-
var bootstrapProcess: PromiseCompleter<any> = PromiseWrapper.completer();
284-
var zone = createNgZone(new ExceptionHandler(DOM));
284+
var bootstrapProcess = PromiseWrapper.completer();
285+
var zone = createNgZone(new ExceptionHandler(DOM, isDart ? false : true));
285286
zone.run(() => {
286287
// TODO(rado): prepopulate template cache, so applications with only
287288
// index.html and main.js are possible.
@@ -290,19 +291,27 @@ export function commonBootstrap(
290291
var exceptionHandler = appInjector.get(ExceptionHandler);
291292
zone.overrideOnErrorHandler((e, s) => exceptionHandler.call(e, s));
292293

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

302-
bootstrapProcess.resolve(new ApplicationRef(componentRef, appComponentType, appInjector));
303-
};
304-
PromiseWrapper.then(compRefToken, tick,
305-
(err, stackTrace) => bootstrapProcess.reject(err, stackTrace));
303+
bootstrapProcess.resolve(new ApplicationRef(componentRef, appComponentType, appInjector));
304+
};
305+
306+
var tickResult = PromiseWrapper.then(compRefToken, tick);
307+
308+
PromiseWrapper.then(tickResult,
309+
(_) => {}); // required for Dart to trigger the default error handler
310+
PromiseWrapper.then(tickResult, null,
311+
(err, stackTrace) => { bootstrapProcess.reject(err, stackTrace); });
312+
} catch (e) {
313+
bootstrapProcess.reject(e, e.stack);
314+
}
306315
});
307316

308317
return bootstrapProcess.promise;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ export class ElementInjector extends TreeNode<ElementInjector> implements Depend
502502
var p = this._preBuiltObjects;
503503
var index = p.elementRef.boundElementIndex - p.view.elementOffset;
504504
var c = this._preBuiltObjects.view.getDebugContext(index, null);
505-
return new _Context(c["element"], c["componentElement"], c["injector"]);
505+
return isPresent(c) ? new _Context(c.element, c.componentElement, c.injector) : null;
506506
}
507507

508508
private _reattachInjectors(imperativelyCreatedInjector: Injector): void {

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

Lines changed: 58 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ export class AppView implements ChangeDispatcher, RenderEventDispatcher {
212212
return isPresent(boundElementIndex) ? this.elementRefs[boundElementIndex] : null;
213213
}
214214

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

229-
return {
230-
element: element,
231-
componentElement: componentElement,
232-
directive: directive,
233-
context: this.context,
234-
locals: _localsToStringMap(this.locals),
235-
injector: injector
236-
};
229+
return new DebugContext(element, componentElement, directive, this.context,
230+
_localsToStringMap(this.locals), injector);
231+
237232
} catch (e) {
238233
// TODO: vsavkin log the exception once we have a good way to log errors and warnings
239234
// if an error happens during getting the debug context, we return an empty map.
240-
return {};
235+
return null;
241236
}
242237
}
243238

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

263258
// returns false if preventDefault must be applied to the DOM event
264259
dispatchEvent(boundElementIndex: number, eventName: string, locals: Map<string, any>): boolean {
265-
// Most of the time the event will be fired only when the view is in the live document.
266-
// However, in a rare circumstance the view might get dehydrated, in between the event
267-
// queuing up and firing.
268-
var allowDefaultBehavior = true;
269-
if (this.hydrated()) {
270-
var elBinder = this.proto.elementBinders[boundElementIndex - this.elementOffset];
271-
if (isBlank(elBinder.hostListeners)) return allowDefaultBehavior;
272-
var eventMap = elBinder.hostListeners[eventName];
273-
if (isBlank(eventMap)) return allowDefaultBehavior;
274-
MapWrapper.forEach(eventMap, (expr, directiveIndex) => {
275-
var context;
276-
if (directiveIndex === -1) {
277-
context = this.context;
278-
} else {
279-
context = this.elementInjectors[boundElementIndex].getDirectiveAtIndex(directiveIndex);
280-
}
281-
var result = expr.eval(context, new Locals(this.locals, locals));
282-
if (isPresent(result)) {
283-
allowDefaultBehavior = allowDefaultBehavior && result == true;
284-
}
285-
});
260+
try {
261+
// Most of the time the event will be fired only when the view is in the live document.
262+
// However, in a rare circumstance the view might get dehydrated, in between the event
263+
// queuing up and firing.
264+
var allowDefaultBehavior = true;
265+
if (this.hydrated()) {
266+
var elBinder = this.proto.elementBinders[boundElementIndex - this.elementOffset];
267+
if (isBlank(elBinder.hostListeners)) return allowDefaultBehavior;
268+
var eventMap = elBinder.hostListeners[eventName];
269+
if (isBlank(eventMap)) return allowDefaultBehavior;
270+
MapWrapper.forEach(eventMap, (expr, directiveIndex) => {
271+
var context;
272+
if (directiveIndex === -1) {
273+
context = this.context;
274+
} else {
275+
context = this.elementInjectors[boundElementIndex].getDirectiveAtIndex(directiveIndex);
276+
}
277+
var result = expr.eval(context, new Locals(this.locals, locals));
278+
if (isPresent(result)) {
279+
allowDefaultBehavior = allowDefaultBehavior && result == true;
280+
}
281+
});
282+
}
283+
return allowDefaultBehavior;
284+
} catch (e) {
285+
var c = this.getDebugContext(boundElementIndex - this.elementOffset, null);
286+
var context = isPresent(c) ? new _Context(c.element, c.componentElement, c.context, c.locals,
287+
c.injector) :
288+
null;
289+
throw new EventEvaluationError(eventName, e, e.stack, context);
286290
}
287-
return allowDefaultBehavior;
288291
}
289292
}
290293

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

304+
export class DebugContext {
305+
constructor(public element: any, public componentElement: any, public directive: any,
306+
public context: any, public locals: any, public injector: any) {}
307+
}
308+
309+
/**
310+
* Error context included when an event handler throws an exception.
311+
*/
312+
class _Context {
313+
constructor(public element: any, public componentElement: any, public context: any,
314+
public locals: any, public injector: any) {}
315+
}
316+
317+
/**
318+
* Wraps an exception thrown by an event handler.
319+
*/
320+
class EventEvaluationError extends BaseException {
321+
constructor(eventName: string, originalException: any, originalStack: any, context: any) {
322+
super(`Error during evaluation of "${eventName}"`, originalException, originalStack, context);
323+
}
324+
}
325+
326+
301327
/**
302328
*
303329
*/

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

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

8181
_longStackTrace(stackTrace: any): any {
8282
return isListLikeIterable(stackTrace) ? (<any>stackTrace).join("\n\n-----async gap-----\n") :
83-
stackTrace;
83+
stackTrace.toString();
8484
}
8585

8686
_findContext(exception: any): any {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import 'dart:math' as math;
55
import 'dart:convert' as convert;
66
import 'dart:async' show Future;
77

8+
bool isDart = true;
9+
810
String getTypeNameForDebugging(Type type) => type.toString();
911

1012
class Math {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ export function getTypeNameForDebugging(type: Type): string {
99
return type['name'];
1010
}
1111

12+
export var isDart = false;
13+
1214
export class BaseException extends Error {
1315
stack;
1416
constructor(public message?: string, private _originalException?, private _originalStack?,

‎modules/angular2/src/test_lib/fake_async.dart‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,13 @@ void tick([int millis = 0]) {
8080
_fakeAsync.elapse(duration);
8181
}
8282

83+
/**
84+
* This is not needed in Dart. Because quiver correctly removes a timer when
85+
* it throws an exception.
86+
*/
87+
void clearPendingTimers() {
88+
}
89+
8390
/**
8491
* Flush any pending microtasks.
8592
*/

‎modules/angular2/src/test_lib/fake_async.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,7 @@ export function fakeAsync(fn: Function): Function {
4141
return function(...args) {
4242
// TODO(tbosch): This class should already be part of the jasmine typings but it is not...
4343
_scheduler = new (<any>jasmine).DelayedFunctionScheduler();
44-
ListWrapper.clear(_microtasks);
45-
ListWrapper.clear(_pendingPeriodicTimers);
46-
ListWrapper.clear(_pendingTimers);
44+
clearPendingTimers();
4745

4846
let res = fakeAsyncZone.run(() => {
4947
let res = fn(...args);
@@ -67,6 +65,14 @@ export function fakeAsync(fn: Function): Function {
6765
}
6866
}
6967

68+
// TODO we should fix tick to dequeue the failed timer instead of relying on clearPendingTimers
69+
export function clearPendingTimers() {
70+
ListWrapper.clear(_microtasks);
71+
ListWrapper.clear(_pendingPeriodicTimers);
72+
ListWrapper.clear(_pendingTimers);
73+
}
74+
75+
7076
/**
7177
* Simulates the asynchronous passage of time for the timers in the fakeAsync zone.
7278
*

‎modules/angular2/src/test_lib/test_lib.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export interface NgMatchers extends jasmine.Matchers {
3232

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

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

3738
export class AsyncTestCompleter {

0 commit comments

Comments
 (0)
X Tutup