AutoRetry distinction between predicate truth and request failure

src/dapi/AutoRetry.js

@@ -23,6 +23,14 @@ var Trait   = require( 'easejs' ).Trait,
     DataApi = require( './DataApi' );
 
 
+/**
+ * Automatically retries requests while satisfying a given predicate
+ *
+ * It is important to distinguish between the concept of a request failure
+ * and a retry predicate: the former represents a problem with the request,
+ * whereas the latter indicates that a retry should be performed, but may
+ * not necessarily imply a request failure.
+ */
 module.exports = Trait( 'AutoRetry' )
     .implement( DataApi )
     .extend(
@@ -51,9 +59,9 @@ module.exports = Trait( 'AutoRetry' )
     /**
      * Initialize auto-retry
      *
-     * If TRIES is negative, then requests will continue indefinitely until
+     * If TRIES is negative, then requests will continue indefinitely while
-     * one succeeds or is aborted by DELAY.  If TRIES is 0, then no requests
+     * the retry predicate is true, or is aborted by DELAY.  If TRIES is 0,
-     * will be performed.
+     * then no requests will be performed.
      *
      * If DELAY is a function, then it invoked with a retry continuation
      * before each retry, the number of tries remaining, and a failure
@@ -87,7 +95,7 @@ module.exports = Trait( 'AutoRetry' )
 
         this._pred  = pred;
         this._tries = +tries;
-        this._delay = delay || function( _, c ) { c(); };
+        this._delay = delay || function( _, c, __ ) { c(); };
     },
 
 
@@ -100,13 +108,13 @@ module.exports = Trait( 'AutoRetry' )
      * necessarily asynchronously---that remains undefined).
      *
      * Otherwise, requests will continue to be re-issued until either the
-     * request succeeds or the number of retries are exhausted, whichever
+     * retry predicate fails or the number of retries are exhausted,
-     * comes first.  Once the retries are exhausted, the error and output
+     * whichever comes first.  Once the retries are exhausted, the error and
-     * data from the final request are returned.
+     * output data from the final request are returned.
      *
      * If the number of tries is negative, then requests will be performed
-     * indefinitely until success; the delay function (as provided via the
+     * indefinitely while the retry predicate is met; the delay function (as
-     * constructor) may be used to abort in this case.
+     * provided via the constructor) may be used to abort in this case.
      *
      * @param {string}             input    binary data to transmit
      * @param {function(?Error,*)} callback continuation upon reply
@@ -122,7 +130,8 @@ module.exports = Trait( 'AutoRetry' )
 
 
     /**
-     * Recursively perform request until success or try exhaustion
+     * Recursively perform request until retry predicate failure or try
+     * count exhaustion
      *
      * For more information, see `#request'.
      *
@@ -146,22 +155,22 @@ module.exports = Trait( 'AutoRetry' )
 
         this.request.super.call( this, input, function( err, output )
         {
+            var complete = function()
+            {
+                callback( err, output );
+            };
+
             // predicate determines whether a retry is necessary
             if ( !!_self._pred( err, output ) === false )
             {
-                return _self._succeed( output, callback );
+                return complete();
             }
 
-            var fail = function()
-            {
-                _self._fail( err, output, callback );
-            };
-
             // note that we intentionally do not want to check <= 1, so that
             // we can proceed indefinitely (JavaScript does not wrap on overflow)
             if ( n === 1 )
             {
-                return fail();
+                return complete();
             }
 
             _self._delay(
@@ -170,37 +179,8 @@ module.exports = Trait( 'AutoRetry' )
                 {
                     _self._try( input, callback, ( n - 1 ) );
                 },
-                fail
+                complete
             );
         } );
     },
-
-
-    /**
-     * Produce a successful response
-     *
-     * @param {*}                  output   output data
-     * @param {function(?Error,*)} callback continuation to invoke
-     *
-     * @return {undefined}
-     */
-    'private _succeed': function( output, callback )
-    {
-        callback( null, output );
-    },
-
-
-    /**
-     * Produce a negative response
-     *
-     * @param {Error}              err      most recent error
-     * @param {*}                  output   most recent output data
-     * @param {function(?Error,*)} callback continuation to invoke
-     *
-     * @return {undefined}
-     */
-    'private _fail': function( err, output, callback )
-    {
-        callback( err, output );
-    },
 } );

test/dapi/AutoRetryTest.js

@@ -32,9 +32,9 @@ var _void = function() {},
 describe( 'dapi.AutoRetry trait', function()
 {
     /**
-     * If there are no failures, then AutoRetry has no observable effects.
+     * If there are no retries, then AutoRetry has no observable effects.
      */
-    describe( 'when the request is successful', function()
+    describe( 'when the request does not need retrying', function()
     {
         it( 'makes only one request', function( done )
         {
@@ -55,19 +55,25 @@ describe( 'dapi.AutoRetry trait', function()
         } );
 
 
-        it( 'returns the response data with no error', function( done )
+        /**
+         * We expect that any error will be proxied back to us; this is an
+         * important concept, since it allow separating the idea of a
+         * "retry" from that of a "failure": the latter represents a problem
+         * with the request, whereas the former indicates that a request
+         * should be performed once again.
+         */
+        it( 'returns the response data, including any error', function( done )
         {
-            var chk = { foo: 'bar' };
+            var chk     = { foo: 'bar' },
+                chk_err = Error( 'foo' );
 
-            // notice that we provide an error to the stub; this will ensure
+            var stub = _createStub( chk_err, chk )
-            // that the returned error is null even when one is provided
-            var stub = _createStub( {}, chk )
                 .use( Sut( _void, 1 ) )
                 ();
 
             stub.request( '', function( err, data )
             {
-                expect( err ).to.equal( null );
+                expect( err ).to.equal( chk_err );
                 expect( data ).to.equal( chk );
                 done();
             } );
@@ -78,9 +84,9 @@ describe( 'dapi.AutoRetry trait', function()
     /**
      * This is when we care.
      */
-    describe( 'when the request fails', function()
+    describe( 'when the retry predicate is true', function()
     {
-        it( 'will re-perform request N-1 times until failure', function( done )
+        it( 'will re-perform request N-1 times until false', function( done )
         {
             var n = 5;
 
@@ -207,7 +213,7 @@ describe( 'dapi.AutoRetry trait', function()
         } );
 
 
-        it( 'will call delay function until success', function()
+        it( 'will call delay function until predicate falsity', function()
         {
             var n = 5;
             var wait = function( tries_left, c )