From 5a86f85936ceaf52d7ab011ab81fb74c61149091 Mon Sep 17 00:00:00 2001 From: vsavkin Date: Wed, 22 Jul 2015 12:00:35 -0700 Subject: [PATCH] feat(di): added context to runtime DI errors --- .../src/core/compiler/element_injector.ts | 15 +++- modules/angular2/src/core/compiler/view.ts | 5 ++ modules/angular2/src/di/exceptions.ts | 72 ++++++------------- modules/angular2/src/di/injector.ts | 37 ++++++++-- .../test/core/compiler/integration_spec.ts | 24 +++++++ modules/angular2/test/di/injector_spec.ts | 32 ++++++++- 6 files changed, 126 insertions(+), 59 deletions(-) diff --git a/modules/angular2/src/core/compiler/element_injector.ts b/modules/angular2/src/core/compiler/element_injector.ts index 4099a2fb18..881a6e4343 100644 --- a/modules/angular2/src/core/compiler/element_injector.ts +++ b/modules/angular2/src/core/compiler/element_injector.ts @@ -419,6 +419,9 @@ export class ProtoElementInjector { getBindingAtIndex(index: number): any { return this.protoInjector.getBindingAtIndex(index); } } +class _Context { + constructor(public element: any, public componentElement: any, public injector: any) {} +} export class ElementInjector extends TreeNode implements DependencyProvider { private _host: ElementInjector; @@ -438,7 +441,9 @@ export class ElementInjector extends TreeNode implements Depend constructor(public _proto: ProtoElementInjector, parent: ElementInjector) { super(parent); - this._injector = new Injector(this._proto.protoInjector, null, this); + this._injector = + new Injector(this._proto.protoInjector, null, this, () => this._debugContext()); + // we couple ourselves to the injector strategy to avoid polymoprhic calls var injectorStrategy = this._injector.internalStrategy; this._strategy = injectorStrategy instanceof InjectorInlineStrategy ? @@ -489,6 +494,12 @@ export class ElementInjector extends TreeNode implements Depend this.hydrated = true; } + private _debugContext(): any { + var p = this._preBuiltObjects; + return new _Context(p.elementRef.nativeElement, p.view.getHostElement().nativeElement, + this._injector); + } + private _reattachInjectors(imperativelyCreatedInjector: Injector): void { // Dynamically-loaded component in the template. Not a root ElementInjector. if (isPresent(this._parent)) { @@ -613,7 +624,7 @@ export class ElementInjector extends TreeNode implements Depend return null; } - throw new NoBindingError(dirDep.key); + throw new NoBindingError(null, dirDep.key); } return this._preBuiltObjects.templateRef; } diff --git a/modules/angular2/src/core/compiler/view.ts b/modules/angular2/src/core/compiler/view.ts index 6985cef2fe..e571551637 100644 --- a/modules/angular2/src/core/compiler/view.ts +++ b/modules/angular2/src/core/compiler/view.ts @@ -200,6 +200,11 @@ export class AppView implements ChangeDispatcher, RenderEventDispatcher { return isPresent(viewIndex) ? this.views[viewIndex] : null; } + getHostElement(): ElementRef { + var boundElementIndex = this.mainMergeMapping.hostElementIndicesByViewIndex[this.viewOffset]; + return this.elementRefs[boundElementIndex]; + } + getDetectorFor(directive: DirectiveIndex): any { var childView = this.getNestedView(this.elementOffset + directive.elementIndex); return isPresent(childView) ? childView.changeDetector : null; diff --git a/modules/angular2/src/di/exceptions.ts b/modules/angular2/src/di/exceptions.ts index 8dab68887e..933053bdce 100644 --- a/modules/angular2/src/di/exceptions.ts +++ b/modules/angular2/src/di/exceptions.ts @@ -1,5 +1,7 @@ import {ListWrapper, List} from 'angular2/src/facade/collection'; import {stringify, BaseException, isBlank} from 'angular2/src/facade/lang'; +import {Key} from './key'; +import {Injector} from './injector'; function findFirstClosedCycle(keys: List): List { var res = []; @@ -31,22 +33,27 @@ function constructResolvingPath(keys: List): string { export class AbstractBindingError extends BaseException { name: string; message: string; - keys: List; + keys: List; + injectors: List; constructResolvingMessage: Function; - // TODO(tbosch): Can't do key:Key as this results in a circular dependency! - constructor(key, constructResolvingMessage: Function, originalException?, originalStack?) { - super(null, originalException, originalStack); + + constructor(injector: Injector, key: Key, constructResolvingMessage: Function, originalException?, + originalStack?) { + super("DI Exception", originalException, originalStack, null); this.keys = [key]; + this.injectors = [injector]; this.constructResolvingMessage = constructResolvingMessage; this.message = this.constructResolvingMessage(this.keys); } - // TODO(tbosch): Can't do key:Key as this results in a circular dependency! - addKey(key: any): void { + addKey(injector: Injector, key: Key): void { + this.injectors.push(injector); this.keys.push(key); this.message = this.constructResolvingMessage(this.keys); } + get context() { return this.injectors[this.injectors.length - 1].debugContext(); } + toString(): string { return this.message; } } @@ -55,47 +62,14 @@ export class AbstractBindingError extends BaseException { * {@link Injector} does not have a {@link Binding} for {@link Key}. */ export class NoBindingError extends AbstractBindingError { - // TODO(tbosch): Can't do key:Key as this results in a circular dependency! - constructor(key) { - super(key, function(keys: List) { + constructor(injector: Injector, key: Key) { + super(injector, key, function(keys: List) { var first = stringify(ListWrapper.first(keys).token); return `No provider for ${first}!${constructResolvingPath(keys)}`; }); } } -/** - * Thrown when trying to retrieve an async {@link Binding} using the sync API. - * - * ## Example - * - * ```javascript - * var injector = Injector.resolveAndCreate([ - * bind(Number).toAsyncFactory(() => { - * return new Promise((resolve) => resolve(1 + 2)); - * }), - * bind(String).toFactory((v) => { return "Value: " + v; }, [String]) - * ]); - * - * injector.asyncGet(String).then((v) => expect(v).toBe('Value: 3')); - * expect(() => { - * injector.get(String); - * }).toThrowError(AsycBindingError); - * ``` - * - * The above example throws because `String` depends on `Number` which is async. If any binding in - * the dependency graph is async then the graph can only be retrieved using the `asyncGet` API. - */ -export class AsyncBindingError extends AbstractBindingError { - // TODO(tbosch): Can't do key:Key as this results in a circular dependency! - constructor(key) { - super(key, function(keys: List) { - var first = stringify(ListWrapper.first(keys).token); - return `Cannot instantiate ${first} synchronously. It is provided as a promise!${constructResolvingPath(keys)}`; - }); - } -} - /** * Thrown when dependencies form a cycle. * @@ -113,9 +87,8 @@ export class AsyncBindingError extends AbstractBindingError { * Retrieving `A` or `B` throws a `CyclicDependencyError` as the graph above cannot be constructed. */ export class CyclicDependencyError extends AbstractBindingError { - // TODO(tbosch): Can't do key:Key as this results in a circular dependency! - constructor(key) { - super(key, function(keys: List) { + constructor(injector: Injector, key: Key) { + super(injector, key, function(keys: List) { return `Cannot instantiate cyclic dependency!${constructResolvingPath(keys)}`; }); } @@ -128,14 +101,13 @@ export class CyclicDependencyError extends AbstractBindingError { * this object to be instantiated. */ export class InstantiationError extends AbstractBindingError { - causeKey; - - // TODO(tbosch): Can't do key:Key as this results in a circular dependency! - constructor(originalException, originalStack, key) { - super(key, function(keys: List) { + causeKey: Key; + constructor(injector: Injector, originalException, originalStack, key: Key) { + super(injector, key, function(keys: List) { var first = stringify(ListWrapper.first(keys).token); return `Error during instantiation of ${first}!${constructResolvingPath(keys)}.` + - ` ORIGINAL ERROR: ${originalException}` + `\n\n ORIGINAL STACK: ${originalStack}`; + `\n\n ORIGINAL ERROR: ${originalException}` + + `\n\n ORIGINAL STACK: ${originalStack} \n`; }, originalException, originalStack); this.causeKey = key; diff --git a/modules/angular2/src/di/injector.ts b/modules/angular2/src/di/injector.ts index cd185c78e7..8fb0118f44 100644 --- a/modules/angular2/src/di/injector.ts +++ b/modules/angular2/src/di/injector.ts @@ -5,7 +5,6 @@ import {ResolvedBinding, Binding, Dependency, BindingBuilder, bind} from './bind import { AbstractBindingError, NoBindingError, - AsyncBindingError, CyclicDependencyError, InstantiationError, InvalidBindingError, @@ -174,8 +173,10 @@ export class ProtoInjectorDynamicStrategy implements ProtoInjectorStrategy { export class ProtoInjector { _strategy: ProtoInjectorStrategy; + numberOfBindings: number; constructor(bwv: BindingWithVisibility[]) { + this.numberOfBindings = bwv.length; this._strategy = bwv.length > _MAX_CONSTRUCTION_COUNTER ? new ProtoInjectorDynamicStrategy(this, bwv) : new ProtoInjectorInlineStrategy(this, bwv); @@ -469,10 +470,18 @@ export class Injector { _constructionCounter: number = 0; constructor(public _proto: ProtoInjector, public _parent: Injector = null, - private _depProvider: DependencyProvider = null) { + private _depProvider: DependencyProvider = null, + private _debugContext: Function = null) { this._strategy = _proto._strategy.createInjectorStrategy(this); } + /** + * Returns debug information about the injector. + * + * This information is included into exceptions thrown by the injector. + */ + debugContext(): any { return this._debugContext(); } + /** * Retrieves an instance from the injector. * @@ -550,7 +559,7 @@ export class Injector { _new(binding: ResolvedBinding, visibility: number): any { if (this._constructionCounter++ > this._strategy.getMaxNumberOfObjects()) { - throw new CyclicDependencyError(binding.key); + throw new CyclicDependencyError(this, binding.key); } var factory = binding.factory; @@ -580,7 +589,9 @@ export class Injector { d18 = length > 18 ? this._getByDependency(binding, deps[18], visibility) : null; d19 = length > 19 ? this._getByDependency(binding, deps[19], visibility) : null; } catch (e) { - if (e instanceof AbstractBindingError) e.addKey(binding.key); + if (e instanceof AbstractBindingError) { + e.addKey(this, binding.key); + } throw e; } @@ -655,7 +666,7 @@ export class Injector { break; } } catch (e) { - throw new InstantiationError(e, e.stack, binding.key); + throw new InstantiationError(this, e, e.stack, binding.key); } return obj; } @@ -693,7 +704,7 @@ export class Injector { if (optional) { return null; } else { - throw new NoBindingError(key); + throw new NoBindingError(this, key); } } @@ -751,6 +762,12 @@ export class Injector { return this._throwOrNull(key, optional); } + + get displayName(): string { + return `Injector(bindings: [${_mapBindings(this, b => ` "${b.key.displayName}" `).join(", ")}])`; + } + + toString(): string { return this.displayName; } } var INJECTOR_KEY = Key.get(Injector); @@ -795,3 +812,11 @@ function _flattenBindings(bindings: List>, }); return res; } + +function _mapBindings(injector: Injector, fn: Function): any[] { + var res = []; + for (var i = 0; i < injector._proto.numberOfBindings; ++i) { + res.push(fn(injector._proto.getBindingAtIndex(i))); + } + return res; +} diff --git a/modules/angular2/test/core/compiler/integration_spec.ts b/modules/angular2/test/core/compiler/integration_spec.ts index 4defa06921..56abb5ecaf 100644 --- a/modules/angular2/test/core/compiler/integration_spec.ts +++ b/modules/angular2/test/core/compiler/integration_spec.ts @@ -1154,6 +1154,22 @@ export function main() { }); })); + it('should provide an error context when an error happens in the DI', + inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { + + tcb = tcb.overrideView(MyComp, new viewAnn.View({ + directives: [DirectiveThrowingAnError], + template: `` + })); + + PromiseWrapper.catchError(tcb.createAsync(MyComp), (e) => { + expect(DOM.nodeName(e.context.element).toUpperCase()) + .toEqual("DIRECTIVE-THROWING-ERROR"); + async.done(); + return null; + }); + })); + if (!IS_DARTIUM) { it('should report a meaningful error when a directive is undefined', inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, @@ -1870,3 +1886,11 @@ class OtherDuplicateDir { DOM.setText(elRef.nativeElement, DOM.getText(elRef.nativeElement) + 'othernoduplicate'); } } + +@Directive({selector: 'directive-throwing-error'}) +class DirectiveThrowingAnError { + constructor() { + throw new BaseException("BOOM"); + ; + } +} diff --git a/modules/angular2/test/di/injector_spec.ts b/modules/angular2/test/di/injector_spec.ts index 491a3d671e..16f062484b 100644 --- a/modules/angular2/test/di/injector_spec.ts +++ b/modules/angular2/test/di/injector_spec.ts @@ -292,7 +292,12 @@ export function main() { }); it('should show the full path when error happens in a constructor', () => { - var injector = createInjector([Car, bind(Engine).toClass(BrokenEngine)]); + var bindings = Injector.resolve([Car, bind(Engine).toClass(BrokenEngine)]); + var proto = new ProtoInjector([ + new BindingWithVisibility(bindings[0], PUBLIC), + new BindingWithVisibility(bindings[1], PUBLIC) + ]); + var injector = new Injector(proto, null, null); try { injector.get(Car); @@ -305,6 +310,24 @@ export function main() { } }); + it('should provide context when throwing an exception ', () => { + var engineBinding = Injector.resolve([bind(Engine).toClass(BrokenEngine)])[0]; + var protoParent = new ProtoInjector([new BindingWithVisibility(engineBinding, PUBLIC)]); + + var carBinding = Injector.resolve([Car])[0]; + var protoChild = new ProtoInjector([new BindingWithVisibility(carBinding, PUBLIC)]); + + var parent = new Injector(protoParent, null, null, () => "parentContext"); + var child = new Injector(protoChild, parent, null, () => "childContext"); + + try { + child.get(Car); + throw "Must throw"; + } catch (e) { + expect(e.context).toEqual("childContext"); + } + }); + it('should instantiate an object after a failed attempt', () => { var isBroken = true; @@ -545,5 +568,12 @@ export function main() { expect(binding.dependencies[0].properties).toEqual([new CustomDependencyMetadata()]); }); }); + + describe("displayName", () => { + it("should work", () => { + expect(Injector.resolveAndCreate([Engine, BrokenEngine]).displayName) + .toEqual('Injector(bindings: [ "Engine" , "BrokenEngine" ])'); + }); + }); }); }