Skip to content

Commit 4f40de1

Browse files
authored
Restructure S3 response handling and pre-resolve credentials for stream uploads
1 parent 407c7c9 commit 4f40de1

2 files changed

Lines changed: 19 additions & 31 deletions

File tree

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

Lines changed: 15 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
type IGetFetchOptions,
1717
type IFetchOptionsWithBody,
1818
type IWebClientResponse,
19+
type IWebClientResponseBase,
1920
type IWebClientStreamResponse,
2021
type WebClient,
2122
AUTHORIZATION_HEADER_NAME
@@ -141,13 +142,7 @@ export class AmazonS3Client {
141142
this._writeDebugLine('Reading object from S3');
142143
return await this._sendCacheRequestWithRetriesAsync(async () => {
143144
const response: IWebClientResponse = await this._makeSignedRequestAsync('GET', objectName);
144-
return this._handleGetResponseAsync(
145-
response.status,
146-
response.statusText,
147-
response.ok,
148-
async () => await response.getBufferAsync(),
149-
async () => await this._getS3ErrorAsync(response)
150-
);
145+
return this._handleGetResponseAsync(response, async () => await response.getBufferAsync());
151146
});
152147
}
153148

@@ -191,22 +186,14 @@ export class AmazonS3Client {
191186
true
192187
);
193188
return this._handleGetResponseAsync<boolean>(
194-
response.status,
195-
response.statusText,
196-
response.ok,
189+
response,
197190
async () => {
198191
const writeStream: FileSystemWriteStream = await FileSystem.createWriteStreamAsync(localFilePath, {
199192
ensureFolderExists: true
200193
});
201194
await pipeline(response.stream, writeStream);
202195
return true;
203196
},
204-
async () => {
205-
response.stream.resume();
206-
return new Error(
207-
`Amazon S3 responded with status code ${response.status} (${response.statusText})`
208-
);
209-
},
210197
() => response.stream.resume()
211198
);
212199
});
@@ -263,35 +250,34 @@ export class AmazonS3Client {
263250

264251
/**
265252
* Shared response handling for GET requests (both buffer and stream).
266-
* The `getSuccessResult` callback extracts the response payload (Buffer or Readable).
267-
* The `getError` callback constructs an error from the response.
253+
* The `getSuccessResult` callback extracts the response payload (Buffer or stream-to-file result).
268254
* The optional `cleanup` callback drains stream responses on non-success paths.
269255
*/
270256
private async _handleGetResponseAsync<T>(
271-
status: number,
272-
statusText: string | undefined,
273-
ok: boolean,
257+
response: IWebClientResponseBase,
274258
getSuccessResult: () => T | Promise<T>,
275-
getError: () => Promise<Error>,
276259
cleanup?: () => void
277260
): Promise<RetryableRequestResponse<T | undefined>> {
278-
if (ok) {
261+
if (response.ok) {
279262
return {
280263
hasNetworkError: false,
281264
response: await getSuccessResult()
282265
};
283-
} else if (status === 404) {
266+
} else if (response.status === 404) {
284267
cleanup?.();
285268
return {
286269
hasNetworkError: false,
287270
response: undefined
288271
};
289-
} else if ((status === 400 || status === 401 || status === 403) && !this._credentials) {
272+
} else if (
273+
(response.status === 400 || response.status === 401 || response.status === 403) &&
274+
!this._credentials
275+
) {
290276
cleanup?.();
291277
// unauthorized due to not providing credentials,
292278
// silence error for better DX when e.g. running locally without credentials
293279
this._writeWarningLine(
294-
`No credentials found and received a ${status}`,
280+
`No credentials found and received a ${response.status}`,
295281
' response code from the cloud storage.',
296282
' Maybe run rush update-cloud-credentials',
297283
' or set the RUSH_BUILD_CACHE_CREDENTIAL env'
@@ -300,15 +286,14 @@ export class AmazonS3Client {
300286
hasNetworkError: false,
301287
response: undefined
302288
};
303-
} else if (status === 400 || status === 401 || status === 403) {
289+
} else if (response.status === 400 || response.status === 401 || response.status === 403) {
304290
cleanup?.();
305-
throw await getError();
291+
throw new Error(`Amazon S3 responded with status code ${response.status} (${response.statusText})`);
306292
} else {
307293
cleanup?.();
308-
const error: Error = await getError();
309294
return {
310295
hasNetworkError: true,
311-
error
296+
error: new Error(`Amazon S3 responded with status code ${response.status} (${response.statusText})`)
312297
};
313298
}
314299
}

rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,10 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider {
212212
// maxAttempts is 1 because the file read stream is consumed after the first attempt
213213
// and cannot be replayed. Downloads use MAX_HTTP_CACHE_ATTEMPTS since each retry
214214
// issues a fresh GET with no request body.
215-
maxAttempts: 1
215+
maxAttempts: 1,
216+
// Pre-resolve credentials for stream uploads because the credential fallback path
217+
// in _makeHttpCoreRequestAsync cannot replay a consumed stream body.
218+
credentialOptions: CredentialsOptions.Required
216219
});
217220

218221
if (result !== false) {

0 commit comments

Comments
 (0)