Skip to content

Commit 4beb454

Browse files
jlogsdonjlogsdon
authored
Add retry support for certain upload failures (#9)
* Add retry support for certain upload failures * Remove duplicate describe key
1 parent 0148018 commit 4beb454

File tree

3 files changed

+127
-27
lines changed

3 files changed

+127
-27
lines changed

‎src/__specs__/cli.spec.js

+85-25
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ import getRawBody from 'raw-body';
66
const CLI_INDEX = './bin/logrocket';
77
const FIXTURE_PATH = './test/fixtures/';
88

9-
const executeCommand = async (cmd, { env = '' } = {}) => {
9+
const executeCommand = async (cmdAsStringOrArray, { env = '' } = {}) => {
10+
const cmd = Array.isArray(cmdAsStringOrArray) ? cmdAsStringOrArray.join(' ') : cmdAsStringOrArray;
1011
return new Promise(resolve => {
1112
exec(
1213
`${env} ${CLI_INDEX} ${cmd}`,
@@ -26,11 +27,24 @@ describe('CLI dispatch tests', function cliTests() {
2627
let matchedRequests;
2728

2829
const addExpectRequest = (url, opts) => {
29-
expectRequests[url] = {
30+
if (!expectRequests[url]) {
31+
expectRequests[url] = [];
32+
}
33+
34+
expectRequests[url].push({
3035
body: {},
3136
status: 200,
3237
...opts,
33-
};
38+
});
39+
};
40+
41+
const addArtifactRequest = () => {
42+
const uploadID = (100000 + Math.floor(Math.random() * 999999)).toString(16);
43+
addExpectRequest('/v1/orgs/org/apps/app/releases/1.0.2/artifacts/', {
44+
status: 200,
45+
body: { signed_url: `http://localhost:8818/upload/${uploadID}` },
46+
});
47+
addExpectRequest(`/upload/${uploadID}`, { status: 200 });
3448
};
3549

3650
const addCliStatusMessage = ({ message = '', status = 204 } = {}) => {
@@ -59,15 +73,16 @@ describe('CLI dispatch tests', function cliTests() {
5973
};
6074
server = createServer(async (req, res) => {
6175
const parts = parse(req.url);
76+
const expected = expectRequests[parts.pathname] || [];
6277

63-
if (expectRequests[parts.pathname]) {
78+
if (expected && expected.length) {
6479
const body = await getRawBody(req);
6580
const req2 = req;
6681

6782
req2.body = body.toString();
6883
matchedRequests.push(simplifyRequest(req2));
6984

70-
const request = expectRequests[parts.pathname];
85+
const request = expected.shift();
7186

7287
res.writeHead(request.status, { 'Content-Type': 'application/json' });
7388
res.write(JSON.stringify(request.body));
@@ -281,12 +296,10 @@ describe('CLI dispatch tests', function cliTests() {
281296

282297
it('should upload the passed directory', mochaAsync(async () => {
283298
addCliStatusMessage();
284-
addExpectRequest('/v1/orgs/org/apps/app/releases/1.0.2/artifacts/', {
285-
status: 200,
286-
body: { signed_url: 'http://localhost:8818/upload/' },
287-
});
288299

289-
addExpectRequest('/upload/', { status: 200 });
300+
addArtifactRequest();
301+
addArtifactRequest();
302+
addArtifactRequest();
290303

291304
const result = await executeCommand(`upload -k org:app:secret -r 1.0.2 --apihost="http://localhost:8818" ${FIXTURE_PATH}`);
292305

@@ -336,12 +349,10 @@ describe('CLI dispatch tests', function cliTests() {
336349

337350
it('should support a custom url prefix', mochaAsync(async () => {
338351
addCliStatusMessage();
339-
addExpectRequest('/v1/orgs/org/apps/app/releases/1.0.2/artifacts/', {
340-
status: 200,
341-
body: { signed_url: 'http://localhost:8818/upload/' },
342-
});
343352

344-
addExpectRequest('/upload/', { status: 200 });
353+
addArtifactRequest();
354+
addArtifactRequest();
355+
addArtifactRequest();
345356

346357
const result = await executeCommand(`upload -k org:app:secret -r 1.0.2 --apihost="http://localhost:8818" ${FIXTURE_PATH} --url-prefix="~/public"`);
347358

@@ -376,12 +387,8 @@ describe('CLI dispatch tests', function cliTests() {
376387

377388
it('should upload the passed file', mochaAsync(async () => {
378389
addCliStatusMessage();
379-
addExpectRequest('/v1/orgs/org/apps/app/releases/1.0.2/artifacts/', {
380-
status: 200,
381-
body: { signed_url: 'http://localhost:8818/upload/' },
382-
});
383390

384-
addExpectRequest('/upload/', { status: 200 });
391+
addArtifactRequest();
385392

386393
const result = await executeCommand(`upload -k org:app:secret -r 1.0.2 --apihost="http://localhost:8818" ${FIXTURE_PATH}subdir/one.js`);
387394

@@ -404,12 +411,9 @@ describe('CLI dispatch tests', function cliTests() {
404411

405412
it('should upload the passed files', mochaAsync(async () => {
406413
addCliStatusMessage();
407-
addExpectRequest('/v1/orgs/org/apps/app/releases/1.0.2/artifacts/', {
408-
status: 200,
409-
body: { signed_url: 'http://localhost:8818/upload/' },
410-
});
411414

412-
addExpectRequest('/upload/', { status: 200 });
415+
addArtifactRequest();
416+
addArtifactRequest();
413417

414418
const result = await executeCommand(`upload -k org:app:secret -r 1.0.2 --apihost="http://localhost:8818" ${FIXTURE_PATH}subdir/one.js ${FIXTURE_PATH}two.jsx`);
415419

@@ -455,4 +459,60 @@ describe('CLI dispatch tests', function cliTests() {
455459
expect(result.err.code).to.equal(1);
456460
expect(result.stderr).to.contain('Some error to show');
457461
}));
462+
463+
it('should retry failed uploads', mochaAsync(async () => {
464+
addCliStatusMessage();
465+
466+
addExpectRequest('/v1/orgs/org/apps/app/releases/1.0.2/artifacts/', {
467+
status: 200,
468+
body: { signed_url: 'http://localhost:8818/upload/' },
469+
});
470+
addExpectRequest('/upload/', { status: 429 });
471+
addExpectRequest('/upload/', { status: 500 });
472+
addExpectRequest('/upload/', { status: 502 });
473+
addExpectRequest('/upload/', { status: 503 });
474+
addExpectRequest('/upload/', { status: 504 });
475+
addExpectRequest('/upload/', { status: 200 });
476+
477+
const result = await executeCommand([
478+
'upload',
479+
'-k org:app:secret',
480+
'-r 1.0.2',
481+
'--apihost="http://localhost:8818"',
482+
'--max-retries 5',
483+
'--max-retry-delay 100',
484+
`${FIXTURE_PATH}subdir/one.js`,
485+
]);
486+
487+
expect(result.err).to.be.null();
488+
expect(result.stdout).to.contain('Found 1 file');
489+
expect(matchedRequests).to.have.length(8);
490+
expect(unmatchedRequests).to.have.length(0);
491+
}));
492+
493+
it('should stop retrying after the configured maximum', mochaAsync(async () => {
494+
addCliStatusMessage();
495+
496+
addExpectRequest('/v1/orgs/org/apps/app/releases/1.0.2/artifacts/', {
497+
status: 200,
498+
body: { signed_url: 'http://localhost:8818/upload/' },
499+
});
500+
addExpectRequest('/upload/', { status: 429 });
501+
addExpectRequest('/upload/', { status: 500 });
502+
503+
const result = await executeCommand([
504+
'upload',
505+
'-k org:app:secret',
506+
'-r 1.0.2',
507+
'--apihost="http://localhost:8818"',
508+
'--max-retries 1',
509+
'--max-retry-delay 100',
510+
`${FIXTURE_PATH}subdir/one.js`,
511+
]);
512+
513+
expect(result.err.message).to.contain('Failed to upload: one.js');
514+
expect(result.stdout).to.contain('Found 1 file');
515+
expect(matchedRequests).to.have.length(4);
516+
expect(unmatchedRequests).to.have.length(0);
517+
}));
458518
});

‎src/apiClient.js

+30-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,32 @@
11
import 'isomorphic-fetch';
22
import { version as cliVersion } from '../package.json';
33

4+
const delay = t => new Promise(resolve => setTimeout(resolve, t));
5+
6+
const RETRY_STATUS = [429, 500, 502, 503, 504];
7+
8+
async function retryableFetch(url, { maxRetries, maxRetryDelay, ...options }) {
9+
let result = await fetch(url, options);
10+
11+
if (result.ok || !RETRY_STATUS.includes(result.status)) {
12+
return result;
13+
}
14+
15+
for (let currentRetry = 0; currentRetry < maxRetries; currentRetry++) {
16+
const jitterMs = Math.round(Math.random() * 1000);
17+
const wait = Math.min(maxRetryDelay, Math.pow(2, currentRetry) * 1000) + jitterMs;
18+
await delay(wait);
19+
20+
result = await fetch(url, options);
21+
22+
if (result.ok || !RETRY_STATUS.includes(result.status)) {
23+
break;
24+
}
25+
}
26+
27+
return result;
28+
}
29+
430
class ApiClient {
531
constructor({
632
apikey,
@@ -62,7 +88,7 @@ class ApiClient {
6288
});
6389
}
6490

65-
async uploadFile({ release, filepath, contents }) {
91+
async uploadFile({ release, filepath, contents, maxRetries, maxRetryDelay }) {
6692
const res = await this._makeRequest({
6793
url: `releases/${release}/artifacts`,
6894
data: { filepath },
@@ -79,7 +105,9 @@ class ApiClient {
79105
throw new Error(`Could not get upload url for: ${filepath}`);
80106
}
81107

82-
const result = fetch(gcloudUrl, {
108+
const result = await retryableFetch(gcloudUrl, {
109+
maxRetries,
110+
maxRetryDelay,
83111
method: 'PUT',
84112
body: contents,
85113
});

‎src/commands/upload.js

+12
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,16 @@ export const builder = (args) => {
3535
'gcs-token': 'gcs-bucket',
3636
'gcs-bucket': 'gcs-token',
3737
})
38+
.option('max-retries', {
39+
type: 'number',
40+
describe: 'Failed upload retry limit (0 disables)',
41+
default: 0,
42+
})
43+
.option('max-retry-delay', {
44+
type: 'number',
45+
describe: 'Maximum delay between retries in ms',
46+
default: 30000,
47+
})
3848
.help('help');
3949
};
4050

@@ -95,6 +105,8 @@ export const handler = async (args) => {
95105
release,
96106
filepath,
97107
contents: createReadStream(path),
108+
maxRetries: args['max-retries'],
109+
maxRetryDelay: args['max-retry-delay'],
98110
};
99111

100112
try {

0 commit comments

Comments
 (0)