From 537cc4b5c1730f4c5269311ead2a0556d13f8034 Mon Sep 17 00:00:00 2001
From: Caitlin Potter <caitpotter88@gmail.com>
Date: Mon, 22 Sep 2014 12:20:34 -0400
Subject: [PATCH] fix($injector): throw when factory $get method does not
 return a value

BREAKING CHANGE:

Previously, not returning a value would fail silently, and an application trying to inject the
value owuld inject an undefined value, quite possibly leading to a TypeError. Now, the application
will fail entirely, and a reason will be given.

Closes #4575
---
 docs/content/error/$injector/undef.ngdoc | 32 ++++++++++++++++++++++++
 src/auto/injector.js                     | 18 +++++++++++--
 test/auto/injectorSpec.js                | 22 +++++++++++-----
 3 files changed, 64 insertions(+), 8 deletions(-)
 create mode 100644 docs/content/error/$injector/undef.ngdoc

diff --git a/docs/content/error/$injector/undef.ngdoc b/docs/content/error/$injector/undef.ngdoc
new file mode 100644
index 000000000000..641676ef39c3
--- /dev/null
+++ b/docs/content/error/$injector/undef.ngdoc
@@ -0,0 +1,32 @@
+@ngdoc error
+@name $injector:undef
+@fullName Undefined Value
+@description
+
+This error results from registering a factory which does not return a value (or whose return value is undefined).
+
+The following is an example of a factory which will throw this error upon injection:
+
+```js
+angular.module("badModule", []).
+  factory("badFactory", function() {
+    doLotsOfThings();
+    butDontReturnAValue();
+  });
+```
+
+In order to prevent the error, return a value of some sort, such as an object which exposes an API for working
+with the injected object.
+
+```js
+angular.module("goodModule", []).
+  factory("goodFactory", function() {
+    doLotsOfThings();
+    butDontReturnAValue();
+
+    return {
+        doTheThing: function methodThatDoesAThing() {
+        }
+    };
+  });
+```
\ No newline at end of file
diff --git a/src/auto/injector.js b/src/auto/injector.js
index 453991167c18..62b860ece618 100644
--- a/src/auto/injector.js
+++ b/src/auto/injector.js
@@ -663,7 +663,21 @@ function createInjector(modulesToLoad, strictDi) {
     return providerCache[name + providerSuffix] = provider_;
   }
 
-  function factory(name, factoryFn) { return provider(name, { $get: factoryFn }); }
+  function enforceReturnValue(name, factory) {
+    return function enforcedReturnValue() {
+      var result = instanceInjector.invoke(factory);
+      if (isUndefined(result)) {
+        throw $injectorMinErr('undef', "Provider '{0}' must return a value from $get factory method.", name);
+      }
+      return result;
+    };
+  }
+
+  function factory(name, factoryFn, enforce) {
+    return provider(name, {
+      $get: enforce !== false ? enforceReturnValue(name, factoryFn) : factoryFn
+    });
+  }
 
   function service(name, constructor) {
     return factory(name, ['$injector', function($injector) {
@@ -671,7 +685,7 @@ function createInjector(modulesToLoad, strictDi) {
     }]);
   }
 
-  function value(name, val) { return factory(name, valueFn(val)); }
+  function value(name, val) { return factory(name, valueFn(val), false); }
 
   function constant(name, value) {
     assertNotHasOwnProperty(name, 'constant');
diff --git a/test/auto/injectorSpec.js b/test/auto/injectorSpec.js
index 0aff27ac79f0..d5aeb7028121 100644
--- a/test/auto/injectorSpec.js
+++ b/test/auto/injectorSpec.js
@@ -45,12 +45,12 @@ describe('injector', function() {
 //   s6
 
 
-    providers('s1', function() { log.push('s1'); }, {$inject: ['s2', 's5', 's6']});
-    providers('s2', function() { log.push('s2'); }, {$inject: ['s3', 's4', 's5']});
-    providers('s3', function() { log.push('s3'); }, {$inject: ['s6']});
-    providers('s4', function() { log.push('s4'); }, {$inject: ['s3', 's5']});
-    providers('s5', function() { log.push('s5'); });
-    providers('s6', function() { log.push('s6'); });
+    providers('s1', function() { log.push('s1'); return {}; }, {$inject: ['s2', 's5', 's6']});
+    providers('s2', function() { log.push('s2'); return {}; }, {$inject: ['s3', 's4', 's5']});
+    providers('s3', function() { log.push('s3'); return {}; }, {$inject: ['s6']});
+    providers('s4', function() { log.push('s4'); return {}; }, {$inject: ['s3', 's5']});
+    providers('s5', function() { log.push('s5'); return {}; });
+    providers('s6', function() { log.push('s6'); return {}; });
 
     injector.get('s1');
 
@@ -983,4 +983,14 @@ describe('strict-di injector', function() {
       }).toThrowMinErr('$injector', 'strictdi');
     });
   });
+
+
+  it('should throw if factory does not return a value', function() {
+    module(function($provide) {
+      $provide.factory('$test', function() {});
+    });
+    expect(function() {
+      inject(function($test) {});
+    }).toThrowMinErr('$injector', 'undef');
+  });
 });