Skip to content

Commit f57489a

Browse files
yaroslav-codefreshyaroslav-codefresh
authored
saas-1116 request-retry (#253)
* request-retry * error handling redone * config examples + version updated * not proper redirects
1 parent 792f1e7 commit f57489a

File tree

6 files changed

+98
-35
lines changed

6 files changed

+98
-35
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ const setCommand = new Command({
2525
describe: 'Property value',
2626
})
2727
.example('codefresh cli-config set', 'Print available property names')
28-
.example('codefresh cli-config set output.pretty false', 'Set "output.pretty" property');
28+
.example('codefresh cli-config set output.pretty false', 'Set "output.pretty" property')
29+
.example('codefresh cli-config set request.maxRetries 10', 'Set "request.maxRetries" property')
30+
.example('codefresh cli-config set request.retryDelay 500', 'Set "request.retryDelay" property');
2931
},
3032
handler: async (argv) => {
3133
if (argv.help) {

‎lib/interface/cli/helpers/general.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@ const defaults = require('../defaults');
66
const CFError = require('cf-errors');
77
const path = require('path');
88

9+
const isDebug = () => (process.env.DEBUG || '').includes(defaults.DEBUG_PATTERN);
910

1011
const printError = (error) => {
11-
if ((process.env.DEBUG || '').includes(defaults.DEBUG_PATTERN)) {
12+
if (isDebug()) {
1213
console.error(error.stack);
1314
} else {
1415
console.error(`${error.toString()}`);
@@ -152,4 +153,5 @@ module.exports = {
152153
pathExists,
153154
readFile,
154155
watchFile,
156+
isDebug,
155157
};

‎lib/logic/api/helper.js‎

Lines changed: 55 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,26 @@
11
const debug = require('debug')('codefresh:http');
22
const fs = require('fs');
33
const path = require('path');
4-
const rp = require('request-promise');
4+
const request = require('requestretry');
55
const _ = require('lodash');
66
const CFError = require('cf-errors');
7-
const { printError } = require('../../interface/cli/helpers/general');
7+
const { printError, isDebug } = require('../../interface/cli/helpers/general');
8+
const config = require('../../logic/cli-config/Manager').config();
89

910
const { version } = JSON.parse(fs.readFileSync(path.resolve(__dirname, '../../../package.json')));
1011

12+
const RETRY_STATUS_CODES = [502, 503, 504];
13+
const retryOptions = {
14+
fullResponse: true,
15+
maxAttempts: config.request.maxRetries,
16+
retryDelay: config.request.retryDelay,
17+
retryStrategy: (err, response) => !isDebug() && (request.RetryStrategies.NetworkError(err) || RETRY_STATUS_CODES.includes(response.statusCode)),
18+
};
19+
20+
function _makeResponseError(response) {
21+
return new Error(JSON.stringify(response.body || response));
22+
}
23+
1124
const sendHttpRequest = async (httpOptions, authContext, throwOnUnauthorized, timeout = 30000) => {
1225
let finalAuthContext;
1326
if (!authContext) {
@@ -20,6 +33,7 @@ const sendHttpRequest = async (httpOptions, authContext, throwOnUnauthorized, ti
2033
const finalOptions = _.merge(
2134
httpOptions,
2235
finalAuthContext.prepareHttpOptions(),
36+
retryOptions,
2337
{
2438
json: true,
2539
timeout,
@@ -30,45 +44,55 @@ const sendHttpRequest = async (httpOptions, authContext, throwOnUnauthorized, ti
3044
},
3145
);
3246
debug('Sending http request:\n%O', finalOptions);
33-
let response;
34-
try {
35-
response = await rp(finalOptions);
36-
} catch (err) {
37-
debug('Response:\n%O', err.response.body);
3847

39-
if (_.isEqual(err.statusCode, 401)) {
40-
const error = new CFError({
41-
cause: err,
42-
message: 'Error: Please create or update your authentication context',
43-
});
48+
// only network errors will be thrown -- no need to catch
49+
const response = await request(finalOptions);
4450

45-
if (!throwOnUnauthorized) {
46-
printError(error);
47-
process.exit(1);
48-
} else {
49-
throw error;
50-
}
51-
}
52-
if (_.isEqual(err.statusCode, 403)) {
53-
printError(new CFError({
54-
cause: err,
55-
message: 'Error: You do not have permissions to perform this action',
56-
}));
51+
debug('Response:\n%O', response.body);
52+
53+
const { statusCode } = response;
54+
55+
// if for some reason request was not properly redirected (when "Location" header is lost, not usual case)
56+
if (statusCode >= 300 && statusCode < 400) {
57+
throw new CFError({
58+
cause: _makeResponseError(response),
59+
message: 'Error: Request was not properly redirected',
60+
});
61+
}
62+
if (statusCode === 401) {
63+
const error = new CFError({
64+
cause: _makeResponseError(response),
65+
message: 'Error: Please create or update your authentication context',
66+
});
67+
68+
if (!throwOnUnauthorized) {
69+
printError(error);
5770
process.exit(1);
71+
} else {
72+
throw error;
5873
}
74+
}
75+
if (statusCode === 403) {
76+
printError(new CFError({
77+
cause: _makeResponseError(response),
78+
message: 'Error: You do not have permissions to perform this action',
79+
}));
80+
process.exit(1);
81+
}
5982

60-
if (_.get(err, 'error.message')) {
61-
if (_.get(err, 'error.error')) {
62-
throw new Error(`message: ${err.error.message}\nerror: ${err.error.error}`);
83+
// other status codes
84+
if (statusCode >= 400 && statusCode < 600) {
85+
if (_.get(response, 'body.message')) {
86+
if (_.get(response, 'body.error')) {
87+
throw new Error(`message: ${response.body.message}\nerror: ${response.body.error}`);
6388
} else {
64-
throw new Error(err.error.message);
89+
throw new Error(response.body.message);
6590
}
6691
} else {
67-
throw err;
92+
throw _makeResponseError(response);
6893
}
6994
}
70-
debug('Response:\n%O', response);
71-
return response;
95+
return response.body;
7296
};
7397

7498
module.exports = {

‎lib/logic/cli-config/schema.json‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,21 @@
1212
"description": "Defines whether to show data in table view in pretty mode or not"
1313
}
1414
}
15+
},
16+
"request": {
17+
"type": "object",
18+
"properties": {
19+
"maxRetries": {
20+
"type": "integer",
21+
"default": 10,
22+
"description": "Defines how many times request will be retried in case of network error"
23+
},
24+
"retryDelay": {
25+
"type": "integer",
26+
"default": 500,
27+
"description": "Defines delay between retries"
28+
}
29+
}
1530
}
1631
}
1732
}

‎package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "codefresh",
3-
"version": "0.9.9",
3+
"version": "0.9.10",
44
"description": "Codefresh command line utility",
55
"main": "index.js",
66
"preferGlobal": true,
@@ -56,6 +56,7 @@
5656
"recursive-readdir": "^2.2.1",
5757
"request": "^2.88.0",
5858
"request-promise": "^4.2.2",
59+
"requestretry": "^3.1.0",
5960
"rimraf": "^2.6.2",
6061
"tty-table": "^2.5.5",
6162
"uuid": "^3.1.0",

‎yarn.lock‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1608,7 +1608,7 @@ extend@3, extend@^3.0.0, extend@~3.0.0, extend@~3.0.1:
16081608
version "3.0.1"
16091609
resolved "https://registry.yarnpkg.com/extend/-/extend-3.0.1.tgz#a755ea7bc1adfcc5a31ce7e762dbaadc5e636444"
16101610

1611-
extend@~3.0.2:
1611+
extend@^3.0.2, extend@~3.0.2:
16121612
version "3.0.2"
16131613
resolved "https://registry.yarnpkg.com/extend/-/extend-3.0.2.tgz#f8b1136b4071fbd8eb140aff858b1019ec2915fa"
16141614

@@ -3275,6 +3275,11 @@ lodash@^4.13.1, lodash@^4.14.0, lodash@^4.17.4, lodash@^4.3.0:
32753275
version "4.17.5"
32763276
resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.5.tgz#99a92d65c0272debe8c96b6057bc8fbfa3bed511"
32773277

3278+
lodash@^4.17.10:
3279+
version "4.17.11"
3280+
resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.11.tgz#b39ea6229ef607ecd89e2c8df12536891cac9b8d"
3281+
integrity sha512-cQKh8igo5QUhZ7lg38DYWAxMvjSAKG0A8wGSVimP07SIUEK2UO+arSRKbRZWtelMtN5V0Hkwh5ryOto/SshYIg==
3282+
32783283
log-symbols@^2.2.0:
32793284
version "2.2.0"
32803285
resolved "https://registry.yarnpkg.com/log-symbols/-/log-symbols-2.2.0.tgz#5740e1c5d6f0dfda4ad9323b5332107ef6b4c40a"
@@ -4392,6 +4397,15 @@ request@^2.88.0:
43924397
tunnel-agent "^0.6.0"
43934398
uuid "^3.3.2"
43944399

4400+
requestretry@^3.1.0:
4401+
version "3.1.0"
4402+
resolved "https://registry.yarnpkg.com/requestretry/-/requestretry-3.1.0.tgz#c8e1976bb946f14889d3604bbad56a01d191c10d"
4403+
integrity sha512-DkvCPK6qvwxIuVA5TRCvi626WHC2rWjF/n7SCQvVHAr2JX9i1/cmIpSEZlmHAo+c1bj9rjaKoZ9IsKwCpTkoXA==
4404+
dependencies:
4405+
extend "^3.0.2"
4406+
lodash "^4.17.10"
4407+
when "^3.7.7"
4408+
43954409
require-directory@^2.1.1:
43964410
version "2.1.1"
43974411
resolved "https://registry.yarnpkg.com/require-directory/-/require-directory-2.1.1.tgz#8c64ad5fd30dab1c976e2344ffe7f792a6a6df42"
@@ -5322,6 +5336,11 @@ whatwg-url@^6.4.0:
53225336
tr46 "^1.0.0"
53235337
webidl-conversions "^4.0.1"
53245338

5339+
when@^3.7.7:
5340+
version "3.7.8"
5341+
resolved "https://registry.yarnpkg.com/when/-/when-3.7.8.tgz#c7130b6a7ea04693e842cdc9e7a1f2aa39a39f82"
5342+
integrity sha1-xxMLan6gRpPoQs3J56Hyqjmjn4I=
5343+
53255344
which-module@^2.0.0:
53265345
version "2.0.0"
53275346
resolved "https://registry.yarnpkg.com/which-module/-/which-module-2.0.0.tgz#d9ef07dce77b9902b8a3a8fa4b31c3e3f7e6e87a"

0 commit comments

Comments
 (0)