Issue 549 (#666)

* Fixed #549

* Removed latest version of nodejs in AppVeyor because there is issues in windows sometimes

* Update sinon to the latest version

* Small improvements

appveyor.yml

@@ -9,7 +9,6 @@ environment:
   NODE_ENV: test
 
   matrix:
-    - nodejs_version: "stable"
     - nodejs_version: "7.10.1"
 
 build: off

package.json

@@ -71,7 +71,7 @@
     "publish-please": "2.3.1",
     "reflect-metadata": "0.1.10",
     "run-sequence": "2.2.0",
-    "sinon": "4.1.0",
+    "sinon": "4.1.1",
     "tslint": "5.8.0",
     "typescript": "2.6.1",
     "vinyl-buffer": "1.0.0",

src/constants/error_msgs.ts

@@ -34,3 +34,9 @@ export const CONTAINER_OPTIONS_INVALID_AUTO_BIND_INJECTABLE = "Invalid Container
 
 export const MULTIPLE_POST_CONSTRUCT_METHODS = "Cannot apply @postConstruct decorator multiple times in the same class";
 export const POST_CONSTRUCT_ERROR = (...values: any[]) => `@postConstruct error in class ${values[0]}: ${values[1]}`;
+
+export const CIRCULAR_DEPENDENCY_IN_FACTORY = (...values: any[]) =>  `It looks like there is a circular dependency ` +
+    `in one of the '${values[0]}' bindings. Please investigate bindings with` +
+    `service identifier '${values[1]}'.`;
+
+export const STACK_OVERFLOW = "Maximum call stack size exceeded";

src/planning/planner.ts

@@ -8,6 +8,7 @@ import { getDependencies } from "./reflection_utils";
 import { Metadata } from "./metadata";
 import * as ERROR_MSGS from "../constants/error_msgs";
 import * as METADATA_KEY from "../constants/metadata_keys";
+import { isStackOverflowExeption } from "../utils/exceptions";
 import { BindingTypeEnum, TargetTypeEnum } from "../constants/literal_types";
 import {
     circularDependencyToException,
@@ -191,7 +192,10 @@ function _createSubRequests(
         });
 
     } catch (error) {
-        if (error instanceof RangeError && parentRequest !== null) {
+        if (
+            isStackOverflowExeption(error) &&
+            parentRequest !== null
+        ) {
             circularDependencyToException(parentRequest.parentContext.plan.rootRequest);
         } else {
             throw new Error(error.message);

src/resolution/instantiation.ts

@@ -11,7 +11,10 @@ function _injectProperties(
 ): any {
 
     let propertyInjectionsRequests = childRequests.filter((childRequest: interfaces.Request) => {
-        return (childRequest.target !== null && childRequest.target.type === TargetTypeEnum.ClassProperty);
+        return (
+            childRequest.target !== null &&
+            childRequest.target.type === TargetTypeEnum.ClassProperty
+        );
     });
 
     let propertyInjections = propertyInjectionsRequests.map((childRequest: interfaces.Request) => {

src/syntax/binding_to_syntax.ts

@@ -2,8 +2,31 @@ import { interfaces } from "../interfaces/interfaces";
 import { BindingInWhenOnSyntax } from "./binding_in_when_on_syntax";
 import { BindingWhenOnSyntax } from "./binding_when_on_syntax";
 import { BindingTypeEnum } from "../constants/literal_types";
+import { isStackOverflowExeption } from "../utils/exceptions";
 import * as ERROR_MSGS from "../constants/error_msgs";
 
+type FactoryType = "toDynamicValue" | "toFactory" | "toAutoFactory" | "toProvider";
+
+function factoryWrapper<T extends Function>(
+    factoryType: FactoryType,
+    serviceIdentifier: interfaces.ServiceIdentifier<any>,
+    factory: T
+) {
+    return (...args: any[]) => {
+        try {
+            return factory(...args);
+        } catch (error) {
+            if (isStackOverflowExeption(error)) {
+                throw new Error(
+                    ERROR_MSGS.CIRCULAR_DEPENDENCY_IN_FACTORY(factoryType, serviceIdentifier)
+                );
+            } else {
+                throw new Error(error.message);
+            }
+        }
+    };
+}
+
 class BindingToSyntax<T> implements interfaces.BindingToSyntax<T> {
 
     private _binding: interfaces.Binding<T>;
@@ -37,7 +60,11 @@ class BindingToSyntax<T> implements interfaces.BindingToSyntax<T> {
     public toDynamicValue(func: (context: interfaces.Context) => T): interfaces.BindingInWhenOnSyntax<T> {
         this._binding.type = BindingTypeEnum.DynamicValue;
         this._binding.cache = null;
-        this._binding.dynamicValue = func;
+        this._binding.dynamicValue = factoryWrapper(
+            "toDynamicValue",
+            this._binding.serviceIdentifier.toString(),
+            func
+        );
         this._binding.implementationType = null;
         return new BindingInWhenOnSyntax<T>(this._binding);
     }
@@ -50,7 +77,11 @@ class BindingToSyntax<T> implements interfaces.BindingToSyntax<T> {
 
     public toFactory<T2>(factory: interfaces.FactoryCreator<T2>): interfaces.BindingWhenOnSyntax<T> {
         this._binding.type = BindingTypeEnum.Factory;
-        this._binding.factory = factory;
+        this._binding.factory = factoryWrapper(
+            "toFactory",
+            this._binding.serviceIdentifier.toString(),
+            factory
+        );
         return new BindingWhenOnSyntax<T>(this._binding);
     }
 
@@ -65,16 +96,23 @@ class BindingToSyntax<T> implements interfaces.BindingToSyntax<T> {
     public toAutoFactory<T2>(serviceIdentifier: interfaces.ServiceIdentifier<T2>): interfaces.BindingWhenOnSyntax<T> {
         this._binding.type = BindingTypeEnum.Factory;
         this._binding.factory = (context) => {
-            return () => {
-                return context.container.get<T2>(serviceIdentifier);
-            };
+            const autofactory = () => context.container.get<T2>(serviceIdentifier);
+            return factoryWrapper(
+                "toAutoFactory",
+                this._binding.serviceIdentifier.toString(),
+                autofactory
+            );
         };
         return new BindingWhenOnSyntax<T>(this._binding);
     }
 
     public toProvider<T2>(provider: interfaces.ProviderCreator<T2>): interfaces.BindingWhenOnSyntax<T> {
         this._binding.type = BindingTypeEnum.Provider;
-        this._binding.provider = provider;
+        this._binding.provider = factoryWrapper(
+            "toProvider",
+            this._binding.serviceIdentifier.toString(),
+            provider
+        );
         return new BindingWhenOnSyntax<T>(this._binding);
     }
 

src/utils/exceptions.ts

@@ -0,0 +1,8 @@
+import * as ERROR_MSGS from "../constants/error_msgs";
+
+export function isStackOverflowExeption(error: Error) {
+    return (
+        error instanceof RangeError ||
+        error.message === ERROR_MSGS.STACK_OVERFLOW
+    );
+}

test/bugs/issue_549.test.ts

@@ -0,0 +1,49 @@
+import { expect } from "chai";
+import { Container, injectable, inject } from "../../src/inversify";
+import * as ERROR_MSGS from "../../src/constants/error_msgs";
+
+describe("issue 549", () => {
+
+    it("Should throw if circular dependencies found with dynamics", () => {
+
+        @injectable()
+        class A {
+            public b: B;
+            public constructor(
+                @inject("B")  b: B
+            ) {
+                this.b = b;
+            }
+        }
+
+        @injectable()
+        class B {
+            public a: A;
+            public constructor(
+                @inject("A") a: A
+            ) {
+                this.a = a;
+            }
+        }
+
+        let container = new Container({defaultScope: "Singleton"});
+        container.bind(A).toSelf();
+        container.bind(B).toSelf();
+        container.bind("A").toDynamicValue(ctx =>
+            ctx.container.get(A)
+        );
+
+        container.bind("B").toDynamicValue(ctx =>
+            ctx.container.get(B)
+        );
+
+        function willThrow() {
+            return container.get<A>("A");
+        }
+
+        const expectedError = ERROR_MSGS.CIRCULAR_DEPENDENCY_IN_FACTORY("toDynamicValue", "A");
+        expect(willThrow).to.throw(expectedError);
+
+    });
+
+});