saas-1116 request-retry (#253)

* request-retry

* error handling redone

* config examples + version updated

* not proper redirects

Author: yaroslav-codefresh

lib/interface/cli/commands/cli-config/ops/set.cmd.js

@@ -25,7 +25,9 @@ const setCommand = new Command({
                 describe: 'Property value',
             })
             .example('codefresh cli-config set', 'Print available property names')
-            .example('codefresh cli-config set output.pretty false', 'Set "output.pretty" property');
+            .example('codefresh cli-config set output.pretty false', 'Set "output.pretty" property')
+            .example('codefresh cli-config set request.maxRetries 10', 'Set "request.maxRetries" property')
+            .example('codefresh cli-config set request.retryDelay 500', 'Set "request.retryDelay" property');
     },
     handler: async (argv) => {
         if (argv.help) {

lib/interface/cli/helpers/general.js

@@ -6,9 +6,10 @@ const defaults = require('../defaults');
 const CFError = require('cf-errors');
 const path = require('path');
 
+const isDebug = () => (process.env.DEBUG || '').includes(defaults.DEBUG_PATTERN);
 
 const printError = (error) => {
-    if ((process.env.DEBUG || '').includes(defaults.DEBUG_PATTERN)) {
+    if (isDebug()) {
         console.error(error.stack);
     } else {
         console.error(`${error.toString()}`);
@@ -152,4 +153,5 @@ module.exports = {
     pathExists,
     readFile,
     watchFile,
+    isDebug,
 };

lib/logic/api/helper.js

@@ -1,13 +1,26 @@
 const debug = require('debug')('codefresh:http');
 const fs = require('fs');
 const path = require('path');
-const rp = require('request-promise');
+const request = require('requestretry');
 const _ = require('lodash');
 const CFError = require('cf-errors');
-const { printError } = require('../../interface/cli/helpers/general');
+const { printError, isDebug } = require('../../interface/cli/helpers/general');
+const config = require('../../logic/cli-config/Manager').config();
 
 const { version } = JSON.parse(fs.readFileSync(path.resolve(__dirname, '../../../package.json')));
 
+const RETRY_STATUS_CODES = [502, 503, 504];
+const retryOptions = {
+    fullResponse: true,
+    maxAttempts: config.request.maxRetries,
+    retryDelay: config.request.retryDelay,
+    retryStrategy: (err, response) => !isDebug() && (request.RetryStrategies.NetworkError(err) || RETRY_STATUS_CODES.includes(response.statusCode)),
+};
+
+function _makeResponseError(response) {
+    return new Error(JSON.stringify(response.body || response));
+}
+
 const sendHttpRequest = async (httpOptions, authContext, throwOnUnauthorized, timeout = 30000) => {
     let finalAuthContext;
     if (!authContext) {
@@ -20,6 +33,7 @@ const sendHttpRequest = async (httpOptions, authContext, throwOnUnauthorized, ti
     const finalOptions = _.merge(
         httpOptions,
         finalAuthContext.prepareHttpOptions(),
+        retryOptions,
         {
             json: true,
             timeout,
@@ -30,45 +44,55 @@ const sendHttpRequest = async (httpOptions, authContext, throwOnUnauthorized, ti
         },
     );
     debug('Sending http request:\n%O', finalOptions);
-    let response;
-    try {
-        response = await rp(finalOptions);
-    } catch (err) {
-        debug('Response:\n%O', err.response.body);
 
-        if (_.isEqual(err.statusCode, 401)) {
-            const error = new CFError({
-                cause: err,
-                message: 'Error: Please create or update your authentication context',
-            });
+    // only network errors will be thrown -- no need to catch
+    const response = await request(finalOptions);
 
-            if (!throwOnUnauthorized) {
-                printError(error);
-                process.exit(1);
-            } else {
-                throw error;
-            }
-        }
-        if (_.isEqual(err.statusCode, 403)) {
-            printError(new CFError({
-                cause: err,
-                message: 'Error: You do not have permissions to perform this action',
-            }));
+    debug('Response:\n%O', response.body);
+
+    const { statusCode } = response;
+
+    // if for some reason request was not properly redirected (when "Location" header is lost, not usual case)
+    if (statusCode >= 300 && statusCode < 400) {
+        throw new CFError({
+            cause: _makeResponseError(response),
+            message: 'Error: Request was not properly redirected',
+        });
+    }
+    if (statusCode === 401) {
+        const error = new CFError({
+            cause: _makeResponseError(response),
+            message: 'Error: Please create or update your authentication context',
+        });
+
+        if (!throwOnUnauthorized) {
+            printError(error);
             process.exit(1);
+        } else {
+            throw error;
         }
+    }
+    if (statusCode === 403) {
+        printError(new CFError({
+            cause: _makeResponseError(response),
+            message: 'Error: You do not have permissions to perform this action',
+        }));
+        process.exit(1);
+    }
 
-        if (_.get(err, 'error.message')) {
-            if (_.get(err, 'error.error')) {
-                throw new Error(`message: ${err.error.message}\nerror: ${err.error.error}`);
+    // other status codes
+    if (statusCode >= 400 && statusCode < 600) {
+        if (_.get(response, 'body.message')) {
+            if (_.get(response, 'body.error')) {
+                throw new Error(`message: ${response.body.message}\nerror: ${response.body.error}`);
             } else {
-                throw new Error(err.error.message);
+                throw new Error(response.body.message);
             }
         } else {
-            throw err;
+            throw _makeResponseError(response);
         }
     }
-    debug('Response:\n%O', response);
-    return response;
+    return response.body;
 };
 
 module.exports = {

lib/logic/cli-config/schema.json

@@ -12,6 +12,21 @@
                     "description": "Defines whether to show data in table view in pretty mode or not"
                 }
             }
+        },
+        "request": {
+            "type": "object",
+            "properties": {
+                "maxRetries": {
+                    "type": "integer",
+                    "default": 10,
+                    "description": "Defines how many times request will be retried in case of network error"
+                },
+                "retryDelay": {
+                    "type": "integer",
+                    "default": 500,
+                    "description": "Defines delay between retries"
+                }
+            }
         }
     }
 }

package.json

@@ -1,6 +1,6 @@
 {
   "name": "codefresh",
-  "version": "0.9.9",
+  "version": "0.9.10",
   "description": "Codefresh command line utility",
   "main": "index.js",
   "preferGlobal": true,
@@ -56,6 +56,7 @@
     "recursive-readdir": "^2.2.1",
     "request": "^2.88.0",
     "request-promise": "^4.2.2",
+    "requestretry": "^3.1.0",
     "rimraf": "^2.6.2",
     "tty-table": "^2.5.5",
     "uuid": "^3.1.0",

yarn.lock

@@ -1608,7 +1608,7 @@ extend@3, extend@^3.0.0, extend@~3.0.0, extend@~3.0.1:
   version "3.0.1"
   resolved "https://registry.yarnpkg.com/extend/-/extend-3.0.1.tgz#a755ea7bc1adfcc5a31ce7e762dbaadc5e636444"
 
-extend@~3.0.2:
+extend@^3.0.2, extend@~3.0.2:
   version "3.0.2"
   resolved "https://registry.yarnpkg.com/extend/-/extend-3.0.2.tgz#f8b1136b4071fbd8eb140aff858b1019ec2915fa"
 
@@ -3275,6 +3275,11 @@ lodash@^4.13.1, lodash@^4.14.0, lodash@^4.17.4, lodash@^4.3.0:
   version "4.17.5"
   resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.5.tgz#99a92d65c0272debe8c96b6057bc8fbfa3bed511"
 
+lodash@^4.17.10:
+  version "4.17.11"
+  resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.11.tgz#b39ea6229ef607ecd89e2c8df12536891cac9b8d"
+  integrity sha512-cQKh8igo5QUhZ7lg38DYWAxMvjSAKG0A8wGSVimP07SIUEK2UO+arSRKbRZWtelMtN5V0Hkwh5ryOto/SshYIg==
+
 log-symbols@^2.2.0:
   version "2.2.0"
   resolved "https://registry.yarnpkg.com/log-symbols/-/log-symbols-2.2.0.tgz#5740e1c5d6f0dfda4ad9323b5332107ef6b4c40a"
@@ -4392,6 +4397,15 @@ request@^2.88.0:
     tunnel-agent "^0.6.0"
     uuid "^3.3.2"
 
+requestretry@^3.1.0:
+  version "3.1.0"
+  resolved "https://registry.yarnpkg.com/requestretry/-/requestretry-3.1.0.tgz#c8e1976bb946f14889d3604bbad56a01d191c10d"
+  integrity sha512-DkvCPK6qvwxIuVA5TRCvi626WHC2rWjF/n7SCQvVHAr2JX9i1/cmIpSEZlmHAo+c1bj9rjaKoZ9IsKwCpTkoXA==
+  dependencies:
+    extend "^3.0.2"
+    lodash "^4.17.10"
+    when "^3.7.7"
+
 require-directory@^2.1.1:
   version "2.1.1"
   resolved "https://registry.yarnpkg.com/require-directory/-/require-directory-2.1.1.tgz#8c64ad5fd30dab1c976e2344ffe7f792a6a6df42"
@@ -5322,6 +5336,11 @@ whatwg-url@^6.4.0:
     tr46 "^1.0.0"
     webidl-conversions "^4.0.1"
 
+when@^3.7.7:
+  version "3.7.8"
+  resolved "https://registry.yarnpkg.com/when/-/when-3.7.8.tgz#c7130b6a7ea04693e842cdc9e7a1f2aa39a39f82"
+  integrity sha1-xxMLan6gRpPoQs3J56Hyqjmjn4I=
+
 which-module@^2.0.0:
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/which-module/-/which-module-2.0.0.tgz#d9ef07dce77b9902b8a3a8fa4b31c3e3f7e6e87a"