fix(returns): should return rejected error when non Promise value

Should promisified function return rejected promise when `done` is not called, but returns a non Promise value. It can only return undefined or Promise, or call the callback fixes #20
hybridables · Apr 4, 2017 · 67ae2b6 · 67ae2b6
1 parent 05d8745
commit 67ae2b6
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 11 deletions.
diff --git a/dist/redolent-testing.js b/dist/redolent-testing.js
@@ -118,10 +118,17 @@ function redolent (fn, opts) {
 
       opts.args = isAsyncFn ? opts.args.concat(done) : opts.args;
       var syncResult = fn.apply(opts.context, opts.args);
-      var xPromise = isAsyncFn && !called && isPromise(syncResult, opts.Promise);
 
-      if ((!isAsyncFn && !called) || xPromise) {
+      var xPromise = isPromise(syncResult, opts.Promise);
+      var hasPromiseReturn = isAsyncFn && !called && xPromise;
+
+      if ((!isAsyncFn && !called) || hasPromiseReturn) {
         resolve(syncResult);
+        return
+      }
+      if (isAsyncFn && !xPromise && syncResult !== undefined) {
+        var msg = 'Asynchronous functions can only return a Promise or invoke a callback';
+        reject(new Error('redolent: ' + msg));
       }
     });
 

diff --git a/dist/redolent.common.js b/dist/redolent.common.js
@@ -392,10 +392,17 @@ function redolent (fn, opts) {
 
       opts.args = isAsyncFn ? opts.args.concat(done) : opts.args;
       var syncResult = fn.apply(opts.context, opts.args);
-      var xPromise = isAsyncFn && !called && isPromise(syncResult, opts.Promise);
 
-      if ((!isAsyncFn && !called) || xPromise) {
+      var xPromise = isPromise(syncResult, opts.Promise);
+      var hasPromiseReturn = isAsyncFn && !called && xPromise;
+
+      if ((!isAsyncFn && !called) || hasPromiseReturn) {
         resolve(syncResult);
+        return
+      }
+      if (isAsyncFn && !xPromise && syncResult !== undefined) {
+        var msg = 'Asynchronous functions can only return a Promise or invoke a callback';
+        reject(new Error('redolent: ' + msg));
       }
     });
 

diff --git a/dist/redolent.es.js b/dist/redolent.es.js
@@ -388,10 +388,17 @@ function redolent (fn, opts) {
 
       opts.args = isAsyncFn ? opts.args.concat(done) : opts.args;
       var syncResult = fn.apply(opts.context, opts.args);
-      var xPromise = isAsyncFn && !called && isPromise(syncResult, opts.Promise);
 
-      if ((!isAsyncFn && !called) || xPromise) {
+      var xPromise = isPromise(syncResult, opts.Promise);
+      var hasPromiseReturn = isAsyncFn && !called && xPromise;
+
+      if ((!isAsyncFn && !called) || hasPromiseReturn) {
         resolve(syncResult);
+        return
+      }
+      if (isAsyncFn && !xPromise && syncResult !== undefined) {
+        var msg = 'Asynchronous functions can only return a Promise or invoke a callback';
+        reject(new Error('redolent: ' + msg));
       }
     });
 

diff --git a/index.js b/index.js
@@ -118,10 +118,17 @@ export default function redolent (fn, opts) {
 
       opts.args = isAsyncFn ? opts.args.concat(done) : opts.args
       var syncResult = fn.apply(opts.context, opts.args)
-      var xPromise = isAsyncFn && !called && isPromise(syncResult, opts.Promise)
 
-      if ((!isAsyncFn && !called) || xPromise) {
+      var xPromise = isPromise(syncResult, opts.Promise)
+      var hasPromiseReturn = isAsyncFn && !called && xPromise
+
+      if ((!isAsyncFn && !called) || hasPromiseReturn) {
         resolve(syncResult)
+        return
+      }
+      if (isAsyncFn && !xPromise && syncResult !== undefined) {
+        var msg = 'Asynchronous functions can only return a Promise or invoke a callback'
+        reject(new Error('redolent: ' + msg))
       }
     })
 

diff --git a/test.js b/test.js
@@ -66,10 +66,10 @@ function factory (promisify) {
     var readFile = promisify(fs.readFile, {
       Promise: Pinkie
     })
-    var promise = readFile('package.json')
+    var promise = readFile('package.json', 'utf8')
 
     promise.then(function (res) {
-      test.strictEqual(typeof res !== 'string', true)
+      test.strictEqual(typeof res, 'string')
       if (semver.lt(process.version, '0.11.13')) {
         test.strictEqual(promise.___customPromise, true)
       }
@@ -160,7 +160,7 @@ function factory (promisify) {
   })
 
   test('should work if `done` is present, but return a promise', function (done) {
-    var fn = promisify(function (xxx, done) {
+    var fn = promisify(function (xxx, cb) {
       return Pinkie.resolve(100 + xxx)
     })
 
@@ -169,6 +169,19 @@ function factory (promisify) {
       done()
     }, done).catch(done)
   })
+
+  test('should reject if async fn returns non Promise, nor call a callback', function (done) {
+    var func = promisify(function (cb) {
+      return 'foo bar'
+    })
+
+    func().catch(function (err) {
+      test.strictEqual(err instanceof Error, true)
+      test.strictEqual(/Asynchronous functions can only/.test(err.message), true)
+      test.strictEqual(/return a Promise or invoke a callback/.test(err.message), true)
+      done()
+    }).catch(done)
+  })
 }
 
 if (semver.lt(process.version, '0.12.0')) {