Merge pull request #131 from Danack/improve_exception_messages

Improve exception messages for invalid callables
rdlowrey · Dec 9, 2015 · d6e03a7 · d6e03a7
2 parents bfaf427 + 5ca20b2
commit d6e03a7
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 26 deletions.
diff --git a/lib/InjectionException.php b/lib/InjectionException.php
@@ -14,6 +14,47 @@ public function __construct(array $inProgressMakes, $message = "", $code = 0, \E
         parent::__construct($message, $code, $previous);
     }
 
+    /**
+     * Add a human readable version of the invalid callable to the standard 'invalid invokable' message.
+     */
+    public static function fromInvalidCallable(
+        array $inProgressMakes,
+        $callableOrMethodStr,
+        \Exception $previous = null
+    ) {
+        $callableString = null;
+
+        if (is_string($callableOrMethodStr)) {
+            $callableString .= $callableOrMethodStr;
+        } else if (is_array($callableOrMethodStr) && 
+            array_key_exists(0, $callableOrMethodStr) &&
+            array_key_exists(0, $callableOrMethodStr)) {
+            if (is_string($callableOrMethodStr[0]) && is_string($callableOrMethodStr[1])) {
+                $callableString .= $callableOrMethodStr[0].'::'.$callableOrMethodStr[1];
+            } else if (is_object($callableOrMethodStr[0]) && is_string($callableOrMethodStr[1])) {
+                $callableString .= sprintf(
+                    "[object(%s), '%s']",
+                    get_class($callableOrMethodStr[0]),
+                    $callableOrMethodStr[1]
+                );
+            }
+        }
+
+        if ($callableString) {
+            // Prevent accidental usage of long strings from filling logs. 
+            $callableString = substr($callableString, 0, 250);
+            $message = sprintf(
+                "%s. Invalid callable was '%s'",
+                Injector::M_INVOKABLE,
+                $callableString
+            );
+        } else {
+            $message = \Auryn\Injector::M_INVOKABLE;
+        }
+
+        return new self($inProgressMakes, $message, Injector::E_INVOKABLE, $previous);
+    }
+
     /**
      * Returns the hierarchy of dependencies that were being created when
      * the exception occurred.

diff --git a/lib/Injector.php b/lib/Injector.php
@@ -222,10 +222,10 @@ private function shareInstance($obj)
     public function prepare($name, $callableOrMethodStr)
     {
         if ($this->isExecutable($callableOrMethodStr) === false) {
-            throw new InjectionException(
+            throw InjectionException::fromInvalidCallable(
                 $this->inProgressMakes,
-                self::M_INVOKABLE,
-                self::E_INVOKABLE
+                self::E_INVOKABLE,
+                $callableOrMethodStr
             );
         }
 
@@ -496,10 +496,9 @@ private function buildArgFromParamDefineArr($definition)
     private function buildArgFromDelegate($paramName, $callableOrMethodStr)
     {
         if ($this->isExecutable($callableOrMethodStr) === false) {
-            throw new InjectionException(
+            throw InjectionException::fromInvalidCallable(
                 $this->inProgressMakes,
-                self::M_INVOKABLE,
-                self::E_INVOKABLE
+                $callableOrMethodStr
             );
         }
 
@@ -631,7 +630,15 @@ public function execute($callableOrMethodStr, array $args = array())
      */
     public function buildExecutable($callableOrMethodStr)
     {
-        list($reflFunc, $invocationObj) = $this->buildExecutableStruct($callableOrMethodStr);
+        try {
+            list($reflFunc, $invocationObj) = $this->buildExecutableStruct($callableOrMethodStr);
+        } catch (\ReflectionException $e) {
+            throw InjectionException::fromInvalidCallable(
+                $this->inProgressMakes,
+                $callableOrMethodStr,
+                $e
+            );
+        }
 
         return new Executable($reflFunc, $invocationObj);
     }
@@ -653,10 +660,9 @@ private function buildExecutableStruct($callableOrMethodStr)
         ) {
             $executableStruct = $this->buildExecutableStructFromArray($callableOrMethodStr);
         } else {
-            throw new InjectionException(
+            throw InjectionException::fromInvalidCallable(
                 $this->inProgressMakes,
-                self::M_INVOKABLE,
-                self::E_INVOKABLE
+                $callableOrMethodStr
             );
         }
 
@@ -676,10 +682,9 @@ private function buildExecutableStructFromString($stringExecutable)
             list($class, $method) = explode('::', $stringExecutable, 2);
             $executableStruct = $this->buildStringClassMethodCallable($class, $method);
         } else {
-            throw new InjectionException(
+            throw InjectionException::fromInvalidCallable(
                 $this->inProgressMakes,
-                self::M_INVOKABLE,
-                self::E_INVOKABLE
+                $stringExecutable
             );
         }
 
@@ -722,10 +727,9 @@ private function buildExecutableStructFromArray($arrayExecutable)
         } elseif (is_string($classOrObj)) {
             $executableStruct = $this->buildStringClassMethodCallable($classOrObj, $method);
         } else {
-            throw new InjectionException(
+            throw InjectionException::fromInvalidCallable(
                 $this->inProgressMakes,
-                self::M_INVOKABLE,
-                self::E_INVOKABLE
+                $arrayExecutable
             );
         }
 

diff --git a/test/InjectorTest.php b/test/InjectorTest.php
@@ -807,25 +807,38 @@ public function testAppropriateExceptionThrownOnNonPublicConstructorWithArgs()
         $injector->make('Auryn\Test\HasNonPublicConstructorWithArgs');
     }
 
-    /**
-     * @expectedException \Auryn\InjectionException
-     * @expectedExceptionCode \Auryn\Injector::E_INVOKABLE
-     */
     public function testMakeExecutableFailsOnNonExistentFunction()
     {
         $injector = new Injector();
+        $this->setExpectedException(
+            'Auryn\InjectionException',
+            'nonExistentFunction',
+            \Auryn\Injector::E_INVOKABLE
+        );
         $injector->buildExecutable('nonExistentFunction');
     }
 
-    /**
-     * @expectedException \Auryn\InjectionException
-     * @expectedExceptionCode \Auryn\Injector::E_INVOKABLE
-     */
-    public function testMakeExecutableFailsOnNonExistentMethod()
+    public function testMakeExecutableFailsOnNonExistentInstanceMethod()
     {
         $injector = new Injector();
         $object = new \StdClass();
-        $injector->buildExecutable(array($object, 'nonExistentFunction'));
+        $this->setExpectedException(
+            'Auryn\InjectionException',
+            "[object(stdClass), 'nonExistentMethod']",
+            \Auryn\Injector::E_INVOKABLE
+        );
+        $injector->buildExecutable(array($object, 'nonExistentMethod'));
+    }
+
+    public function testMakeExecutableFailsOnNonExistentStaticMethod()
+    {
+        $injector = new Injector();
+        $this->setExpectedException(
+            'Auryn\InjectionException',
+            "StdClass::nonExistentMethod",
+            \Auryn\Injector::E_INVOKABLE
+        );
+        $injector->buildExecutable(array('StdClass', 'nonExistentMethod'));
     }
 
     /**