Changed return type of retryStrategy to [mustRetry, options], overw…

…rite `options` if strategy returned it

index.js

@@ -80,7 +80,7 @@ function Request(url, options, f, retryConfig) {
 
   /**
    * Return true if the request should be retried
-   * @type {Function} (err, response) -> Boolean
+   * @type {Function} (err, response, body, options) -> [Boolean, Object (optional)]
    */
   this.retryStrategy = _.isFunction(options.retryStrategy) ? options.retryStrategy : RetryStrategies.HTTPOrNetworkError;
 
@@ -130,7 +130,11 @@ Request.prototype._tryUntilFail = function () {
       err.attempts = this.attempts;
     }
 
-    if (this.retryStrategy(err, response, body) && this.maxAttempts > 0) {
+    var [mustRetry, options] = this.retryStrategy(err, response, body, this.options);
+    if (_.isObject(options)) {
+      this.options = options;
+    }
+    if (mustRetry && this.maxAttempts > 0) {
       this._timeout = setTimeout(this._tryUntilFail.bind(this), this.delayStrategy.call(this, err, response, body));
       return;
     }

strategies/HTTPError.js

@@ -3,11 +3,11 @@
 /**
  * @param  {Null | Object} err
  * @param  {Object} response
- * @return {Boolean} true if the request had a recoverable HTTP error
+ * @return {Array} with first Boolean item, true if the request had a recoverable HTTP error
  */
 module.exports = function HTTPError(err, response) {
   const statusCode = response ? response.statusCode : null;
 
   // 429 means "Too Many Requests" while 5xx means "Server Error"
-  return statusCode && (statusCode === 429 || (500 <= statusCode && statusCode < 600));
+  return [statusCode && (statusCode === 429 || (500 <= statusCode && statusCode < 600))];
 };

strategies/HTTPOrNetworkError.js

@@ -3,10 +3,10 @@ module.exports = function HTTPOrNetworkError(httpError, networkError) {
   /**
    * @param  {Null | Object} err
    * @param  {Object} response
-   * @return {Boolean} true if the request had a recoverable HTTP or network error
+   * @return {Array} with first Boolean item, true if the request had a recoverable HTTP or network error
    */
   return function HTTPError(err, response) {
-    return httpError(err, response) || networkError(err, response);
+    return [httpError(err, response)[0] || networkError(err, response)[0]];
   };
 
 };

strategies/NetworkError.js

@@ -6,10 +6,10 @@ var _ = require('lodash');
 /**
  * @param  {Null | Object} err
  * @param  {Object} response
- * @return {Boolean} true if the request had a network error
+ * @return {Array} with first Boolean item, true if the request had a network error
  */
 function NetworkError(err /*, response*/ ) {
-  return err && _.includes(RETRIABLE_ERRORS, err.code);
+  return [err && _.includes(RETRIABLE_ERRORS, err.code)];
 }
 
 NetworkError.RETRIABLE_ERRORS = RETRIABLE_ERRORS;

test/attempts.test.js

@@ -38,6 +38,7 @@ describe('Request attempts', function () {
   });
 
   it('should call delay strategy 2 times after some retries', function (done) {
+    this.timeout(3000);
     var mockDelayStrategy = sinon.stub().returns(500);
     var startTime = process.hrtime();
     request({

test/strategies.test.js

@@ -4,44 +4,66 @@ var request = require('../');
 var t = require('chai').assert;
 
 describe('RetryStrategies', function () {
-  it('should have a strategy `HTTPError` to only retry on HTTP errors', function () {
-    checkHTTPErrors(request.RetryStrategies.HTTPError);
-  });
 
-  it('should have a strategy `NetworkError` to only retry on nodejs network errors', function () {
-    checkNetworkErrors(request.RetryStrategies.NetworkError, request.RetryStrategies.NetworkError.RETRIABLE_ERRORS);
+  describe('Default strategies', function () {
+    it('should have a strategy `HTTPError` to only retry on HTTP errors', function () {
+      checkHTTPErrors(request.RetryStrategies.HTTPError);
+    });
+
+    it('should have a strategy `NetworkError` to only retry on nodejs network errors', function () {
+      checkNetworkErrors(request.RetryStrategies.NetworkError, request.RetryStrategies.NetworkError.RETRIABLE_ERRORS);
+    });
+
+    it('should have a strategy `HTTPOrNetworkError` to only retry on nodejs network and HTTP errors', function () {
+      checkHTTPErrors(request.RetryStrategies.HTTPOrNetworkError);
+      checkNetworkErrors(request.RetryStrategies.HTTPOrNetworkError, request.RetryStrategies.NetworkError.RETRIABLE_ERRORS);
+    });
   });
 
-  it('should have a strategy `HTTPOrNetworkError` to only retry on nodejs network and HTTP errors', function () {
-    checkHTTPErrors(request.RetryStrategies.HTTPOrNetworkError);
-    checkNetworkErrors(request.RetryStrategies.HTTPOrNetworkError, request.RetryStrategies.NetworkError.RETRIABLE_ERRORS);
+  describe('Custom strategies', function () {
+    it('should overwrite `options` object if strategy returned it', function (done) {
+      var strategy = function (err, response, body, options) {
+        options.url = 'http://www.filltext.com/?rows=1&err=200'; //overwrite url to return 200
+        return [true, options];
+      };
+
+      request({
+        url: 'http://www.filltext.com/?rows=1&err=500', // return a 500 status
+        maxAttempts: 3,
+        retryDelay: 100,
+        retryStrategy: strategy
+      }, function(err, response, body) {
+        t.strictEqual(200, response.statusCode);
+        done();
+      });
+    });
   });
 });
 
 function checkNetworkErrors(strategy, errorCodes) {
   errorCodes.forEach(function (errorCode) {
     var err = new Error();
     err.code = errorCode;
-    t.ok(strategy(err), 'error code ' + errorCode + ' is recoverable');
+    t.ok(strategy(err)[0], 'error code ' + errorCode + ' is recoverable');
   });
 
   ['hello', 'plop'].forEach(function (errorCode) {
     var err = new Error();
     err.code = errorCode;
-    t.ok(!strategy(err), 'error code ' + errorCode + ' is not recoverable');
+    t.ok(!strategy(err)[0], 'error code ' + errorCode + ' is not recoverable');
   });
 }
 
 function checkHTTPErrors(strategy) {
   [400, 301, 600].forEach(function (code) {
     t.ok(!strategy(null, {
       statusCode: code
-    }), code + ' error is not recoverable');
+    })[0], code + ' error is not recoverable');
   });
 
   [429, 500, 599].forEach(function (code) {
     t.ok(strategy(null, {
       statusCode: code
-    }), code + ' error is recoverable');
+    })[0], code + ' error is recoverable');
   });
 }