Some fixes for retry()

Author: calvinwiebe

src/index.js

@@ -208,7 +208,7 @@ exports.doWhilst = (fn, test) => {
 
 /*
  * keep calling `fn` until it returns a non-error value, doesn't throw, or returns a Promise that resolves. `fn` will be
- * attempted `times` many times before rejecting. If `times` is given as `exports.FOREVER`, then `retry` will attempt to
+ * attempted `times` many times before rejecting. If `times` is given as `Infinity`, then `retry` will attempt to
  * resolve forever (useful if you are just waiting for something to finish).
  * @param {Object|Number} options hash to provide `times` and `interval`. Defaults (times=5, interval=0). If this value
  *                        is a number, only `times` will be set.
@@ -222,16 +222,20 @@ exports.retry = (options, fn) => {
     let attempts = 0;
     let lastAttempt = null;
 
-    if ('number' === typeof(options)) {
-        times = options;
+    function makeTimeOptionError(value) {
+        return new Error(`Unsupported argument type for \'times\': ${typeof(value)}`);
     }
+
+    if ('function' === typeof(options)) fn = options;
+    else if ('number' === typeof(options)) times = +options;
     else if ('object' === typeof(options)) {
-        if (options.times) {times = parseInt(options.times, 10);}
-        if (options.interval) {interval = parseInt(options.interval, 10);}
-    }
-    else {
-        throw new Error('Unsupported argument type for \'times\': ' + typeof(options));
+        if ('number' === typeof(options.times)) times = +options.times;
+        else if (options.times) return Promise.reject(makeTimeOptionError(options.times));
+
+        if (options.interval) interval = parseInt(options.interval, 10);
     }
+    else if (options) return Promise.reject(makeTimeOptionError(options));
+    else return Promise.reject(new Error('No parameters given'));
 
     return new Promise((resolve, reject) => {
         let doIt = () => {

test/retry.js

@@ -39,6 +39,8 @@ describe('retry', () => {
     [
         {options: {times: 5, interval: 10}, msg: 'times and interval'},
         {options: {times: 5}, msg: 'just times'},
+        {options: {times: 5.4}, msg: 'just times'},
+        {options: {times: Infinity}, msg: 'times = Infinity'},
         {options: {}, msg: 'neither times nor interval'}
     ].forEach((args) => {
         it(`should accept first argument as an options hash with ${args.msg}`, () => {
@@ -47,15 +49,21 @@ describe('retry', () => {
         });
     });
 
+    it('should call the default 5 times with no options provided', () => {
+        let retry = promiseTools.retry(getTest(5));
+        return expect(retry).to.eventually.equal(5);
+    })
+
+    it('should throw when given a bogus `times` option', () => {
+        let retry = promiseTools.retry('5', getTest(5));
+        return expect(retry).to.be.eventually.rejected;
+    })
+
     it('should return an error with invalid options argument', () => {
-        let p = Promise.resolve().then(() => {
-            // had to do this for some reason. Otherwise `chai-as-promised` always passed erroneously.
-            try {
-                return promiseTools.retry(undefined, getTest(1));
-            } catch (err) {
-                throw err;
-            }
-        });
-        expect(p).to.eventually.be.rejectedWith('Unsupported argument type for \'times\': undefined');
+        expect(promiseTools.retry(undefined, getTest(1))).to.eventually.be.rejectedWith('Unsupported argument type for \'times\': undefined');
     });
+
+    it('should reject when called with nothing', () => {
+        expect(promiseTools.retry()).to.eventually.be.rejectedWith('No parameters given');
+    })
 });