8000 Merge pull request #3248 from ricsam/retry-cloud-cache-requests · psy-repos-typescript/rushstack@3d660bc · GitHub
[go: up one dir, main page]
Skip to content

Commit 3d660bc

Browse files
iclantoniclanton
authored
Merge pull request microsoft#3248 from ricsam/retry-cloud-cache-requests
[rush-amazon-s3-build-cache-plugin] retry failed S3 requests with the same behavior as the the azure blob storage
2 parents 88434d0 + 9f5457a commit 3d660bc

File tree

11 files changed

+565
-70
lines changed

11 files changed

+565
-70
lines changed

‎build-tests/rush-amazon-s3-build-cache-plugin-integration-test/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
s3data/.minio.sys
2+
s3data/rush-build-cache/test

‎build-tests/rush-amazon-s3-build-cache-plugin-integration-test/README.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,32 @@ In this folder run `docker-compose down`
1616
# build the code: rushx build
1717
rushx read-s3-object
1818
```
19+
20+
# Testing retries
21+
22+
To test that requests can be retried start the proxy server which will fail every second request:
23+
24+
```bash
25+
rushx start-proxy-server
26+
```
27+
28+
Update the build-cache.json file:
29+
```json
30+
{
31+
"cacheProvider": "amazon-s3",
32+
"amazonS3Configuration": {
33+
"s3Endpoint": "http://localhost:9002",
34+
"s3Region": "us-east-1",
35+
"s3Prefix": "rush-build-cache/test",
36+
"isCacheWriteAllowed": true
37+
}
38+
}
39+
```
40+
41+
Run the rush rebuild command
42+
43+
```bash
44+
cd apps
45+
cd rush
46+
RUSH_BUILD_CACHE_CREDENTIAL="minio:minio123" node lib/start-dev.js --debug rebuild --verbose
47+
```

‎build-tests/rush-amazon-s3-build-cache-plugin-integration-test/package.json

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
"scripts": {
88
"build": "heft build --clean",
99
"_phase:build": "heft build --clean",
10-
"read-s3-object": "node ./lib/readObject.js"
10+
"read-s3-object": "node ./lib/readObject.js",
11+
"start-proxy-server": "node ./lib/startProxyServer.js"
1112
},
1213
"devDependencies": {
1314
"@rushstack/eslint-config": "workspace:*",
@@ -16,6 +17,8 @@
1617
"@rushstack/node-core-library": "workspace:*",
1718
"@types/node": "12.20.24",
1819
"eslint": "~8.7.0",
19-
"typescript": "~4.5.2"
20+
"typescript": "~4.5.2",
21+
"http-proxy": "~1.18.1",
22+
"@types/http-proxy": "~1.17.8"
2023
}
2124
}

‎build-tests/rush-amazon-s3-build-cache-plugin-integration-test/src/startProxyServer.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import * as httpProxy from 'http-proxy';
2+
import * as http from 'http';
3+
4+
const proxy: httpProxy = httpProxy.createProxyServer({});
5+
6+
const hasFailed: { [k: string]: boolean } = {};
7+
8+
let requestCount: number = 0;
9+
const server: http.Server = http.createServer((req, res) => {
10+
requestCount += 1;
11+
12+
if (req.url && requestCount % 2 === 0 && !hasFailed[req.url]) {
13+
console.log('failing', req.url);
14+
hasFailed[req.url] = true;
15+
res.statusCode = 500;
16+
res.end();
17+
return;
18+
} else if (req.url) {
19+
console.log('proxying', req.url);
20+
}
21+
22+
proxy.web(req, res, {
23+
target: 'http://127.0.0.1:9000'
24+
});
25+
});
26+
27+
server.listen(9002);

‎common/changes/@microsoft/rush/retry-cloud-cache-requests_2022-03-10-22-16.json

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@microsoft/rush",
5+
"comment": "Improve retry logic in the Amazon S3 cloud cache plugin and improve reporting when the user is not authenticated.",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@microsoft/rush"
10+
}

‎common/config/rush/nonbrowser-approved-packages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,10 @@
442442
"name": "html-webpack-plugin",
443443
"allowedCategories": [ "libraries", "tests" ]
444444
},
445+
{
446+
"name": "http-proxy",
447+
"allowedCategories": [ "tests" ]
448+
},
445449
{
446450
"name": "https-proxy-agent",
447451
"allowedCategories": [ "libraries" ]

‎common/config/rush/pnpm-lock.yaml

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎rush-plugins/rush-amazon-s3-build-cache-plugin/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
"scripts": {
1414
"build": "heft build --clean",
1515
"start": "heft test --clean --watch",
16+
"test": "heft test",
1617
"_phase:build": "heft build --clean",
1718
"_phase:test": "heft test --no-build"
1819
},

‎rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts

Lines changed: 140 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
22
// See LICENSE in the project root for license information.
33

4-
import { Colors, IColorableSequence, ITerminal } from '@rushstack/node-core-library';
4+
import { Async, Colors, IColorableSequence, ITerminal } from '@rushstack/node-core-library';
55
import * as crypto from 'crypto';
66
import * as fetch from 'node-fetch';
77

@@ -29,9 +29,40 @@ interface IIsoDateString {
2929
dateTime: string;
3030
}
3131

32+
type RetryableRequestResponse<T> =
33+
| {
34+
hasNetworkError: true;
35+
error: Error;
36+
}
37+
| {
38+
hasNetworkError: false;
39+
response: T;
40+
};
41+
3242
const protocolRegex: RegExp = /^https?:\/\//;
3343
const portRegex: RegExp = /:(\d{1,5})$/;
3444

45+
// Similar to https://docs.microsoft.com/en-us/javascript/api/@azure/storage-blob/storageretrypolicytype?view=azure-node-latest
46+
enum StorageRetryPolicyType {
47+
EXPONENTIAL = 0,
48+
FIXED = 1
49+
}
50+
51+
// Similar to https://docs.microsoft.com/en-us/javascript/api/@azure/storage-blob/storageretryoptions?view=azure-node-latest
52+
interface IStorageRetryOptions {
53+
maxRetryDelayInMs: number;
54+
maxTries: number;
55+
retryDelayInMs: number;
56+
retryPolicyType: StorageRetryPolicyType;
57+
}
58+
59+
const storageRetryOptions: IStorageRetryOptions = {
60+
maxRetryDelayInMs: 120 * 1000,
61+
maxTries: 4,
62+
retryDelayInMs: 4 * 1000,
63+
retryPolicyType: StorageRetryPolicyType.EXPONENTIAL
64+
};
65+
3566
/**
3667
* A helper for reading and updating objects on Amazon S3
3768
*
@@ -103,35 +134,81 @@ export class AmazonS3Client {
103134

104135
public async getObjectAsync(objectName: string): Promise<Buffer | undefined> {
105136
this._writeDebugLine('Reading object from S3');
106-
const response: fetch.Response = await this._makeRequestAsync('GET', objectName);
107-
if (response.ok) {
108-
return await response.buffer();
109-
} else if (response.status === 404) {
110-
return undefined;
111-
} else if (response.status === 403 && !this._credentials) {
112-
return undefined;
113-
} else {
114-
this._throwS3Error(response, await this._safeReadResponseText(response));
115-
}
137+
return await this._sendCacheRequestWithRetries(async () => {
138+
const response: fetch.Response = await this._makeRequestAsync('GET', objectName);
139+
if (response.ok) {
140+
return {
141+
hasNetworkError: false,
142+
response: await response.buffer()
143+
};
144+
} else if (response.status === 404) {
145+
return {
146+
hasNetworkError: false,
147+
response: undefined
148+
};
149+
} else if (
150+
(response.status === 400 || response.status === 401 || response.status === 403) &&
151+
!this._credentials
152+
) {
153+
// unauthorized due to not providing credentials,
154+
// silence error for better DX when e.g. running locally without credentials
155+
this._writeWarningLine(
156+
`No credentials found and received a ${response.status}`,
157+
' response code from the cloud storage.',
158+
' Maybe run rush update-cloud-credentials',
159+
' or set the RUSH_BUILD_CACHE_CREDENTIAL env'
160+
);
161+
return {
162+
hasNetworkError: false,
163+
response: undefined
164+
};
165+
} else if (response.status === 400 || response.status === 401 || response.status === 403) {
166+
throw await this._getS3ErrorAsync(response);
167+
} else {
168+
const error: Error = await this._getS3ErrorAsync(response);
169+
return {
170+
hasNetworkError: true,
171+
error
172+
};
173+
}
174+
});
116175
}
117176

118177
public async uploadObjectAsync(objectName: string, objectBuffer: Buffer): Promise<void> {
119178
if (!this._credentials) {
120179
throw new Error('Credentials are required to upload objects to S3.');
121180
}
122181

123-
const response: fetch.Response = await this._makeRequestAsync('PUT', objectName, objectBuffer);
124-
if (!response.ok) {
125-
this._throwS3Error(response, await this._safeReadResponseText(response));
126-
}
182+
await this._sendCacheRequestWithRetries(async () => {
183+
const response: fetch.Response = await this._makeRequestAsync('PUT', objectName, objectBuffer);
184+
if (!response.ok) {
185+
return {
186+
hasNetworkError: true,
187+
error: await this._getS3ErrorAsync(response)
188+
};
189+
}
190+
return {
191+
hasNetworkError: false,
192+
response: undefined
193+
};
194+
});
127195
}
128196

129197
private _writeDebugLine(...messageParts: (string | IColorableSequence)[]): void {
130198
// if the terminal has been closed then don't bother sending a debug message
131199
try {
132200
this._terminal.writeDebugLine(...messageParts);
133201
} catch (err) {
134-
//
202+
// ignore error
203+
}
204+
}
205+
206+
private _writeWarningLine(...messageParts: (string | IColorableSequence)[]): void {
207+
// if the terminal has been closed then don't bother sending a warning message
208+
try {
209+
this._terminal.writeWarningLine(...messageParts);
210+
} catch (err) {
211+
// ignore error
135212
}
136213
}
137214

@@ -305,8 +382,9 @@ export class AmazonS3Client {
305382
return undefined;
306383
}
307384

308-
private _throwS3Error(response: fetch.Response, text: string | undefined): never {
309-
throw new Error(
385+
private async _getS3ErrorAsync(response: fetch.Response): Promise<Error> {
386+
const text: string | undefined = await this._safeReadResponseText(response);
387+
return new Error(
310388
`Amazon S3 responded with status code ${response.status} (${response.statusText})${
311389
text ? `\n${text}` : ''
312390
}`
@@ -371,4 +449,48 @@ export class AmazonS3Client {
371449
);
372450
}
373451
}
452+
453+
private async _sendCacheRequestWithRetries<T>(
454+
sendRequest: () => Promise<RetryableRequestResponse<T>>
455+
): Promise<T> {
456+
const response: RetryableRequestResponse<T> = await sendRequest();
457+
458+
const log: (...messageParts: (string | IColorableSequence)[]) => void = this._writeDebugLine.bind(this);
459+
460+
if (response.hasNetworkError) {
461+
if (storageRetryOptions && storageRetryOptions.maxTries > 1) {
462+
log('Network request failed. Will retry request as specified in storageRetryOptions');
463+
async function retry(retryAttempt: number): Promise<T> {
464+
const { retryDelayInMs, retryPolicyType, maxTries, maxRetryDelayInMs } = storageRetryOptions;
465+
let delay: number = retryDelayInMs;
466+
if (retryPolicyType === StorageRetryPolicyType.EXPONENTIAL) {
467+
delay = retryDelayInMs * Math.pow(2, retryAttempt - 1);
468+
}
469+
delay = Math.min(maxRetryDelayInMs, delay);
470+
471+
log(`Will retry request in ${delay}s...`);
472+
await Async.sleep(delay);
473+
const response: RetryableRequestResponse<T> = await sendRequest();
474+
475+
if (response.hasNetworkError) {
476+
if (retryAttempt < maxTries - 1) {
477+
log('The retried request failed, will try again');
478+
return retry(retryAttempt + 1);
479+
} else {
480+
log('The retried request failed and has reached the maxTries limit');
481+
throw response.error;
482+
}
483+
}
484+
485+
return response.response;
486+
}
487+
return retry(1);
488+
} else {
489+
log('Network request failed and storageRetryOptions is not specified');
490+
throw response.error;
491+
}
492+
}
493+
494+
return response.response;
495+
}
374496
}

0 commit comments

Comments
 (0)
0