From 66b650d3f82d05211a6bf567926e9bcf691fecbc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 5 Apr 2026 03:35:42 +0000 Subject: [PATCH 01/45] feat: add streaming support to build cache provider interface and HTTP plugin Add optional tryGetCacheEntryStreamByIdAsync and trySetCacheEntryStreamAsync methods to ICloudBuildCacheProvider. Implement streaming in HttpBuildCacheProvider and update OperationBuildCache to prefer streaming when available. Add fetchStreamAsync to WebClient and stream write support to FileSystemBuildCacheProvider. Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/050e10a7-3cad-4da4-93e5-9941453283b9 Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- .../FileSystemBuildCacheProvider.ts | 19 +++ .../buildCache/ICloudBuildCacheProvider.ts | 19 +++ .../logic/buildCache/OperationBuildCache.ts | 76 ++++++--- libraries/rush-lib/src/utilities/WebClient.ts | 147 ++++++++++++++++-- .../src/HttpBuildCacheProvider.ts | 138 +++++++++++++++- 5 files changed, 361 insertions(+), 38 deletions(-) diff --git a/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts b/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts index 8fdd54bf444..eed1bca7bf8 100644 --- a/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts +++ b/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts @@ -2,6 +2,9 @@ // See LICENSE in the project root for license information. import * as path from 'node:path'; +import { createWriteStream } from 'node:fs'; +import { pipeline } from 'node:stream/promises'; +import type { Readable } from 'node:stream'; import { FileSystem } from '@rushstack/node-core-library'; import type { ITerminal } from '@rushstack/terminal'; @@ -75,4 +78,20 @@ export class FileSystemBuildCacheProvider { terminal.writeVerboseLine(`Wrote cache entry to "${cacheEntryFilePath}".`); return cacheEntryFilePath; } + + /** + * Writes the specified stream to the corresponding file system path for the cache id. + * This avoids loading the entire cache entry into memory. + */ + public async trySetCacheEntryStreamAsync( + terminal: ITerminal, + cacheId: string, + entryStream: Readable + ): Promise { + const cacheEntryFilePath: string = this.getCacheEntryPath(cacheId); + await FileSystem.ensureFolderAsync(path.dirname(cacheEntryFilePath)); + await pipeline(entryStream, createWriteStream(cacheEntryFilePath)); + terminal.writeVerboseLine(`Wrote cache entry to "${cacheEntryFilePath}".`); + return cacheEntryFilePath; + } } diff --git a/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts b/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts index f55a0870ad8..1f1543a4d9f 100644 --- a/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts +++ b/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. +import type { Readable } from 'node:stream'; import type { ITerminal } from '@rushstack/terminal'; /** @@ -11,6 +12,24 @@ export interface ICloudBuildCacheProvider { tryGetCacheEntryBufferByIdAsync(terminal: ITerminal, cacheId: string): Promise; trySetCacheEntryBufferAsync(terminal: ITerminal, cacheId: string, entryBuffer: Buffer): Promise; + + /** + * If implemented, the build cache will prefer to use this method over + * {@link ICloudBuildCacheProvider.tryGetCacheEntryBufferByIdAsync} to avoid loading the entire + * cache entry into memory. + */ + tryGetCacheEntryStreamByIdAsync?(terminal: ITerminal, cacheId: string): Promise; + /** + * If implemented, the build cache will prefer to use this method over + * {@link ICloudBuildCacheProvider.trySetCacheEntryBufferAsync} to avoid loading the entire + * cache entry into memory. + */ + trySetCacheEntryStreamAsync?( + terminal: ITerminal, + cacheId: string, + entryStream: Readable + ): Promise; + updateCachedCredentialAsync(terminal: ITerminal, credential: string): Promise; updateCachedCredentialInteractiveAsync(terminal: ITerminal): Promise; deleteCachedCredentialsAsync(terminal: ITerminal): Promise; diff --git a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts index 0abf76c221b..9ac2462b092 100644 --- a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts +++ b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts @@ -3,6 +3,7 @@ import * as path from 'node:path'; import * as crypto from 'node:crypto'; +import { createReadStream } from 'node:fs'; import { FileSystem, type FolderItem, InternalError, Async } from '@rushstack/node-core-library'; import type { ITerminal } from '@rushstack/terminal'; @@ -159,20 +160,40 @@ export class OperationBuildCache { 'This project was not found in the local build cache. Querying the cloud build cache.' ); - cacheEntryBuffer = await this._cloudBuildCacheProvider.tryGetCacheEntryBufferByIdAsync( - terminal, - cacheId - ); - if (cacheEntryBuffer) { - try { - localCacheEntryPath = await this._localBuildCacheProvider.trySetCacheEntryBufferAsync( - terminal, - cacheId, - cacheEntryBuffer - ); - updateLocalCacheSuccess = true; - } catch (e) { - updateLocalCacheSuccess = false; + if (this._cloudBuildCacheProvider.tryGetCacheEntryStreamByIdAsync) { + // Use streaming path to avoid loading the entire cache entry into memory + const cacheEntryStream = await this._cloudBuildCacheProvider.tryGetCacheEntryStreamByIdAsync( + terminal, + cacheId + ); + if (cacheEntryStream) { + try { + localCacheEntryPath = await this._localBuildCacheProvider.trySetCacheEntryStreamAsync( + terminal, + cacheId, + cacheEntryStream + ); + updateLocalCacheSuccess = true; + } catch (e) { + updateLocalCacheSuccess = false; + } + } + } else { + cacheEntryBuffer = await this._cloudBuildCacheProvider.tryGetCacheEntryBufferByIdAsync( + terminal, + cacheId + ); + if (cacheEntryBuffer) { + try { + localCacheEntryPath = await this._localBuildCacheProvider.trySetCacheEntryBufferAsync( + terminal, + cacheId, + cacheEntryBuffer + ); + updateLocalCacheSuccess = true; + } catch (e) { + updateLocalCacheSuccess = false; + } } } } @@ -300,8 +321,6 @@ export class OperationBuildCache { return false; } - let cacheEntryBuffer: Buffer | undefined; - let setCloudCacheEntryPromise: Promise | undefined; // Note that "writeAllowed" settings (whether in config or environment) always apply to @@ -309,17 +328,26 @@ export class OperationBuildCache { // write to the local build cache. if (this._cloudBuildCacheProvider?.isCacheWriteAllowed) { - if (localCacheEntryPath) { - cacheEntryBuffer = await FileSystem.readFileToBufferAsync(localCacheEntryPath); - } else { + if (!localCacheEntryPath) { throw new InternalError('Expected the local cache entry path to be set.'); } - setCloudCacheEntryPromise = this._cloudBuildCacheProvider?.trySetCacheEntryBufferAsync( - terminal, - cacheId, - cacheEntryBuffer - ); + if (this._cloudBuildCacheProvider.trySetCacheEntryStreamAsync) { + // Use streaming upload to avoid loading the entire cache entry into memory + const entryStream = createReadStream(localCacheEntryPath); + setCloudCacheEntryPromise = this._cloudBuildCacheProvider.trySetCacheEntryStreamAsync( + terminal, + cacheId, + entryStream + ); + } else { + const cacheEntryBuffer: Buffer = await FileSystem.readFileToBufferAsync(localCacheEntryPath); + setCloudCacheEntryPromise = this._cloudBuildCacheProvider.trySetCacheEntryBufferAsync( + terminal, + cacheId, + cacheEntryBuffer + ); + } } const updateCloudCacheSuccess: boolean | undefined = (await setCloudCacheEntryPromise) ?? true; diff --git a/libraries/rush-lib/src/utilities/WebClient.ts b/libraries/rush-lib/src/utilities/WebClient.ts index bdd16823332..3ea6c66e318 100644 --- a/libraries/rush-lib/src/utilities/WebClient.ts +++ b/libraries/rush-lib/src/utilities/WebClient.ts @@ -3,7 +3,13 @@ import * as os from 'node:os'; import * as process from 'node:process'; -import { request as httpRequest, type IncomingMessage, type Agent as HttpAgent } from 'node:http'; +import type { Readable } from 'node:stream'; +import { + request as httpRequest, + type IncomingMessage, + type ClientRequest, + type Agent as HttpAgent +} from 'node:http'; import { request as httpsRequest, type RequestOptions } from 'node:https'; import { Import, LegacyAdapters } from '@rushstack/node-core-library'; @@ -24,6 +30,19 @@ export interface IWebClientResponse { getBufferAsync: () => Promise; } +/** + * A response from {@link WebClient.fetchStreamAsync} that provides the response body as a + * readable stream, avoiding buffering the entire response in memory. + */ +export interface IWebClientStreamResponse { + ok: boolean; + status: number; + statusText?: string; + redirected: boolean; + headers: Record; + stream: Readable; +} + /** * For use with {@link WebClient}. */ @@ -49,7 +68,7 @@ export interface IGetFetchOptions extends IWebFetchOptionsBase { */ export interface IFetchOptionsWithBody extends IWebFetchOptionsBase { verb: 'PUT' | 'POST' | 'PATCH'; - body?: Buffer; + body?: Buffer | Readable; } /** @@ -91,7 +110,7 @@ const makeRequestAsync: FetchFn = async ( const requestFunction: typeof httpRequest | typeof httpsRequest = parsedUrl.protocol === 'https:' ? httpsRequest : httpRequest; - requestFunction(url, options, (response: IncomingMessage) => { + const req: ClientRequest = requestFunction(url, options, (response: IncomingMessage) => { const responseBuffers: (Buffer | Uint8Array)[] = []; response.on('data', (chunk: string | Buffer | Uint8Array) => { responseBuffers.push(Buffer.from(chunk)); @@ -197,20 +216,103 @@ const makeRequestAsync: FetchFn = async ( }; resolve(result); }); - }) - .on('error', (error: Error) => { - reject(error); - }) - .end(body); + }).on('error', (error: Error) => { + reject(error); + }); + + _sendRequestBody(req, body, reject); } ); }; +type StreamFetchFn = ( + url: string, + options: IRequestOptions, + isRedirect?: boolean +) => Promise; + +/** + * Makes an HTTP request that resolves as soon as headers are received, providing the + * response body as a readable stream. This avoids buffering the entire response in memory. + */ +const makeStreamRequestAsync: StreamFetchFn = async ( + url: string, + options: IRequestOptions, + redirected: boolean = false +) => { + const { body, redirect } = options; + + return await new Promise( + (resolve: (result: IWebClientStreamResponse) => void, reject: (error: Error) => void) => { + const parsedUrl: URL = typeof url === 'string' ? new URL(url) : url; + const requestFunction: typeof httpRequest | typeof httpsRequest = + parsedUrl.protocol === 'https:' ? httpsRequest : httpRequest; + + const req: ClientRequest = requestFunction(url, options, (response: IncomingMessage) => { + const statusCode: number | undefined = response.statusCode; + + // Handle redirects + if (statusCode === 301 || statusCode === 302) { + // Consume/drain the redirect response before following + response.resume(); + switch (redirect) { + case 'follow': { + const redirectUrl: string | string[] | undefined = response.headers.location; + if (redirectUrl) { + makeStreamRequestAsync(redirectUrl, options, true).then(resolve).catch(reject); + } else { + reject( + new Error(`Received status code ${response.statusCode} with no location header: ${url}`) + ); + } + return; + } + case 'error': + reject(new Error(`Received status code ${response.statusCode}: ${url}`)); + return; + } + } + + const status: number = response.statusCode || 0; + const statusText: string | undefined = response.statusMessage; + const headers: Record = response.headers; + + resolve({ + ok: status >= 200 && status < 300, + status, + statusText, + redirected, + headers, + stream: response + }); + }).on('error', (error: Error) => { + reject(error); + }); + + _sendRequestBody(req, body, reject); + } + ); +}; + +function _sendRequestBody( + req: ClientRequest, + body: Buffer | Readable | undefined, + reject: (error: Error) => void +): void { + if (body && typeof (body as Readable).pipe === 'function') { + (body as Readable).on('error', reject); + (body as Readable).pipe(req); + } else { + req.end(body as Buffer | undefined); + } +} + /** * A helper for issuing HTTP requests. */ export class WebClient { private static _requestFn: FetchFn = makeRequestAsync; + private static _streamRequestFn: StreamFetchFn = makeStreamRequestAsync; public readonly standardHeaders: Record = {}; @@ -227,6 +329,14 @@ export class WebClient { WebClient._requestFn = makeRequestAsync; } + public static mockStreamRequestFn(fn: StreamFetchFn): void { + WebClient._streamRequestFn = fn; + } + + public static resetMockStreamRequestFn(): void { + WebClient._streamRequestFn = makeStreamRequestAsync; + } + public static mergeHeaders(target: Record, source: Record): void { for (const [name, value] of Object.entries(source)) { target[name] = value; @@ -242,6 +352,23 @@ export class WebClient { url: string, options?: IGetFetchOptions | IFetchOptionsWithBody ): Promise { + const requestInit: IRequestOptions = this._buildRequestOptions(options); + return await WebClient._requestFn(url, requestInit); + } + + /** + * Makes an HTTP request that resolves as soon as headers are received, providing the + * response body as a readable stream. This avoids buffering the entire response in memory. + */ + public async fetchStreamAsync( + url: string, + options?: IGetFetchOptions | IFetchOptionsWithBody + ): Promise { + const requestInit: IRequestOptions = this._buildRequestOptions(options); + return await WebClient._streamRequestFn(url, requestInit); + } + + private _buildRequestOptions(options?: IGetFetchOptions | IFetchOptionsWithBody): IRequestOptions { const { headers: optionsHeaders, timeoutMs = 15 * 1000, @@ -291,7 +418,7 @@ export class WebClient { agent = createHttpsProxyAgent(proxyUrl); } - const requestInit: IRequestOptions = { + return { method: verb, headers, agent, @@ -300,7 +427,5 @@ export class WebClient { body, noDecode }; - - return await WebClient._requestFn(url, requestInit); } } diff --git a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts index 0ff34473e7e..c915fd56c16 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts @@ -2,6 +2,7 @@ // See LICENSE in the project root for license information. import type { SpawnSyncReturns } from 'node:child_process'; +import type { Readable } from 'node:stream'; import { type ICredentialCacheEntry, CredentialCache } from '@rushstack/credential-cache'; import { Executable, Async } from '@rushstack/node-core-library'; @@ -11,7 +12,11 @@ import { type RushSession, EnvironmentConfiguration } from '@rushstack/rush-sdk'; -import { WebClient, type IWebClientResponse } from '@rushstack/rush-sdk/lib/utilities/WebClient'; +import { + WebClient, + type IWebClientResponse, + type IWebClientStreamResponse +} from '@rushstack/rush-sdk/lib/utilities/WebClient'; enum CredentialsOptions { Optional, @@ -150,6 +155,61 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { } } + public async tryGetCacheEntryStreamByIdAsync( + terminal: ITerminal, + cacheId: string + ): Promise { + try { + const result: IWebClientStreamResponse | false = await this._makeHttpStreamRequestAsync({ + terminal: terminal, + relUrl: `${this._cacheKeyPrefix}${cacheId}`, + method: 'GET', + body: undefined, + warningText: 'Could not get cache entry', + maxAttempts: MAX_HTTP_CACHE_ATTEMPTS + }); + + return result !== false ? result.stream : undefined; + } catch (e) { + terminal.writeWarningLine(`Error getting cache entry: ${e}`); + return undefined; + } + } + + public async trySetCacheEntryStreamAsync( + terminal: ITerminal, + cacheId: string, + entryStream: Readable + ): Promise { + if (!this.isCacheWriteAllowed) { + terminal.writeErrorLine('Writing to cache is not allowed in the current configuration.'); + return false; + } + + terminal.writeDebugLine('Uploading object with cacheId: ', cacheId); + + try { + const result: IWebClientStreamResponse | false = await this._makeHttpStreamRequestAsync({ + terminal: terminal, + relUrl: `${this._cacheKeyPrefix}${cacheId}`, + method: this._uploadMethod, + body: entryStream, + warningText: 'Could not write cache entry', + // Streaming uploads cannot be retried because the stream is consumed + maxAttempts: 1 + }); + + if (result !== false) { + // Drain the response body + result.stream.resume(); + } + return result !== false; + } catch (e) { + terminal.writeWarningLine(`Error uploading cache entry: ${e}`); + return false; + } + } + public async updateCachedCredentialAsync(terminal: ITerminal, credential: string): Promise { await CredentialCache.usingAsync( { @@ -309,6 +369,78 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { return result; } + private async _makeHttpStreamRequestAsync(options: { + terminal: ITerminal; + relUrl: string; + method: 'GET' | UploadMethod; + body: Readable | undefined; + warningText: string; + maxAttempts: number; + credentialOptions?: CredentialsOptions; + }): Promise { + const { terminal, relUrl, method, body, warningText, credentialOptions } = options; + const safeCredentialOptions: CredentialsOptions = credentialOptions ?? CredentialsOptions.Optional; + const credentials: string | undefined = await this._tryGetCredentialsAsync(safeCredentialOptions); + const url: string = new URL(relUrl, this._url).href; + + const headers: Record = {}; + if (typeof credentials === 'string') { + headers.Authorization = credentials; + } + + for (const [key, value] of Object.entries(this._headers)) { + if (typeof value === 'string') { + headers[key] = value; + } + } + + terminal.writeDebugLine(`[http-build-cache] stream request: ${method} ${url}`); + + const webClient: WebClient = new WebClient(); + const response: IWebClientStreamResponse = await webClient.fetchStreamAsync(url, { + verb: method, + headers: headers, + body: body, + redirect: 'follow', + timeoutMs: 0 // Use the default timeout + }); + + if (!response.ok) { + // Drain the response body so the connection can be reused + response.stream.resume(); + + const isNonCredentialResponse: boolean = response.status >= 500 && response.status < 600; + + if ( + !isNonCredentialResponse && + typeof credentials !== 'string' && + safeCredentialOptions === CredentialsOptions.Optional + ) { + return await this._makeHttpStreamRequestAsync({ + ...options, + credentialOptions: CredentialsOptions.Required + }); + } + + if (options.maxAttempts > 1) { + const factor: number = 1.0 + Math.random(); + const retryDelay: number = Math.floor(factor * this._minHttpRetryDelayMs); + await Async.sleepAsync(retryDelay); + return await this._makeHttpStreamRequestAsync({ + ...options, + maxAttempts: options.maxAttempts - 1 + }); + } + + this._reportFailure(terminal, method, response, false, warningText); + return false; + } + + terminal.writeDebugLine(`[http-build-cache] stream response: ${response.status} ${url}`); + + return response; + } + private async _tryGetCredentialsAsync(options: CredentialsOptions.Required): Promise; private async _tryGetCredentialsAsync(options: CredentialsOptions.Optional): Promise; private async _tryGetCredentialsAsync(options: CredentialsOptions.Omit): Promise; @@ -363,7 +495,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { private _getFailureType( requestMethod: string, - response: IWebClientResponse, + response: IWebClientResponse | IWebClientStreamResponse, isRedirect: boolean ): FailureType { if (response.ok) { @@ -415,7 +547,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { private _reportFailure( terminal: ITerminal, requestMethod: string, - response: IWebClientResponse, + response: IWebClientResponse | IWebClientStreamResponse, isRedirect: boolean, message: string ): void { From 64d05f19b07aea36463cc79f03f5a172b7ccaefd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 5 Apr 2026 03:43:15 +0000 Subject: [PATCH 02/45] fix lint warnings, add change files and API report update Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/050e10a7-3cad-4da4-93e5-9941453283b9 Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- .../rush/stream-build-cache_2026-04-05-03-25.json | 10 ++++++++++ .../stream-build-cache_2026-04-05-03-25.json | 10 ++++++++++ common/reviews/api/rush-lib.api.md | 4 ++++ .../src/logic/buildCache/ICloudBuildCacheProvider.ts | 1 + .../src/logic/buildCache/OperationBuildCache.ts | 9 ++++----- 5 files changed, 29 insertions(+), 5 deletions(-) create mode 100644 common/changes/@microsoft/rush/stream-build-cache_2026-04-05-03-25.json create mode 100644 common/changes/@rushstack/rush-http-build-cache-plugin/stream-build-cache_2026-04-05-03-25.json diff --git a/common/changes/@microsoft/rush/stream-build-cache_2026-04-05-03-25.json b/common/changes/@microsoft/rush/stream-build-cache_2026-04-05-03-25.json new file mode 100644 index 00000000000..d26042ecc84 --- /dev/null +++ b/common/changes/@microsoft/rush/stream-build-cache_2026-04-05-03-25.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Add optional streaming APIs (tryGetCacheEntryStreamByIdAsync, trySetCacheEntryStreamAsync) to ICloudBuildCacheProvider and FileSystemBuildCacheProvider, allowing cache plugins to transfer entries without buffering the entire contents in memory.", + "type": "none" + } + ], + "packageName": "@microsoft/rush" +} diff --git a/common/changes/@rushstack/rush-http-build-cache-plugin/stream-build-cache_2026-04-05-03-25.json b/common/changes/@rushstack/rush-http-build-cache-plugin/stream-build-cache_2026-04-05-03-25.json new file mode 100644 index 00000000000..e9f01dc3adb --- /dev/null +++ b/common/changes/@rushstack/rush-http-build-cache-plugin/stream-build-cache_2026-04-05-03-25.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/rush-http-build-cache-plugin", + "comment": "Implement streaming cache entry download and upload to avoid loading entire cache entries into memory, preventing out-of-memory crashes with large build outputs.", + "type": "minor" + } + ], + "packageName": "@rushstack/rush-http-build-cache-plugin" +} diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index dbbb440e40b..01c299eed19 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -28,6 +28,7 @@ import { JsonObject } from '@rushstack/node-core-library'; import { LookupByPath } from '@rushstack/lookup-by-path'; import { PackageNameParser } from '@rushstack/node-core-library'; import type { PerformanceEntry as PerformanceEntry_2 } from 'node:perf_hooks'; +import type { Readable } from 'node:stream'; import type { StdioSummarizer } from '@rushstack/terminal'; import { SyncHook } from 'tapable'; import { SyncWaterfallHook } from 'tapable'; @@ -315,6 +316,7 @@ export class FileSystemBuildCacheProvider { getCacheEntryPath(cacheId: string): string; tryGetCacheEntryPathByIdAsync(terminal: ITerminal, cacheId: string): Promise; trySetCacheEntryBufferAsync(terminal: ITerminal, cacheId: string, entryBuffer: Buffer): Promise; + trySetCacheEntryStreamAsync(terminal: ITerminal, cacheId: string, entryStream: Readable): Promise; } // @internal @@ -347,8 +349,10 @@ export interface ICloudBuildCacheProvider { readonly isCacheWriteAllowed: boolean; // (undocumented) tryGetCacheEntryBufferByIdAsync(terminal: ITerminal, cacheId: string): Promise; + tryGetCacheEntryStreamByIdAsync?(terminal: ITerminal, cacheId: string): Promise; // (undocumented) trySetCacheEntryBufferAsync(terminal: ITerminal, cacheId: string, entryBuffer: Buffer): Promise; + trySetCacheEntryStreamAsync?(terminal: ITerminal, cacheId: string, entryStream: Readable): Promise; // (undocumented) updateCachedCredentialAsync(terminal: ITerminal, credential: string): Promise; // (undocumented) diff --git a/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts b/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts index 1f1543a4d9f..14a0ce41bfa 100644 --- a/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts +++ b/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts @@ -2,6 +2,7 @@ // See LICENSE in the project root for license information. import type { Readable } from 'node:stream'; + import type { ITerminal } from '@rushstack/terminal'; /** diff --git a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts index 9ac2462b092..26fcaf3cdec 100644 --- a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts +++ b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts @@ -4,6 +4,7 @@ import * as path from 'node:path'; import * as crypto from 'node:crypto'; import { createReadStream } from 'node:fs'; +import type { Readable } from 'node:stream'; import { FileSystem, type FolderItem, InternalError, Async } from '@rushstack/node-core-library'; import type { ITerminal } from '@rushstack/terminal'; @@ -162,10 +163,8 @@ export class OperationBuildCache { if (this._cloudBuildCacheProvider.tryGetCacheEntryStreamByIdAsync) { // Use streaming path to avoid loading the entire cache entry into memory - const cacheEntryStream = await this._cloudBuildCacheProvider.tryGetCacheEntryStreamByIdAsync( - terminal, - cacheId - ); + const cacheEntryStream: Readable | undefined = + await this._cloudBuildCacheProvider.tryGetCacheEntryStreamByIdAsync(terminal, cacheId); if (cacheEntryStream) { try { localCacheEntryPath = await this._localBuildCacheProvider.trySetCacheEntryStreamAsync( @@ -334,7 +333,7 @@ export class OperationBuildCache { if (this._cloudBuildCacheProvider.trySetCacheEntryStreamAsync) { // Use streaming upload to avoid loading the entire cache entry into memory - const entryStream = createReadStream(localCacheEntryPath); + const entryStream: import('node:fs').ReadStream = createReadStream(localCacheEntryPath); setCloudCacheEntryPromise = this._cloudBuildCacheProvider.trySetCacheEntryStreamAsync( terminal, cacheId, From d9972f56ff18d84ed0e6b2020d26f056e4220a90 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 5 Apr 2026 03:48:01 +0000 Subject: [PATCH 03/45] address code review feedback: improve type annotations and JSDoc Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/050e10a7-3cad-4da4-93e5-9941453283b9 Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- .../src/logic/buildCache/ICloudBuildCacheProvider.ts | 5 +++++ .../rush-lib/src/logic/buildCache/OperationBuildCache.ts | 4 ++-- libraries/rush-lib/src/utilities/WebClient.ts | 3 ++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts b/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts index 14a0ce41bfa..9fee1f86125 100644 --- a/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts +++ b/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts @@ -24,6 +24,11 @@ export interface ICloudBuildCacheProvider { * If implemented, the build cache will prefer to use this method over * {@link ICloudBuildCacheProvider.trySetCacheEntryBufferAsync} to avoid loading the entire * cache entry into memory. + * + * @remarks + * Because the provided stream can only be consumed once, implementations should not + * attempt to retry the upload using the same stream. If retry logic is needed, + * consider buffering internally or returning `false` so the caller can retry. */ trySetCacheEntryStreamAsync?( terminal: ITerminal, diff --git a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts index 26fcaf3cdec..00a4d97b810 100644 --- a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts +++ b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts @@ -3,7 +3,7 @@ import * as path from 'node:path'; import * as crypto from 'node:crypto'; -import { createReadStream } from 'node:fs'; +import { type ReadStream, createReadStream } from 'node:fs'; import type { Readable } from 'node:stream'; import { FileSystem, type FolderItem, InternalError, Async } from '@rushstack/node-core-library'; @@ -333,7 +333,7 @@ export class OperationBuildCache { if (this._cloudBuildCacheProvider.trySetCacheEntryStreamAsync) { // Use streaming upload to avoid loading the entire cache entry into memory - const entryStream: import('node:fs').ReadStream = createReadStream(localCacheEntryPath); + const entryStream: ReadStream = createReadStream(localCacheEntryPath); setCloudCacheEntryPromise = this._cloudBuildCacheProvider.trySetCacheEntryStreamAsync( terminal, cacheId, diff --git a/libraries/rush-lib/src/utilities/WebClient.ts b/libraries/rush-lib/src/utilities/WebClient.ts index 3ea6c66e318..8d290da046d 100644 --- a/libraries/rush-lib/src/utilities/WebClient.ts +++ b/libraries/rush-lib/src/utilities/WebClient.ts @@ -299,7 +299,8 @@ function _sendRequestBody( body: Buffer | Readable | undefined, reject: (error: Error) => void ): void { - if (body && typeof (body as Readable).pipe === 'function') { + const isStream: boolean = !!body && typeof (body as Readable).pipe === 'function'; + if (isStream) { (body as Readable).on('error', reject); (body as Readable).pipe(req); } else { From 79b9197136c2dee91916a70dd0d648b84241d47b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 5 Apr 2026 03:56:55 +0000 Subject: [PATCH 04/45] Replace manual change files with rush change --bulk generated change file Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/798065bf-040f-462e-8ea9-b9ca5659196d Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- ...-cache-entry-for-http-plugin_2026-04-05-03-56.json | 11 +++++++++++ .../rush/stream-build-cache_2026-04-05-03-25.json | 10 ---------- .../stream-build-cache_2026-04-05-03-25.json | 10 ---------- 3 files changed, 11 insertions(+), 20 deletions(-) create mode 100644 common/changes/@microsoft/rush/copilot-stream-cache-entry-for-http-plugin_2026-04-05-03-56.json delete mode 100644 common/changes/@microsoft/rush/stream-build-cache_2026-04-05-03-25.json delete mode 100644 common/changes/@rushstack/rush-http-build-cache-plugin/stream-build-cache_2026-04-05-03-25.json diff --git a/common/changes/@microsoft/rush/copilot-stream-cache-entry-for-http-plugin_2026-04-05-03-56.json b/common/changes/@microsoft/rush/copilot-stream-cache-entry-for-http-plugin_2026-04-05-03-56.json new file mode 100644 index 00000000000..c8a4c76997e --- /dev/null +++ b/common/changes/@microsoft/rush/copilot-stream-cache-entry-for-http-plugin_2026-04-05-03-56.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "comment": "Add optional streaming APIs to ICloudBuildCacheProvider and FileSystemBuildCacheProvider, allowing cache plugins to stream entries to and from the cloud cache without buffering entire contents in memory. Implement streaming in rush-http-build-cache-plugin.", + "type": "none", + "packageName": "@microsoft/rush" + } + ], + "packageName": "@microsoft/rush", + "email": "198982749+Copilot@users.noreply.github.com" +} \ No newline at end of file diff --git a/common/changes/@microsoft/rush/stream-build-cache_2026-04-05-03-25.json b/common/changes/@microsoft/rush/stream-build-cache_2026-04-05-03-25.json deleted file mode 100644 index d26042ecc84..00000000000 --- a/common/changes/@microsoft/rush/stream-build-cache_2026-04-05-03-25.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "changes": [ - { - "packageName": "@microsoft/rush", - "comment": "Add optional streaming APIs (tryGetCacheEntryStreamByIdAsync, trySetCacheEntryStreamAsync) to ICloudBuildCacheProvider and FileSystemBuildCacheProvider, allowing cache plugins to transfer entries without buffering the entire contents in memory.", - "type": "none" - } - ], - "packageName": "@microsoft/rush" -} diff --git a/common/changes/@rushstack/rush-http-build-cache-plugin/stream-build-cache_2026-04-05-03-25.json b/common/changes/@rushstack/rush-http-build-cache-plugin/stream-build-cache_2026-04-05-03-25.json deleted file mode 100644 index e9f01dc3adb..00000000000 --- a/common/changes/@rushstack/rush-http-build-cache-plugin/stream-build-cache_2026-04-05-03-25.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "changes": [ - { - "packageName": "@rushstack/rush-http-build-cache-plugin", - "comment": "Implement streaming cache entry download and upload to avoid loading entire cache entries into memory, preventing out-of-memory crashes with large build outputs.", - "type": "minor" - } - ], - "packageName": "@rushstack/rush-http-build-cache-plugin" -} From bc7fa969f390376e1d375c6355af7d672fdcfe86 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 5 Apr 2026 04:28:22 +0000 Subject: [PATCH 05/45] Use createReadStream/createWriteStream from FileSystem instead of fs directly Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/5ad20d4d-c9a4-4855-bb13-8dd9e2c1350b Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- .../logic/buildCache/FileSystemBuildCacheProvider.ts | 3 +-- .../src/logic/buildCache/OperationBuildCache.ts | 11 ++++++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts b/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts index eed1bca7bf8..19f88b74e90 100644 --- a/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts +++ b/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts @@ -2,7 +2,6 @@ // See LICENSE in the project root for license information. import * as path from 'node:path'; -import { createWriteStream } from 'node:fs'; import { pipeline } from 'node:stream/promises'; import type { Readable } from 'node:stream'; @@ -90,7 +89,7 @@ export class FileSystemBuildCacheProvider { ): Promise { const cacheEntryFilePath: string = this.getCacheEntryPath(cacheId); await FileSystem.ensureFolderAsync(path.dirname(cacheEntryFilePath)); - await pipeline(entryStream, createWriteStream(cacheEntryFilePath)); + await pipeline(entryStream, FileSystem.createWriteStream(cacheEntryFilePath)); terminal.writeVerboseLine(`Wrote cache entry to "${cacheEntryFilePath}".`); return cacheEntryFilePath; } diff --git a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts index 00a4d97b810..9f56d6d4aca 100644 --- a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts +++ b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts @@ -3,10 +3,15 @@ import * as path from 'node:path'; import * as crypto from 'node:crypto'; -import { type ReadStream, createReadStream } from 'node:fs'; import type { Readable } from 'node:stream'; -import { FileSystem, type FolderItem, InternalError, Async } from '@rushstack/node-core-library'; +import { + FileSystem, + type FileSystemReadStream, + type FolderItem, + InternalError, + Async +} from '@rushstack/node-core-library'; import type { ITerminal } from '@rushstack/terminal'; import type { RushConfigurationProject } from '../../api/RushConfigurationProject'; @@ -333,7 +338,7 @@ export class OperationBuildCache { if (this._cloudBuildCacheProvider.trySetCacheEntryStreamAsync) { // Use streaming upload to avoid loading the entire cache entry into memory - const entryStream: ReadStream = createReadStream(localCacheEntryPath); + const entryStream: FileSystemReadStream = FileSystem.createReadStream(localCacheEntryPath); setCloudCacheEntryPromise = this._cloudBuildCacheProvider.trySetCacheEntryStreamAsync( terminal, cacheId, From 3dc688fea5fdcd117b80b24cd4428af79376cd27 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 5 Apr 2026 05:23:28 +0000 Subject: [PATCH 06/45] Use ensureFolderExists option in FileSystem.createWriteStream Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/199d4b3e-1f3f-44e1-9fc6-7b4a0e027c7e Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- .../src/logic/buildCache/FileSystemBuildCacheProvider.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts b/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts index 19f88b74e90..57b8836ac41 100644 --- a/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts +++ b/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts @@ -88,8 +88,10 @@ export class FileSystemBuildCacheProvider { entryStream: Readable ): Promise { const cacheEntryFilePath: string = this.getCacheEntryPath(cacheId); - await FileSystem.ensureFolderAsync(path.dirname(cacheEntryFilePath)); - await pipeline(entryStream, FileSystem.createWriteStream(cacheEntryFilePath)); + await pipeline( + entryStream, + FileSystem.createWriteStream(cacheEntryFilePath, { ensureFolderExists: true }) + ); terminal.writeVerboseLine(`Wrote cache entry to "${cacheEntryFilePath}".`); return cacheEntryFilePath; } From 9a408ab95d15a10c3eb36821c3ef91850820a681 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 5 Apr 2026 06:43:55 +0000 Subject: [PATCH 07/45] Add streaming support to Amazon S3 and Azure Storage build cache plugins Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/6212baeb-266c-4823-94df-251c69a8f74c Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- .../rush-amazon-s3-build-cache-plugin.api.md | 5 + .../src/AmazonS3BuildCacheProvider.ts | 40 ++++++ .../src/AmazonS3Client.ts | 133 ++++++++++++++++-- .../src/AzureStorageBuildCacheProvider.ts | 118 ++++++++++++++++ 4 files changed, 284 insertions(+), 12 deletions(-) diff --git a/common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md b/common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md index e0d5fe032ad..34baf02e416 100644 --- a/common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md +++ b/common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md @@ -8,6 +8,7 @@ import type { IRushPlugin } from '@rushstack/rush-sdk'; import { ITerminal } from '@rushstack/terminal'; +import type { Readable } from 'node:stream'; import type { RushConfiguration } from '@rushstack/rush-sdk'; import type { RushSession } from '@rushstack/rush-sdk'; import { WebClient } from '@rushstack/rush-sdk/lib/utilities/WebClient'; @@ -18,6 +19,8 @@ export class AmazonS3Client { // (undocumented) getObjectAsync(objectName: string): Promise; // (undocumented) + getObjectStreamAsync(objectName: string): Promise; + // (undocumented) _getSha256Hmac(key: string | Buffer, data: string): Buffer; // (undocumented) _getSha256Hmac(key: string | Buffer, data: string, encoding: 'hex'): string; @@ -26,6 +29,8 @@ export class AmazonS3Client { // (undocumented) uploadObjectAsync(objectName: string, objectBuffer: Buffer): Promise; // (undocumented) + uploadObjectStreamAsync(objectName: string, objectStream: Readable): Promise; + // (undocumented) static UriEncode(input: string): string; } diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts index ea8d7b903c0..30afacb2f84 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. +import type { Readable } from 'node:stream'; + import { type ICredentialCacheEntry, CredentialCache } from '@rushstack/credential-cache'; import type { ITerminal } from '@rushstack/terminal'; import { @@ -182,6 +184,44 @@ export class AmazonS3BuildCacheProvider implements ICloudBuildCacheProvider { } } + public async tryGetCacheEntryStreamByIdAsync( + terminal: ITerminal, + cacheId: string + ): Promise { + try { + const client: AmazonS3Client = await this._getS3ClientAsync(terminal); + return await client.getObjectStreamAsync(this._s3Prefix ? `${this._s3Prefix}/${cacheId}` : cacheId); + } catch (e) { + terminal.writeWarningLine(`Error getting cache entry stream from S3: ${e}`); + return undefined; + } + } + + public async trySetCacheEntryStreamAsync( + terminal: ITerminal, + cacheId: string, + entryStream: Readable + ): Promise { + if (!this.isCacheWriteAllowed) { + terminal.writeErrorLine('Writing to S3 cache is not allowed in the current configuration.'); + return false; + } + + terminal.writeDebugLine('Uploading object stream with cacheId: ', cacheId); + + try { + const client: AmazonS3Client = await this._getS3ClientAsync(terminal); + await client.uploadObjectStreamAsync( + this._s3Prefix ? `${this._s3Prefix}/${cacheId}` : cacheId, + entryStream + ); + return true; + } catch (e) { + terminal.writeWarningLine(`Error uploading cache entry stream to S3: ${e}`); + return false; + } + } + public async updateCachedCredentialAsync(terminal: ITerminal, credential: string): Promise { await CredentialCache.usingAsync( { diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts index 890aeba8593..15093888d5f 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts @@ -2,6 +2,7 @@ // See LICENSE in the project root for license information. import * as crypto from 'node:crypto'; +import type { Readable } from 'node:stream'; import { Async } from '@rushstack/node-core-library'; import { Colorize, type ITerminal } from '@rushstack/terminal'; @@ -9,6 +10,7 @@ import { type IGetFetchOptions, type IFetchOptionsWithBody, type IWebClientResponse, + type IWebClientStreamResponse, type WebClient, AUTHORIZATION_HEADER_NAME } from '@rushstack/rush-sdk/lib/utilities/WebClient'; @@ -21,6 +23,13 @@ const DATE_HEADER_NAME: 'x-amz-date' = 'x-amz-date'; const HOST_HEADER_NAME: 'host' = 'host'; const SECURITY_TOKEN_HEADER_NAME: 'x-amz-security-token' = 'x-amz-security-token'; +/** + * AWS Signature V4 allows using this sentinel value as the content hash when the request + * payload is not signed. This is used for streaming uploads where the body cannot be hashed + * upfront. + */ +const UNSIGNED_PAYLOAD: 'UNSIGNED-PAYLOAD' = 'UNSIGNED-PAYLOAD'; + interface IIsoDateString { date: string; dateTime: string; @@ -178,6 +187,73 @@ export class AmazonS3Client { }); } + public async getObjectStreamAsync(objectName: string): Promise { + this._writeDebugLine('Reading object stream from S3'); + return await this._sendCacheRequestWithRetriesAsync(async () => { + const response: IWebClientStreamResponse = await this._makeStreamRequestAsync('GET', objectName); + if (response.ok) { + return { + hasNetworkError: false, + response: response.stream + }; + } else if (response.status === 404) { + response.stream.resume(); + return { + hasNetworkError: false, + response: undefined + }; + } else if ( + (response.status === 400 || response.status === 401 || response.status === 403) && + !this._credentials + ) { + response.stream.resume(); + this._writeWarningLine( + `No credentials found and received a ${response.status}`, + ' response code from the cloud storage.', + ' Maybe run rush update-cloud-credentials', + ' or set the RUSH_BUILD_CACHE_CREDENTIAL env' + ); + return { + hasNetworkError: false, + response: undefined + }; + } else if (response.status === 400 || response.status === 401 || response.status === 403) { + response.stream.resume(); + throw new Error( + `Amazon S3 responded with status code ${response.status} (${response.statusText})` + ); + } else { + response.stream.resume(); + return { + hasNetworkError: true, + error: new Error( + `Amazon S3 responded with status code ${response.status} (${response.statusText})` + ) + }; + } + }); + } + + public async uploadObjectStreamAsync(objectName: string, objectStream: Readable): Promise { + if (!this._credentials) { + throw new Error('Credentials are required to upload objects to S3.'); + } + + // Streaming uploads cannot be retried because the stream is consumed after the first attempt. + const response: IWebClientStreamResponse = await this._makeStreamRequestAsync( + 'PUT', + objectName, + objectStream + ); + if (!response.ok) { + response.stream.resume(); + throw new Error( + `Amazon S3 responded with status code ${response.status} (${response.statusText})` + ); + } + response.stream.resume(); + } + private _writeDebugLine(...messageParts: string[]): void { // if the terminal has been closed then don't bother sending a debug message try { @@ -201,8 +277,51 @@ export class AmazonS3Client { objectName: string, body?: Buffer ): Promise { - const isoDateString: IIsoDateString = this._getIsoDateString(); const bodyHash: string = this._getSha256(body); + const { url, headers } = this._buildSignedRequest(verb, objectName, bodyHash); + + const webFetchOptions: IGetFetchOptions | IFetchOptionsWithBody = { + verb, + headers + }; + if (verb === 'PUT') { + (webFetchOptions as IFetchOptionsWithBody).body = body; + } + + const response: IWebClientResponse = await this._webClient.fetchAsync(url, webFetchOptions); + + return response; + } + + private async _makeStreamRequestAsync( + verb: 'GET' | 'PUT', + objectName: string, + body?: Readable + ): Promise { + // For streaming uploads, the body cannot be hashed upfront, so we use UNSIGNED-PAYLOAD. + const bodyHash: string = body ? UNSIGNED_PAYLOAD : this._getSha256(undefined); + const { url, headers } = this._buildSignedRequest(verb, objectName, bodyHash); + + const webFetchOptions: IGetFetchOptions | IFetchOptionsWithBody = { + verb, + headers + }; + if (verb === 'PUT' && body) { + (webFetchOptions as IFetchOptionsWithBody).body = body; + } + + return await this._webClient.fetchStreamAsync(url, webFetchOptions); + } + + /** + * Builds an AWS Signature V4 signed request, returning the URL and signed headers. + */ + private _buildSignedRequest( + verb: 'GET' | 'PUT', + objectName: string, + bodyHash: string + ): { url: string; headers: Record } { + const isoDateString: IIsoDateString = this._getIsoDateString(); const headers: Record = {}; headers[DATE_HEADER_NAME] = isoDateString.dateTime; headers[CONTENT_HASH_HEADER_NAME] = bodyHash; @@ -299,14 +418,6 @@ export class AmazonS3Client { } } - const webFetchOptions: IGetFetchOptions | IFetchOptionsWithBody = { - verb, - headers - }; - if (verb === 'PUT') { - (webFetchOptions as IFetchOptionsWithBody).body = body; - } - const url: string = `${this._s3Endpoint}${canonicalUri}`; this._writeDebugLine(Colorize.bold(Colorize.underline('Sending request to S3'))); @@ -316,9 +427,7 @@ export class AmazonS3Client { this._writeDebugLine(Colorize.cyan(`\t${name}: ${value}`)); } - const response: IWebClientResponse = await this._webClient.fetchAsync(url, webFetchOptions); - - return response; + return { url, headers }; } public _getSha256Hmac(key: string | Buffer, data: string): Buffer; diff --git a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts index 146ee2e8f00..a6ebcdf93af 100644 --- a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts +++ b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts @@ -1,8 +1,11 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. +import type { Readable } from 'node:stream'; + import { type BlobClient, + type BlobDownloadResponseParsed, BlobServiceClient, type BlockBlobClient, type ContainerClient @@ -190,12 +193,127 @@ export class AzureStorageBuildCacheProvider } } + public async tryGetCacheEntryStreamByIdAsync( + terminal: ITerminal, + cacheId: string + ): Promise { + const blobClient: BlobClient = await this._getBlobClientForCacheIdAsync(cacheId, terminal); + + try { + const blobExists: boolean = await blobClient.exists(); + if (blobExists) { + const downloadResponse: BlobDownloadResponseParsed = await blobClient.download(); + const readableStreamBody: NodeJS.ReadableStream | undefined = + downloadResponse.readableStreamBody; + if (readableStreamBody) { + return readableStreamBody as unknown as Readable; + } + return undefined; + } else { + return undefined; + } + } catch (err) { + this._logBlobError(terminal, err, 'Error getting cache entry stream from Azure Storage: '); + return undefined; + } + } + + public async trySetCacheEntryStreamAsync( + terminal: ITerminal, + cacheId: string, + entryStream: Readable + ): Promise { + if (!this.isCacheWriteAllowed) { + terminal.writeErrorLine( + 'Writing to Azure Blob Storage cache is not allowed in the current configuration.' + ); + return false; + } + + const blobClient: BlobClient = await this._getBlobClientForCacheIdAsync(cacheId, terminal); + const blockBlobClient: BlockBlobClient = blobClient.getBlockBlobClient(); + let blobAlreadyExists: boolean = false; + + try { + blobAlreadyExists = await blockBlobClient.exists(); + } catch (err) { + const e: IBlobError = err as IBlobError; + + const errorMessage: string = + 'Error checking if cache entry exists in Azure Storage: ' + + [e.name, e.message, e.response?.status, e.response?.parsedHeaders?.errorCode] + .filter((piece: string | undefined) => piece) + .join(' '); + + terminal.writeWarningLine(errorMessage); + } + + if (blobAlreadyExists) { + terminal.writeVerboseLine('Build cache entry blob already exists.'); + // Drain the incoming stream since we won't consume it + entryStream.resume(); + return true; + } else { + try { + await blockBlobClient.uploadStream(entryStream); + return true; + } catch (e) { + if ((e as IBlobError).statusCode === 409 /* conflict */) { + terminal.writeVerboseLine( + 'Azure Storage returned status 409 (conflict). The cache entry has ' + + `probably already been set by another builder. Code: "${(e as IBlobError).code}".` + ); + return true; + } else { + terminal.writeWarningLine(`Error uploading cache entry stream to Azure Storage: ${e}`); + return false; + } + } + } + } + private async _getBlobClientForCacheIdAsync(cacheId: string, terminal: ITerminal): Promise { const client: ContainerClient = await this._getContainerClientAsync(terminal); const blobName: string = this._blobPrefix ? `${this._blobPrefix}/${cacheId}` : cacheId; return client.getBlobClient(blobName); } + private _logBlobError(terminal: ITerminal, err: unknown, prefix: string): void { + const e: IBlobError = err as IBlobError; + const errorMessage: string = + prefix + + [e.name, e.message, e.response?.status, e.response?.parsedHeaders?.errorCode] + .filter((piece: string | undefined) => piece) + .join(' '); + + if (e.response?.parsedHeaders?.errorCode === 'PublicAccessNotPermitted') { + terminal.writeWarningLine( + `${errorMessage}\n\n` + + `You need to configure Azure Storage SAS credentials to access the build cache.\n` + + `Update the credentials by running "rush ${RushConstants.updateCloudCredentialsCommandName}", \n` + + `or provide a SAS in the ` + + `${EnvironmentVariableNames.RUSH_BUILD_CACHE_CREDENTIAL} environment variable.` + ); + } else if (e.response?.parsedHeaders?.errorCode === 'AuthenticationFailed') { + terminal.writeWarningLine( + `${errorMessage}\n\n` + + `Your Azure Storage SAS credentials are not valid.\n` + + `Update the credentials by running "rush ${RushConstants.updateCloudCredentialsCommandName}", \n` + + `or provide a SAS in the ` + + `${EnvironmentVariableNames.RUSH_BUILD_CACHE_CREDENTIAL} environment variable.` + ); + } else if (e.response?.parsedHeaders?.errorCode === 'AuthorizationPermissionMismatch') { + terminal.writeWarningLine( + `${errorMessage}\n\n` + + `Your Azure Storage SAS credentials are valid, but do not have permission to read the build cache.\n` + + `Make sure you have added the role 'Storage Blob Data Reader' to the appropriate user(s) or group(s)\n` + + `on your storage account in the Azure Portal.` + ); + } else { + terminal.writeWarningLine(errorMessage); + } + } + private async _getContainerClientAsync(terminal: ITerminal): Promise { if (!this._containerClient) { let sasString: string | undefined = this._environmentCredential; From 2ad8219a90e360dea630a6a73e6f823553a5682d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 5 Apr 2026 06:48:02 +0000 Subject: [PATCH 08/45] Address code review: add documentation for type assertion and no-retry streaming upload Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/6212baeb-266c-4823-94df-251c69a8f74c Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md | 1 - .../rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts | 5 +++++ .../src/AzureStorageBuildCacheProvider.ts | 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md b/common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md index 34baf02e416..14c9b2f6445 100644 --- a/common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md +++ b/common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md @@ -28,7 +28,6 @@ export class AmazonS3Client { static tryDeserializeCredentials(credentialString: string | undefined): IAmazonS3Credentials | undefined; // (undocumented) uploadObjectAsync(objectName: string, objectBuffer: Buffer): Promise; - // (undocumented) uploadObjectStreamAsync(objectName: string, objectStream: Readable): Promise; // (undocumented) static UriEncode(input: string): string; diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts index 15093888d5f..f01e76b7b18 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts @@ -234,6 +234,11 @@ export class AmazonS3Client { }); } + /** + * Uploads a readable stream to S3. Unlike {@link AmazonS3Client.uploadObjectAsync}, this method + * does not use retry logic because the stream is consumed after the first attempt and cannot be + * replayed. The caller should handle failures accordingly. + */ public async uploadObjectStreamAsync(objectName: string, objectStream: Readable): Promise { if (!this._credentials) { throw new Error('Credentials are required to upload objects to S3.'); diff --git a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts index a6ebcdf93af..72925454eeb 100644 --- a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts +++ b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts @@ -206,6 +206,9 @@ export class AzureStorageBuildCacheProvider const readableStreamBody: NodeJS.ReadableStream | undefined = downloadResponse.readableStreamBody; if (readableStreamBody) { + // The Azure SDK types readableStreamBody as NodeJS.ReadableStream, which is a minimal + // interface. In practice, the underlying implementation is a Node.js Readable stream + // (http.IncomingMessage), so this assertion is safe. return readableStreamBody as unknown as Readable; } return undefined; From 80affd8f8be6906f6fb7fac299851b8667e5555a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 5 Apr 2026 19:57:01 +0000 Subject: [PATCH 09/45] Gate streaming build cache behind useStreamingBuildCache experiment flag Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/63fb5abe-e500-4a9c-bd82-3ed613989ef4 Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- common/reviews/api/rush-lib.api.md | 2 ++ .../src/api/ExperimentsConfiguration.ts | 8 +++++ .../cli/scriptActions/PhasedScriptAction.ts | 4 ++- .../logic/buildCache/OperationBuildCache.ts | 25 ++++++++++++---- .../test/OperationBuildCache.test.ts | 3 +- .../operations/CacheableOperationPlugin.ts | 29 ++++++++++++++----- .../src/schemas/experiments.schema.json | 4 +++ 7 files changed, 61 insertions(+), 14 deletions(-) diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index 01c299eed19..5034164d89f 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -490,6 +490,7 @@ export interface IExperimentsJson { usePnpmLockfileOnlyThenFrozenLockfileForRushUpdate?: boolean; usePnpmPreferFrozenLockfileForRushUpdate?: boolean; usePnpmSyncForInjectedDependencies?: boolean; + useStreamingBuildCache?: boolean; } // @beta @@ -598,6 +599,7 @@ export interface _IOperationBuildCacheOptions { buildCacheConfiguration: BuildCacheConfiguration; excludeAppleDoubleFiles: boolean; terminal: ITerminal; + useStreamingBuildCache: boolean; } // @alpha diff --git a/libraries/rush-lib/src/api/ExperimentsConfiguration.ts b/libraries/rush-lib/src/api/ExperimentsConfiguration.ts index f8a9ae66e34..75c4d654385 100644 --- a/libraries/rush-lib/src/api/ExperimentsConfiguration.ts +++ b/libraries/rush-lib/src/api/ExperimentsConfiguration.ts @@ -136,6 +136,14 @@ export interface IExperimentsJson { * be included in the shared build cache. */ omitAppleDoubleFilesFromBuildCache?: boolean; + + /** + * If true, the build cache will use streaming APIs to transfer cache entries to and from cloud + * storage. This avoids loading the entire cache entry into memory, which can prevent out-of-memory + * errors for large build outputs. The cloud cache provider plugin must implement the optional + * streaming methods for this to take effect; otherwise it falls back to the buffer-based approach. + */ + useStreamingBuildCache?: boolean; } const _EXPERIMENTS_JSON_SCHEMA: JsonSchema = JsonSchema.fromLoadedObject(schemaJson); diff --git a/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts b/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts index fd41ff7ce7c..96bce13ea26 100644 --- a/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts +++ b/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts @@ -524,7 +524,9 @@ export class PhasedScriptAction extends BaseScriptAction i terminal, excludeAppleDoubleFiles: !!this.rushConfiguration.experimentsConfiguration.configuration - .omitAppleDoubleFilesFromBuildCache + .omitAppleDoubleFilesFromBuildCache, + useStreamingBuildCache: + !!this.rushConfiguration.experimentsConfiguration.configuration.useStreamingBuildCache }).apply(this.hooks); if (this._debugBuildCacheIdsParameter.value) { diff --git a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts index 9f56d6d4aca..06796c95482 100644 --- a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts +++ b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts @@ -39,6 +39,11 @@ export interface IOperationBuildCacheOptions { * and a companion file exists in the same directory. */ excludeAppleDoubleFiles: boolean; + /** + * If true, use streaming APIs (when available) to transfer cache entries to and from the + * cloud provider, avoiding buffering the entire entry in memory. + */ + useStreamingBuildCache: boolean; } /** @@ -82,6 +87,7 @@ export class OperationBuildCache { private readonly _projectOutputFolderNames: ReadonlyArray; private readonly _cacheId: string | undefined; private readonly _excludeAppleDoubleFiles: boolean; + private readonly _useStreamingBuildCache: boolean; private constructor(cacheId: string | undefined, options: IProjectBuildCacheOptions) { const { @@ -93,7 +99,8 @@ export class OperationBuildCache { }, project, projectOutputFolderNames, - excludeAppleDoubleFiles + excludeAppleDoubleFiles, + useStreamingBuildCache } = options; this._project = project; this._localBuildCacheProvider = localCacheProvider; @@ -103,6 +110,7 @@ export class OperationBuildCache { this._projectOutputFolderNames = projectOutputFolderNames || []; this._cacheId = cacheId; this._excludeAppleDoubleFiles = excludeAppleDoubleFiles && process.platform === 'darwin'; + this._useStreamingBuildCache = useStreamingBuildCache; } private static _tryGetTarUtility(terminal: ITerminal): Promise { @@ -126,7 +134,7 @@ export class OperationBuildCache { executionResult: IOperationExecutionResult, options: IOperationBuildCacheOptions ): OperationBuildCache { - const { buildCacheConfiguration, terminal, excludeAppleDoubleFiles } = options; + const { buildCacheConfiguration, terminal, excludeAppleDoubleFiles, useStreamingBuildCache } = options; const outputFolders: string[] = [...(executionResult.operation.settings?.outputFolderNames ?? [])]; if (executionResult.metadataFolderPath) { outputFolders.push(executionResult.metadataFolderPath); @@ -139,7 +147,8 @@ export class OperationBuildCache { phaseName: executionResult.operation.associatedPhase.name, projectOutputFolderNames: outputFolders, operationStateHash: executionResult.getStateHash(), - excludeAppleDoubleFiles + excludeAppleDoubleFiles, + useStreamingBuildCache }; const cacheId: string | undefined = OperationBuildCache._getCacheId(buildCacheOptions); return new OperationBuildCache(cacheId, buildCacheOptions); @@ -166,7 +175,10 @@ export class OperationBuildCache { 'This project was not found in the local build cache. Querying the cloud build cache.' ); - if (this._cloudBuildCacheProvider.tryGetCacheEntryStreamByIdAsync) { + if ( + this._useStreamingBuildCache && + this._cloudBuildCacheProvider.tryGetCacheEntryStreamByIdAsync + ) { // Use streaming path to avoid loading the entire cache entry into memory const cacheEntryStream: Readable | undefined = await this._cloudBuildCacheProvider.tryGetCacheEntryStreamByIdAsync(terminal, cacheId); @@ -336,7 +348,10 @@ export class OperationBuildCache { throw new InternalError('Expected the local cache entry path to be set.'); } - if (this._cloudBuildCacheProvider.trySetCacheEntryStreamAsync) { + if ( + this._useStreamingBuildCache && + this._cloudBuildCacheProvider.trySetCacheEntryStreamAsync + ) { // Use streaming upload to avoid loading the entire cache entry into memory const entryStream: FileSystemReadStream = FileSystem.createReadStream(localCacheEntryPath); setCloudCacheEntryPromise = this._cloudBuildCacheProvider.trySetCacheEntryStreamAsync( diff --git a/libraries/rush-lib/src/logic/buildCache/test/OperationBuildCache.test.ts b/libraries/rush-lib/src/logic/buildCache/test/OperationBuildCache.test.ts index 08a35a85428..c5e99901d60 100644 --- a/libraries/rush-lib/src/logic/buildCache/test/OperationBuildCache.test.ts +++ b/libraries/rush-lib/src/logic/buildCache/test/OperationBuildCache.test.ts @@ -59,7 +59,8 @@ describe(OperationBuildCache.name, () => { operationStateHash: '1926f30e8ed24cb47be89aea39e7efd70fcda075', terminal, phaseName: 'build', - excludeAppleDoubleFiles: !!options.excludeAppleDoubleFiles + excludeAppleDoubleFiles: !!options.excludeAppleDoubleFiles, + useStreamingBuildCache: false }); return subject; diff --git a/libraries/rush-lib/src/logic/operations/CacheableOperationPlugin.ts b/libraries/rush-lib/src/logic/operations/CacheableOperationPlugin.ts index 90af287b9dd..3ad8d73a170 100644 --- a/libraries/rush-lib/src/logic/operations/CacheableOperationPlugin.ts +++ b/libraries/rush-lib/src/logic/operations/CacheableOperationPlugin.ts @@ -77,6 +77,7 @@ export interface ICacheableOperationPluginOptions { cobuildConfiguration: CobuildConfiguration | undefined; terminal: ITerminal; excludeAppleDoubleFiles: boolean; + useStreamingBuildCache: boolean; } interface ITryGetOperationBuildCacheOptionsBase { @@ -84,6 +85,7 @@ interface ITryGetOperationBuildCacheOptionsBase { buildCacheConfiguration: BuildCacheConfiguration | undefined; terminal: ITerminal; excludeAppleDoubleFiles: boolean; + useStreamingBuildCache: boolean; record: TRecord; } @@ -108,7 +110,8 @@ export class CacheableOperationPlugin implements IPhasedCommandPlugin { allowWarningsInSuccessfulBuild, buildCacheConfiguration, cobuildConfiguration, - excludeAppleDoubleFiles + excludeAppleDoubleFiles, + useStreamingBuildCache } = this._options; hooks.beforeExecuteOperations.tap( @@ -272,7 +275,8 @@ export class CacheableOperationPlugin implements IPhasedCommandPlugin { buildCacheConfiguration, terminal: buildCacheTerminal, record, - excludeAppleDoubleFiles + excludeAppleDoubleFiles, + useStreamingBuildCache }); // Try to acquire the cobuild lock @@ -291,7 +295,8 @@ export class CacheableOperationPlugin implements IPhasedCommandPlugin { buildCacheContext, record, terminal: buildCacheTerminal, - excludeAppleDoubleFiles + excludeAppleDoubleFiles, + useStreamingBuildCache }); if (operationBuildCache) { buildCacheTerminal.writeVerboseLine( @@ -585,7 +590,14 @@ export class CacheableOperationPlugin implements IPhasedCommandPlugin { private _tryGetOperationBuildCache( options: ITryGetOperationBuildCacheOptions ): OperationBuildCache | undefined { - const { buildCacheConfiguration, buildCacheContext, terminal, record, excludeAppleDoubleFiles } = options; + const { + buildCacheConfiguration, + buildCacheContext, + terminal, + record, + excludeAppleDoubleFiles, + useStreamingBuildCache + } = options; if (!buildCacheContext.operationBuildCache) { const { cacheDisabledReason } = buildCacheContext; if (cacheDisabledReason && !record.operation.settings?.allowCobuildWithoutCache) { @@ -601,7 +613,8 @@ export class CacheableOperationPlugin implements IPhasedCommandPlugin { buildCacheContext.operationBuildCache = OperationBuildCache.forOperation(record, { buildCacheConfiguration, terminal, - excludeAppleDoubleFiles + excludeAppleDoubleFiles, + useStreamingBuildCache }); } @@ -618,7 +631,8 @@ export class CacheableOperationPlugin implements IPhasedCommandPlugin { cobuildConfiguration, record, terminal, - excludeAppleDoubleFiles + excludeAppleDoubleFiles, + useStreamingBuildCache } = options; if (!buildCacheConfiguration?.buildCacheEnabled) { @@ -649,7 +663,8 @@ export class CacheableOperationPlugin implements IPhasedCommandPlugin { terminal, operationStateHash, phaseName: associatedPhase.name, - excludeAppleDoubleFiles + excludeAppleDoubleFiles, + useStreamingBuildCache }); buildCacheContext.operationBuildCache = operationBuildCache; diff --git a/libraries/rush-lib/src/schemas/experiments.schema.json b/libraries/rush-lib/src/schemas/experiments.schema.json index 8a92fa9ee67..45e3d6fbcc8 100644 --- a/libraries/rush-lib/src/schemas/experiments.schema.json +++ b/libraries/rush-lib/src/schemas/experiments.schema.json @@ -85,6 +85,10 @@ "omitAppleDoubleFilesFromBuildCache": { "description": "If true, when running on macOS, Rush will omit AppleDouble files (._*) from build cache archives when a companion file exists in the same directory. AppleDouble files are automatically created by macOS to store extended attributes on filesystems that don't support them, and should generally not be included in the shared build cache.", "type": "boolean" + }, + "useStreamingBuildCache": { + "description": "If true, the build cache will use streaming APIs to transfer cache entries to and from cloud storage. This avoids loading the entire cache entry into memory, which can prevent out-of-memory errors for large build outputs. The cloud cache provider plugin must implement the optional streaming methods for this to take effect; otherwise it falls back to the buffer-based approach.", + "type": "boolean" } }, "additionalProperties": false From 047586d3528177756c9093294225412577666fec Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 5 Apr 2026 13:42:49 -0700 Subject: [PATCH 10/45] Update changelogs. Co-authored-by: Ian Clanton-Thuon --- ...lot-stream-cache-entry-for-http-plugin_2026-04-05-03-56.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/changes/@microsoft/rush/copilot-stream-cache-entry-for-http-plugin_2026-04-05-03-56.json b/common/changes/@microsoft/rush/copilot-stream-cache-entry-for-http-plugin_2026-04-05-03-56.json index c8a4c76997e..7942e4c0699 100644 --- a/common/changes/@microsoft/rush/copilot-stream-cache-entry-for-http-plugin_2026-04-05-03-56.json +++ b/common/changes/@microsoft/rush/copilot-stream-cache-entry-for-http-plugin_2026-04-05-03-56.json @@ -1,7 +1,7 @@ { "changes": [ { - "comment": "Add optional streaming APIs to ICloudBuildCacheProvider and FileSystemBuildCacheProvider, allowing cache plugins to stream entries to and from the cloud cache without buffering entire contents in memory. Implement streaming in rush-http-build-cache-plugin.", + "comment": "Add optional streaming APIs to `ICloudBuildCacheProvider` and `FileSystemBuildCacheProvider`, allowing cache plugins to stream entries to and from the cloud cache without buffering entire contents in memory. Implement streaming in `@rushstack/rush-http-build-cache-plugin`, `@rushstack/rush-amazon-s3-build-cache-plugin`, and `@rushstack/rush-azure-storage-build-cache-plugin`.", "type": "none", "packageName": "@microsoft/rush" } From 689248cc5ace48444c34d30756ed116816e95e17 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 5 Apr 2026 13:24:20 -0700 Subject: [PATCH 11/45] Expand FileSystem.createWriteStream. --- .../logic/buildCache/FileSystemBuildCacheProvider.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts b/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts index 57b8836ac41..5b73a5916b6 100644 --- a/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts +++ b/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts @@ -5,7 +5,7 @@ import * as path from 'node:path'; import { pipeline } from 'node:stream/promises'; import type { Readable } from 'node:stream'; -import { FileSystem } from '@rushstack/node-core-library'; +import { FileSystem, type FileSystemWriteStream } from '@rushstack/node-core-library'; import type { ITerminal } from '@rushstack/terminal'; import type { RushConfiguration } from '../../api/RushConfiguration'; @@ -88,10 +88,10 @@ export class FileSystemBuildCacheProvider { entryStream: Readable ): Promise { const cacheEntryFilePath: string = this.getCacheEntryPath(cacheId); - await pipeline( - entryStream, - FileSystem.createWriteStream(cacheEntryFilePath, { ensureFolderExists: true }) - ); + const writeStream: FileSystemWriteStream = await FileSystem.createWriteStreamAsync(cacheEntryFilePath, { + ensureFolderExists: true + }); + await pipeline(entryStream, writeStream); terminal.writeVerboseLine(`Wrote cache entry to "${cacheEntryFilePath}".`); return cacheEntryFilePath; } From f685cc919db01b4c3eb240d2e58bd2c965546b8d Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 5 Apr 2026 13:42:23 -0700 Subject: [PATCH 12/45] Use destructuring on rushConfiguration in PhasedScriptAction. --- .../cli/scriptActions/PhasedScriptAction.ts | 44 +++++++++++-------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts b/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts index 96bce13ea26..09ecbbe410c 100644 --- a/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts +++ b/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts @@ -350,6 +350,11 @@ export class PhasedScriptAction extends BaseScriptAction i public async runAsync(): Promise { const stopwatch: Stopwatch = Stopwatch.start(); + const { + defaultSubspace, + subspacesFeatureEnabled, + pnpmOptions: { useWorkspaces } + } = this.rushConfiguration; if (this._alwaysInstall || this._installParameter?.value) { await measureAsyncFn(`${PERF_PREFIX}:install`, async () => { const { doBasicInstallAsync } = await import( @@ -373,7 +378,7 @@ export class PhasedScriptAction extends BaseScriptAction i afterInstallAsync: (subspace: Subspace) => this.rushSession.hooks.afterInstall.promise(this, subspace, variant), // Eventually we may want to allow a subspace to be selected here - subspace: this.rushConfiguration.defaultSubspace + subspace: defaultSubspace }); }); } @@ -382,14 +387,12 @@ export class PhasedScriptAction extends BaseScriptAction i await measureAsyncFn(`${PERF_PREFIX}:checkInstallFlag`, async () => { // TODO: Replace with last-install.flag when "rush link" and "rush unlink" are removed const lastLinkFlag: FlagFile = new FlagFile( - this.rushConfiguration.defaultSubspace.getSubspaceTempFolderPath(), + defaultSubspace.getSubspaceTempFolderPath(), RushConstants.lastLinkFlagFilename, {} ); // Only check for a valid link flag when subspaces is not enabled - if (!(await lastLinkFlag.isValidAsync()) && !this.rushConfiguration.subspacesFeatureEnabled) { - const useWorkspaces: boolean = - this.rushConfiguration.pnpmOptions && this.rushConfiguration.pnpmOptions.useWorkspaces; + if (!(await lastLinkFlag.isValidAsync()) && !subspacesFeatureEnabled) { if (useWorkspaces) { throw new Error('Link flag invalid.\nDid you run "rush install" or "rush update"?'); } else { @@ -513,20 +516,27 @@ export class PhasedScriptAction extends BaseScriptAction i ).IPCOperationRunnerPlugin().apply(this.hooks); } + const { + experimentsConfiguration: { + configuration: { + buildCacheWithAllowWarningsInSuccessfulBuild = false, + buildSkipWithAllowWarningsInSuccessfulBuild, + omitAppleDoubleFilesFromBuildCache: excludeAppleDoubleFiles = false, + useStreamingBuildCache = false, + usePnpmSyncForInjectedDependencies + } + }, + isPnpm + } = this.rushConfiguration; if (buildCacheConfiguration?.buildCacheEnabled) { terminal.writeVerboseLine(`Incremental strategy: cache restoration`); new CacheableOperationPlugin({ - allowWarningsInSuccessfulBuild: - !!this.rushConfiguration.experimentsConfiguration.configuration - .buildCacheWithAllowWarningsInSuccessfulBuild, + allowWarningsInSuccessfulBuild: buildCacheWithAllowWarningsInSuccessfulBuild, buildCacheConfiguration, cobuildConfiguration, terminal, - excludeAppleDoubleFiles: - !!this.rushConfiguration.experimentsConfiguration.configuration - .omitAppleDoubleFilesFromBuildCache, - useStreamingBuildCache: - !!this.rushConfiguration.experimentsConfiguration.configuration.useStreamingBuildCache + excludeAppleDoubleFiles, + useStreamingBuildCache }).apply(this.hooks); if (this._debugBuildCacheIdsParameter.value) { @@ -536,9 +546,7 @@ export class PhasedScriptAction extends BaseScriptAction i terminal.writeVerboseLine(`Incremental strategy: output preservation`); // Explicitly disabling the build cache also disables legacy skip detection. new LegacySkipPlugin({ - allowWarningsInSuccessfulBuild: - this.rushConfiguration.experimentsConfiguration.configuration - .buildSkipWithAllowWarningsInSuccessfulBuild, + allowWarningsInSuccessfulBuild: buildSkipWithAllowWarningsInSuccessfulBuild, terminal, changedProjectsOnly, isIncrementalBuildAllowed: this._isIncrementalBuildAllowed @@ -553,12 +561,12 @@ export class PhasedScriptAction extends BaseScriptAction i if (!buildCacheConfiguration?.buildCacheEnabled) { throw new Error('You must have build cache enabled to use this option.'); } + const { BuildPlanPlugin } = await import('../../logic/operations/BuildPlanPlugin'); new BuildPlanPlugin(terminal).apply(this.hooks); } - const { configuration: experiments } = this.rushConfiguration.experimentsConfiguration; - if (this.rushConfiguration?.isPnpm && experiments?.usePnpmSyncForInjectedDependencies) { + if (isPnpm && usePnpmSyncForInjectedDependencies) { const { PnpmSyncCopyOperationPlugin } = await import( '../../logic/operations/PnpmSyncCopyOperationPlugin' ); From 97307f3521419aebf09bcbaecddd6ac59df03c7d Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 5 Apr 2026 13:52:09 -0700 Subject: [PATCH 13/45] Clean up some duplicated types. --- libraries/rush-lib/src/utilities/WebClient.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/libraries/rush-lib/src/utilities/WebClient.ts b/libraries/rush-lib/src/utilities/WebClient.ts index 8d290da046d..752b3b90cab 100644 --- a/libraries/rush-lib/src/utilities/WebClient.ts +++ b/libraries/rush-lib/src/utilities/WebClient.ts @@ -16,15 +16,18 @@ import { Import, LegacyAdapters } from '@rushstack/node-core-library'; const createHttpsProxyAgent: typeof import('https-proxy-agent') = Import.lazy('https-proxy-agent', require); -/** - * For use with {@link WebClient}. - */ -export interface IWebClientResponse { +export interface IWebClientResponseBase { ok: boolean; status: number; statusText?: string; redirected: boolean; headers: Record; +} + +/** + * For use with {@link WebClient}. + */ +export interface IWebClientResponse extends IWebClientResponseBase { getTextAsync: () => Promise; getJsonAsync: () => Promise; getBufferAsync: () => Promise; @@ -34,12 +37,7 @@ export interface IWebClientResponse { * A response from {@link WebClient.fetchStreamAsync} that provides the response body as a * readable stream, avoiding buffering the entire response in memory. */ -export interface IWebClientStreamResponse { - ok: boolean; - status: number; - statusText?: string; - redirected: boolean; - headers: Record; +export interface IWebClientStreamResponse extends IWebClientResponseBase { stream: Readable; } From 5eb0cc3e7eb9db29cb46665da77efd8800d1565a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 5 Apr 2026 21:22:47 +0000 Subject: [PATCH 14/45] Fix CI: add missing useStreamingBuildCache to bridge plugin, fix WebClient private member type mismatch Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/1b32730c-9ce0-4e75-a2e1-45f06a628960 Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- libraries/rush-lib/src/utilities/WebClient.ts | 117 +++++++++--------- .../src/BridgeCachePlugin.ts | 3 +- 2 files changed, 62 insertions(+), 58 deletions(-) diff --git a/libraries/rush-lib/src/utilities/WebClient.ts b/libraries/rush-lib/src/utilities/WebClient.ts index 752b3b90cab..054c8fe1f81 100644 --- a/libraries/rush-lib/src/utilities/WebClient.ts +++ b/libraries/rush-lib/src/utilities/WebClient.ts @@ -351,7 +351,7 @@ export class WebClient { url: string, options?: IGetFetchOptions | IFetchOptionsWithBody ): Promise { - const requestInit: IRequestOptions = this._buildRequestOptions(options); + const requestInit: IRequestOptions = buildRequestOptions(this, options); return await WebClient._requestFn(url, requestInit); } @@ -363,68 +363,71 @@ export class WebClient { url: string, options?: IGetFetchOptions | IFetchOptionsWithBody ): Promise { - const requestInit: IRequestOptions = this._buildRequestOptions(options); + const requestInit: IRequestOptions = buildRequestOptions(this, options); return await WebClient._streamRequestFn(url, requestInit); } +} - private _buildRequestOptions(options?: IGetFetchOptions | IFetchOptionsWithBody): IRequestOptions { - const { - headers: optionsHeaders, - timeoutMs = 15 * 1000, - verb, - redirect, - body, - noDecode - } = (options as IFetchOptionsWithBody | undefined) ?? {}; - - const headers: Record = {}; - - WebClient.mergeHeaders(headers, this.standardHeaders); - - if (optionsHeaders) { - WebClient.mergeHeaders(headers, optionsHeaders); - } - - if (this.userAgent) { - headers[USER_AGENT_HEADER_NAME] = this.userAgent; - } - - if (this.accept) { - headers[ACCEPT_HEADER_NAME] = this.accept; - } +function buildRequestOptions( + client: WebClient, + options?: IGetFetchOptions | IFetchOptionsWithBody +): IRequestOptions { + const { + headers: optionsHeaders, + timeoutMs = 15 * 1000, + verb, + redirect, + body, + noDecode + } = (options as IFetchOptionsWithBody | undefined) ?? {}; + + const headers: Record = {}; + + WebClient.mergeHeaders(headers, client.standardHeaders); + + if (optionsHeaders) { + WebClient.mergeHeaders(headers, optionsHeaders); + } - let proxyUrl: string = ''; + if (client.userAgent) { + headers[USER_AGENT_HEADER_NAME] = client.userAgent; + } - switch (this.proxy) { - case WebClientProxy.Detect: - if (process.env.HTTPS_PROXY) { - proxyUrl = process.env.HTTPS_PROXY; - } else if (process.env.HTTP_PROXY) { - proxyUrl = process.env.HTTP_PROXY; - } - break; - - case WebClientProxy.Fiddler: - // For debugging, disable cert validation - // eslint-disable-next-line - process.env['NODE_TLS_REJECT_UNAUTHORIZED'] = '0'; - proxyUrl = 'http://localhost:8888/'; - break; - } + if (client.accept) { + headers[ACCEPT_HEADER_NAME] = client.accept; + } - let agent: HttpAgent | undefined = undefined; - if (proxyUrl) { - agent = createHttpsProxyAgent(proxyUrl); - } + let proxyUrl: string = ''; + + switch (client.proxy) { + case WebClientProxy.Detect: + if (process.env.HTTPS_PROXY) { + proxyUrl = process.env.HTTPS_PROXY; + } else if (process.env.HTTP_PROXY) { + proxyUrl = process.env.HTTP_PROXY; + } + break; + + case WebClientProxy.Fiddler: + // For debugging, disable cert validation + // eslint-disable-next-line + process.env['NODE_TLS_REJECT_UNAUTHORIZED'] = '0'; + proxyUrl = 'http://localhost:8888/'; + break; + } - return { - method: verb, - headers, - agent, - timeout: timeoutMs, - redirect, - body, - noDecode - }; + let agent: HttpAgent | undefined = undefined; + if (proxyUrl) { + agent = createHttpsProxyAgent(proxyUrl); } + + return { + method: verb, + headers, + agent, + timeout: timeoutMs, + redirect, + body, + noDecode + }; } diff --git a/rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts b/rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts index f0cd91e4d52..fec5deebdb1 100644 --- a/rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts +++ b/rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts @@ -119,7 +119,8 @@ export class BridgeCachePlugin implements IRushPlugin { { buildCacheConfiguration, terminal, - excludeAppleDoubleFiles: !!omitAppleDoubleFilesFromBuildCache + excludeAppleDoubleFiles: !!omitAppleDoubleFilesFromBuildCache, + useStreamingBuildCache: false } ); From d4c86b7669b22ffaa2abcff4130509eb61391d67 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 5 Apr 2026 21:41:42 +0000 Subject: [PATCH 15/45] DRY up duplicated code across WebClient, AmazonS3Client, and HttpBuildCacheProvider Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/c103fb17-b8b1-4d02-88ba-9db236e9f48f Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- libraries/rush-lib/src/utilities/WebClient.ts | 318 +++++++++--------- .../src/AmazonS3Client.ts | 212 ++++++------ .../src/HttpBuildCacheProvider.ts | 148 ++++---- 3 files changed, 337 insertions(+), 341 deletions(-) diff --git a/libraries/rush-lib/src/utilities/WebClient.ts b/libraries/rush-lib/src/utilities/WebClient.ts index 054c8fe1f81..c1dd9aa4794 100644 --- a/libraries/rush-lib/src/utilities/WebClient.ts +++ b/libraries/rush-lib/src/utilities/WebClient.ts @@ -95,134 +95,6 @@ const ACCEPT_HEADER_NAME: 'accept' = 'accept'; const USER_AGENT_HEADER_NAME: 'user-agent' = 'user-agent'; const CONTENT_ENCODING_HEADER_NAME: 'content-encoding' = 'content-encoding'; -const makeRequestAsync: FetchFn = async ( - url: string, - options: IRequestOptions, - redirected: boolean = false -) => { - const { body, redirect, noDecode } = options; - - return await new Promise( - (resolve: (result: IWebClientResponse) => void, reject: (error: Error) => void) => { - const parsedUrl: URL = typeof url === 'string' ? new URL(url) : url; - const requestFunction: typeof httpRequest | typeof httpsRequest = - parsedUrl.protocol === 'https:' ? httpsRequest : httpRequest; - - const req: ClientRequest = requestFunction(url, options, (response: IncomingMessage) => { - const responseBuffers: (Buffer | Uint8Array)[] = []; - response.on('data', (chunk: string | Buffer | Uint8Array) => { - responseBuffers.push(Buffer.from(chunk)); - }); - response.on('end', () => { - // Handle retries by calling the method recursively with the redirect URL - const statusCode: number | undefined = response.statusCode; - if (statusCode === 301 || statusCode === 302) { - switch (redirect) { - case 'follow': { - const redirectUrl: string | string[] | undefined = response.headers.location; - if (redirectUrl) { - makeRequestAsync(redirectUrl, options, true).then(resolve).catch(reject); - } else { - reject( - new Error(`Received status code ${response.statusCode} with no location header: ${url}`) - ); - } - - break; - } - case 'error': - reject(new Error(`Received status code ${response.statusCode}: ${url}`)); - return; - } - } - - const responseData: Buffer = Buffer.concat(responseBuffers); - const status: number = response.statusCode || 0; - const statusText: string | undefined = response.statusMessage; - const headers: Record = response.headers; - - let bodyString: string | undefined; - let bodyJson: unknown | undefined; - let decodedBuffer: Buffer | undefined; - const result: IWebClientResponse = { - ok: status >= 200 && status < 300, - status, - statusText, - redirected, - headers, - getTextAsync: async () => { - if (bodyString === undefined) { - const buffer: Buffer = await result.getBufferAsync(); - // eslint-disable-next-line require-atomic-updates - bodyString = buffer.toString(); - } - - return bodyString; - }, - getJsonAsync: async () => { - if (bodyJson === undefined) { - const text: string = await result.getTextAsync(); - // eslint-disable-next-line require-atomic-updates - bodyJson = JSON.parse(text); - } - - return bodyJson as TJson; - }, - getBufferAsync: async () => { - // Determine if the buffer is compressed and decode it if necessary - if (decodedBuffer === undefined) { - let encodings: string | string[] | undefined = headers[CONTENT_ENCODING_HEADER_NAME]; - if (!noDecode && encodings !== undefined) { - const zlib: typeof import('zlib') = await import('node:zlib'); - if (!Array.isArray(encodings)) { - encodings = encodings.split(','); - } - - let buffer: Buffer = responseData; - for (const encoding of encodings) { - let decompressFn: (buffer: Buffer, callback: import('zlib').CompressCallback) => void; - switch (encoding.trim()) { - case DEFLATE_ENCODING: { - decompressFn = zlib.inflate.bind(zlib); - break; - } - case GZIP_ENCODING: { - decompressFn = zlib.gunzip.bind(zlib); - break; - } - case BROTLI_ENCODING: { - decompressFn = zlib.brotliDecompress.bind(zlib); - break; - } - default: { - throw new Error(`Unsupported content-encoding: ${encodings}`); - } - } - - buffer = await LegacyAdapters.convertCallbackToPromise(decompressFn, buffer); - } - - // eslint-disable-next-line require-atomic-updates - decodedBuffer = buffer; - } else { - decodedBuffer = responseData; - } - } - - return decodedBuffer; - } - }; - resolve(result); - }); - }).on('error', (error: Error) => { - reject(error); - }); - - _sendRequestBody(req, body, reject); - } - ); -}; - type StreamFetchFn = ( url: string, options: IRequestOptions, @@ -230,18 +102,27 @@ type StreamFetchFn = ( ) => Promise; /** - * Makes an HTTP request that resolves as soon as headers are received, providing the - * response body as a readable stream. This avoids buffering the entire response in memory. + * Shared HTTP request core used by both buffer-based and streaming request functions. + * Handles URL parsing, protocol selection, redirect following, body sending, and error handling. + * The `handleResponse` callback is responsible for processing the response and calling + * `resolve`/`reject` to complete the outer promise. */ -const makeStreamRequestAsync: StreamFetchFn = async ( +function _makeRawRequestAsync( url: string, options: IRequestOptions, - redirected: boolean = false -) => { + redirected: boolean, + handleResponse: ( + response: IncomingMessage, + redirected: boolean, + resolve: (result: TResponse | PromiseLike) => void, + reject: (error: Error) => void + ) => void, + selfFn: (url: string, options: IRequestOptions, isRedirect?: boolean) => Promise +): Promise { const { body, redirect } = options; - return await new Promise( - (resolve: (result: IWebClientStreamResponse) => void, reject: (error: Error) => void) => { + return new Promise( + (resolve: (result: TResponse | PromiseLike) => void, reject: (error: Error) => void) => { const parsedUrl: URL = typeof url === 'string' ? new URL(url) : url; const requestFunction: typeof httpRequest | typeof httpsRequest = parsedUrl.protocol === 'https:' ? httpsRequest : httpRequest; @@ -249,15 +130,14 @@ const makeStreamRequestAsync: StreamFetchFn = async ( const req: ClientRequest = requestFunction(url, options, (response: IncomingMessage) => { const statusCode: number | undefined = response.statusCode; - // Handle redirects if (statusCode === 301 || statusCode === 302) { - // Consume/drain the redirect response before following + // Drain the redirect response before following response.resume(); switch (redirect) { case 'follow': { const redirectUrl: string | string[] | undefined = response.headers.location; - if (redirectUrl) { - makeStreamRequestAsync(redirectUrl, options, true).then(resolve).catch(reject); + if (typeof redirectUrl === 'string') { + resolve(selfFn(redirectUrl, options, true)); } else { reject( new Error(`Received status code ${response.statusCode} with no location header: ${url}`) @@ -271,40 +151,156 @@ const makeStreamRequestAsync: StreamFetchFn = async ( } } + handleResponse(response, redirected, resolve, reject); + }).on('error', (error: Error) => { + reject(error); + }); + + const isStream: boolean = !!body && typeof (body as Readable).pipe === 'function'; + if (isStream) { + (body as Readable).on('error', reject); + (body as Readable).pipe(req); + } else { + req.end(body as Buffer | undefined); + } + } + ); +} + +const makeRequestAsync: FetchFn = async ( + url: string, + options: IRequestOptions, + redirected: boolean = false +) => { + const { noDecode } = options; + + return _makeRawRequestAsync( + url, + options, + redirected, + ( + response: IncomingMessage, + wasRedirected: boolean, + resolve: (result: IWebClientResponse | PromiseLike) => void + ): void => { + const responseBuffers: (Buffer | Uint8Array)[] = []; + response.on('data', (chunk: string | Buffer | Uint8Array) => { + responseBuffers.push(Buffer.from(chunk)); + }); + response.on('end', () => { + const responseData: Buffer = Buffer.concat(responseBuffers); const status: number = response.statusCode || 0; const statusText: string | undefined = response.statusMessage; const headers: Record = response.headers; - resolve({ + let bodyString: string | undefined; + let bodyJson: unknown | undefined; + let decodedBuffer: Buffer | undefined; + const result: IWebClientResponse = { ok: status >= 200 && status < 300, status, statusText, - redirected, + redirected: wasRedirected, headers, - stream: response - }); - }).on('error', (error: Error) => { - reject(error); - }); + getTextAsync: async () => { + if (bodyString === undefined) { + const buffer: Buffer = await result.getBufferAsync(); + // eslint-disable-next-line require-atomic-updates + bodyString = buffer.toString(); + } - _sendRequestBody(req, body, reject); - } + return bodyString; + }, + getJsonAsync: async () => { + if (bodyJson === undefined) { + const text: string = await result.getTextAsync(); + // eslint-disable-next-line require-atomic-updates + bodyJson = JSON.parse(text); + } + + return bodyJson as TJson; + }, + getBufferAsync: async () => { + // Determine if the buffer is compressed and decode it if necessary + if (decodedBuffer === undefined) { + let encodings: string | string[] | undefined = headers[CONTENT_ENCODING_HEADER_NAME]; + if (!noDecode && encodings !== undefined) { + const zlib: typeof import('zlib') = await import('node:zlib'); + if (!Array.isArray(encodings)) { + encodings = encodings.split(','); + } + + let buffer: Buffer = responseData; + for (const encoding of encodings) { + let decompressFn: (buffer: Buffer, callback: import('zlib').CompressCallback) => void; + switch (encoding.trim()) { + case DEFLATE_ENCODING: { + decompressFn = zlib.inflate.bind(zlib); + break; + } + case GZIP_ENCODING: { + decompressFn = zlib.gunzip.bind(zlib); + break; + } + case BROTLI_ENCODING: { + decompressFn = zlib.brotliDecompress.bind(zlib); + break; + } + default: { + throw new Error(`Unsupported content-encoding: ${encodings}`); + } + } + + buffer = await LegacyAdapters.convertCallbackToPromise(decompressFn, buffer); + } + + // eslint-disable-next-line require-atomic-updates + decodedBuffer = buffer; + } else { + decodedBuffer = responseData; + } + } + + return decodedBuffer; + } + }; + resolve(result); + }); + }, + makeRequestAsync ); }; -function _sendRequestBody( - req: ClientRequest, - body: Buffer | Readable | undefined, - reject: (error: Error) => void -): void { - const isStream: boolean = !!body && typeof (body as Readable).pipe === 'function'; - if (isStream) { - (body as Readable).on('error', reject); - (body as Readable).pipe(req); - } else { - req.end(body as Buffer | undefined); - } -} +const makeStreamRequestAsync: StreamFetchFn = async ( + url: string, + options: IRequestOptions, + redirected: boolean = false +) => { + return _makeRawRequestAsync( + url, + options, + redirected, + ( + response: IncomingMessage, + wasRedirected: boolean, + resolve: (result: IWebClientStreamResponse | PromiseLike) => void + ): void => { + const status: number = response.statusCode || 0; + const statusText: string | undefined = response.statusMessage; + const headers: Record = response.headers; + + resolve({ + ok: status >= 200 && status < 300, + status, + statusText, + redirected: wasRedirected, + headers, + stream: response + }); + }, + makeStreamRequestAsync + ); +}; /** * A helper for issuing HTTP requests. diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts index f01e76b7b18..bada5bde201 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts @@ -128,42 +128,14 @@ export class AmazonS3Client { public async getObjectAsync(objectName: string): Promise { this._writeDebugLine('Reading object from S3'); return await this._sendCacheRequestWithRetriesAsync(async () => { - const response: IWebClientResponse = await this._makeRequestAsync('GET', objectName); - if (response.ok) { - return { - hasNetworkError: false, - response: await response.getBufferAsync() - }; - } else if (response.status === 404) { - return { - hasNetworkError: false, - response: undefined - }; - } else if ( - (response.status === 400 || response.status === 401 || response.status === 403) && - !this._credentials - ) { - // unauthorized due to not providing credentials, - // silence error for better DX when e.g. running locally without credentials - this._writeWarningLine( - `No credentials found and received a ${response.status}`, - ' response code from the cloud storage.', - ' Maybe run rush update-cloud-credentials', - ' or set the RUSH_BUILD_CACHE_CREDENTIAL env' - ); - return { - hasNetworkError: false, - response: undefined - }; - } else if (response.status === 400 || response.status === 401 || response.status === 403) { - throw await this._getS3ErrorAsync(response); - } else { - const error: Error = await this._getS3ErrorAsync(response); - return { - hasNetworkError: true, - error - }; - } + const response: IWebClientResponse = await this._makeSignedRequestAsync('GET', objectName); + return this._handleGetResponseAsync( + response.status, + response.statusText, + response.ok, + async () => await response.getBufferAsync(), + async () => await this._getS3ErrorAsync(response) + ); }); } @@ -173,7 +145,11 @@ export class AmazonS3Client { } await this._sendCacheRequestWithRetriesAsync(async () => { - const response: IWebClientResponse = await this._makeRequestAsync('PUT', objectName, objectBuffer); + const response: IWebClientResponse = await this._makeSignedRequestAsync( + 'PUT', + objectName, + objectBuffer + ); if (!response.ok) { return { hasNetworkError: true, @@ -190,47 +166,25 @@ export class AmazonS3Client { public async getObjectStreamAsync(objectName: string): Promise { this._writeDebugLine('Reading object stream from S3'); return await this._sendCacheRequestWithRetriesAsync(async () => { - const response: IWebClientStreamResponse = await this._makeStreamRequestAsync('GET', objectName); - if (response.ok) { - return { - hasNetworkError: false, - response: response.stream - }; - } else if (response.status === 404) { - response.stream.resume(); - return { - hasNetworkError: false, - response: undefined - }; - } else if ( - (response.status === 400 || response.status === 401 || response.status === 403) && - !this._credentials - ) { - response.stream.resume(); - this._writeWarningLine( - `No credentials found and received a ${response.status}`, - ' response code from the cloud storage.', - ' Maybe run rush update-cloud-credentials', - ' or set the RUSH_BUILD_CACHE_CREDENTIAL env' - ); - return { - hasNetworkError: false, - response: undefined - }; - } else if (response.status === 400 || response.status === 401 || response.status === 403) { - response.stream.resume(); - throw new Error( - `Amazon S3 responded with status code ${response.status} (${response.statusText})` - ); - } else { - response.stream.resume(); - return { - hasNetworkError: true, - error: new Error( + const response: IWebClientStreamResponse = await this._makeSignedRequestAsync( + 'GET', + objectName, + undefined, + true + ); + return this._handleGetResponseAsync( + response.status, + response.statusText, + response.ok, + () => response.stream, + async () => { + response.stream.resume(); + return new Error( `Amazon S3 responded with status code ${response.status} (${response.statusText})` - ) - }; - } + ); + }, + () => response.stream.resume() + ); }); } @@ -245,10 +199,11 @@ export class AmazonS3Client { } // Streaming uploads cannot be retried because the stream is consumed after the first attempt. - const response: IWebClientStreamResponse = await this._makeStreamRequestAsync( + const response: IWebClientStreamResponse = await this._makeSignedRequestAsync( 'PUT', objectName, - objectStream + objectStream, + true ); if (!response.ok) { response.stream.resume(); @@ -277,34 +232,83 @@ export class AmazonS3Client { } } - private async _makeRequestAsync( - verb: 'GET' | 'PUT', - objectName: string, - body?: Buffer - ): Promise { - const bodyHash: string = this._getSha256(body); - const { url, headers } = this._buildSignedRequest(verb, objectName, bodyHash); - - const webFetchOptions: IGetFetchOptions | IFetchOptionsWithBody = { - verb, - headers - }; - if (verb === 'PUT') { - (webFetchOptions as IFetchOptionsWithBody).body = body; + /** + * Shared response handling for GET requests (both buffer and stream). + * The `getSuccessResult` callback extracts the response payload (Buffer or Readable). + * The `getError` callback constructs an error from the response. + * The optional `cleanup` callback drains stream responses on non-success paths. + */ + private async _handleGetResponseAsync( + status: number, + statusText: string | undefined, + ok: boolean, + getSuccessResult: () => T | Promise, + getError: () => Promise, + cleanup?: () => void + ): Promise> { + if (ok) { + return { + hasNetworkError: false, + response: await getSuccessResult() + }; + } else if (status === 404) { + cleanup?.(); + return { + hasNetworkError: false, + response: undefined + }; + } else if ( + (status === 400 || status === 401 || status === 403) && + !this._credentials + ) { + cleanup?.(); + // unauthorized due to not providing credentials, + // silence error for better DX when e.g. running locally without credentials + this._writeWarningLine( + `No credentials found and received a ${status}`, + ' response code from the cloud storage.', + ' Maybe run rush update-cloud-credentials', + ' or set the RUSH_BUILD_CACHE_CREDENTIAL env' + ); + return { + hasNetworkError: false, + response: undefined + }; + } else if (status === 400 || status === 401 || status === 403) { + cleanup?.(); + throw await getError(); + } else { + cleanup?.(); + const error: Error = await getError(); + return { + hasNetworkError: true, + error + }; } - - const response: IWebClientResponse = await this._webClient.fetchAsync(url, webFetchOptions); - - return response; } - private async _makeStreamRequestAsync( + private async _makeSignedRequestAsync( verb: 'GET' | 'PUT', objectName: string, - body?: Readable - ): Promise { + body?: Buffer + ): Promise; + private async _makeSignedRequestAsync( + verb: 'GET' | 'PUT', + objectName: string, + body: Readable | undefined, + stream: true + ): Promise; + private async _makeSignedRequestAsync( + verb: 'GET' | 'PUT', + objectName: string, + body?: Buffer | Readable, + stream?: boolean + ): Promise { // For streaming uploads, the body cannot be hashed upfront, so we use UNSIGNED-PAYLOAD. - const bodyHash: string = body ? UNSIGNED_PAYLOAD : this._getSha256(undefined); + const isStreamBody: boolean = !!body && typeof (body as Readable).pipe === 'function'; + const bodyHash: string = isStreamBody + ? UNSIGNED_PAYLOAD + : this._getSha256(body as Buffer | undefined); const { url, headers } = this._buildSignedRequest(verb, objectName, bodyHash); const webFetchOptions: IGetFetchOptions | IFetchOptionsWithBody = { @@ -315,7 +319,11 @@ export class AmazonS3Client { (webFetchOptions as IFetchOptionsWithBody).body = body; } - return await this._webClient.fetchStreamAsync(url, webFetchOptions); + if (stream) { + return await this._webClient.fetchStreamAsync(url, webFetchOptions); + } else { + return await this._webClient.fetchAsync(url, webFetchOptions); + } } /** diff --git a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts index c915fd56c16..11c0d0ffadd 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts @@ -14,6 +14,8 @@ import { } from '@rushstack/rush-sdk'; import { WebClient, + type IGetFetchOptions, + type IFetchOptionsWithBody, type IWebClientResponse, type IWebClientStreamResponse } from '@rushstack/rush-sdk/lib/utilities/WebClient'; @@ -294,74 +296,20 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { maxAttempts: number; credentialOptions?: CredentialsOptions; }): Promise { - const { terminal, relUrl, method, body, warningText, readBody, credentialOptions } = options; - const safeCredentialOptions: CredentialsOptions = credentialOptions ?? CredentialsOptions.Optional; - const credentials: string | undefined = await this._tryGetCredentialsAsync(safeCredentialOptions); - const url: string = new URL(relUrl, this._url).href; - - const headers: Record = {}; - if (typeof credentials === 'string') { - headers.Authorization = credentials; - } - - for (const [key, value] of Object.entries(this._headers)) { - if (typeof value === 'string') { - headers[key] = value; - } - } - - const bodyLength: number | 'unknown' = (body as { length: number })?.length || 'unknown'; - - terminal.writeDebugLine(`[http-build-cache] request: ${method} ${url} ${bodyLength} bytes`); - - const webClient: WebClient = new WebClient(); - const response: IWebClientResponse = await webClient.fetchAsync(url, { - verb: method, - headers: headers, - body: body, - redirect: 'follow', - timeoutMs: 0 // Use the default timeout - }); - - if (!response.ok) { - const isNonCredentialResponse: boolean = response.status >= 500 && response.status < 600; - - if ( - !isNonCredentialResponse && - typeof credentials !== 'string' && - safeCredentialOptions === CredentialsOptions.Optional - ) { - // If we don't already have credentials yet, and we got a response from the server - // that is a "normal" failure (4xx), then we assume that credentials are probably - // required. Re-attempt the request, requiring credentials this time. - // - // This counts as part of the "first attempt", so it is not included in the max attempts - return await this._makeHttpRequestAsync({ - ...options, - credentialOptions: CredentialsOptions.Required - }); - } - - if (options.maxAttempts > 1) { - // Pause a bit before retrying in case the server is busy - // Add some random jitter to the retry so we can spread out load on the remote service - // A proper solution might add exponential back off in case the retry count is high (10 or more) - const factor: number = 1.0 + Math.random(); // A random number between 1.0 and 2.0 - const retryDelay: number = Math.floor(factor * this._minHttpRetryDelayMs); - - await Async.sleepAsync(retryDelay); - - return await this._makeHttpRequestAsync({ ...options, maxAttempts: options.maxAttempts - 1 }); - } - - this._reportFailure(terminal, method, response, false, warningText); + const { readBody } = options; + // The stream: false flag ensures the response is an IWebClientResponse + const response: IWebClientResponse | false = (await this._makeHttpCoreRequestAsync({ + ...options, + stream: false + })) as IWebClientResponse | false; + + if (response === false) { return false; } const result: Buffer | boolean = readBody ? await response.getBufferAsync() : true; - - terminal.writeDebugLine( - `[http-build-cache] actual response: ${response.status} ${url} ${ + options.terminal.writeDebugLine( + `[http-build-cache] actual response: ${response.status} ${new URL(options.relUrl, this._url).href} ${ result === true ? 'true' : result.length } bytes` ); @@ -378,7 +326,38 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { maxAttempts: number; credentialOptions?: CredentialsOptions; }): Promise { - const { terminal, relUrl, method, body, warningText, credentialOptions } = options; + // The stream: true flag ensures the response is an IWebClientStreamResponse + const response: IWebClientStreamResponse | false = (await this._makeHttpCoreRequestAsync({ + ...options, + stream: true + })) as IWebClientStreamResponse | false; + + if (response === false) { + return false; + } + + options.terminal.writeDebugLine( + `[http-build-cache] stream response: ${response.status} ${new URL(options.relUrl, this._url).href}` + ); + + return response; + } + + /** + * Shared request core for both buffer-based and streaming HTTP requests. + * Handles credentials resolution, header construction, retry logic, and failure reporting. + */ + private async _makeHttpCoreRequestAsync(options: { + terminal: ITerminal; + relUrl: string; + method: 'GET' | UploadMethod; + body: Buffer | Readable | undefined; + warningText: string; + maxAttempts: number; + credentialOptions?: CredentialsOptions; + stream: boolean; + }): Promise { + const { terminal, relUrl, method, body, warningText, credentialOptions, stream } = options; const safeCredentialOptions: CredentialsOptions = credentialOptions ?? CredentialsOptions.Optional; const credentials: string | undefined = await this._tryGetCredentialsAsync(safeCredentialOptions); const url: string = new URL(relUrl, this._url).href; @@ -394,20 +373,28 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { } } - terminal.writeDebugLine(`[http-build-cache] stream request: ${method} ${url}`); + const bodyLength: number | string = (body as Buffer | undefined)?.length ?? 'unknown'; + + terminal.writeDebugLine(`[http-build-cache] request: ${method} ${url} ${bodyLength} bytes`); const webClient: WebClient = new WebClient(); - const response: IWebClientStreamResponse = await webClient.fetchStreamAsync(url, { + const fetchOptions: IGetFetchOptions | IFetchOptionsWithBody = { verb: method, headers: headers, body: body, redirect: 'follow', timeoutMs: 0 // Use the default timeout - }); + }; + + const response: IWebClientResponse | IWebClientStreamResponse = stream + ? await webClient.fetchStreamAsync(url, fetchOptions) + : await webClient.fetchAsync(url, fetchOptions); if (!response.ok) { - // Drain the response body so the connection can be reused - response.stream.resume(); + // Drain the response body on stream responses so the connection can be reused + if ('stream' in response) { + response.stream.resume(); + } const isNonCredentialResponse: boolean = response.status >= 500 && response.status < 600; @@ -416,28 +403,33 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { typeof credentials !== 'string' && safeCredentialOptions === CredentialsOptions.Optional ) { - return await this._makeHttpStreamRequestAsync({ + // If we don't already have credentials yet, and we got a response from the server + // that is a "normal" failure (4xx), then we assume that credentials are probably + // required. Re-attempt the request, requiring credentials this time. + // + // This counts as part of the "first attempt", so it is not included in the max attempts + return await this._makeHttpCoreRequestAsync({ ...options, credentialOptions: CredentialsOptions.Required }); } if (options.maxAttempts > 1) { - const factor: number = 1.0 + Math.random(); + // Pause a bit before retrying in case the server is busy + // Add some random jitter to the retry so we can spread out load on the remote service + // A proper solution might add exponential back off in case the retry count is high (10 or more) + const factor: number = 1.0 + Math.random(); // A random number between 1.0 and 2.0 const retryDelay: number = Math.floor(factor * this._minHttpRetryDelayMs); + await Async.sleepAsync(retryDelay); - return await this._makeHttpStreamRequestAsync({ - ...options, - maxAttempts: options.maxAttempts - 1 - }); + + return await this._makeHttpCoreRequestAsync({ ...options, maxAttempts: options.maxAttempts - 1 }); } this._reportFailure(terminal, method, response, false, warningText); return false; } - terminal.writeDebugLine(`[http-build-cache] stream response: ${response.status} ${url}`); - return response; } From 125408455a3bec5768d5957491632ebfe24e5f16 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 5 Apr 2026 21:46:09 +0000 Subject: [PATCH 16/45] Address code review: use Buffer.isBuffer for clearer body length check Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/c103fb17-b8b1-4d02-88ba-9db236e9f48f Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- .../rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts index 11c0d0ffadd..61c3a4adec7 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts @@ -373,7 +373,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { } } - const bodyLength: number | string = (body as Buffer | undefined)?.length ?? 'unknown'; + const bodyLength: number | string = Buffer.isBuffer(body) ? body.length : 'unknown'; terminal.writeDebugLine(`[http-build-cache] request: ${method} ${url} ${bodyLength} bytes`); From e0018558bd6125e8458743edab998fd64cf5b29a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Apr 2026 00:03:20 +0000 Subject: [PATCH 17/45] Change streaming cache APIs from Readable to NodeJS.ReadableStream Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/66d5d3e1-e14b-41bb-9382-46fc4176f977 Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- .../api/rush-amazon-s3-build-cache-plugin.api.md | 5 ++--- common/reviews/api/rush-lib.api.md | 7 +++---- .../buildCache/FileSystemBuildCacheProvider.ts | 3 +-- .../logic/buildCache/ICloudBuildCacheProvider.ts | 9 +++++---- .../src/logic/buildCache/OperationBuildCache.ts | 3 +-- .../src/AmazonS3BuildCacheProvider.ts | 6 +++--- .../src/AmazonS3Client.ts | 9 ++++++--- .../src/AzureStorageBuildCacheProvider.ts | 16 ++++------------ .../src/HttpBuildCacheProvider.ts | 6 +++--- 9 files changed, 28 insertions(+), 36 deletions(-) diff --git a/common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md b/common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md index 14c9b2f6445..1c390c5eae8 100644 --- a/common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md +++ b/common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md @@ -8,7 +8,6 @@ import type { IRushPlugin } from '@rushstack/rush-sdk'; import { ITerminal } from '@rushstack/terminal'; -import type { Readable } from 'node:stream'; import type { RushConfiguration } from '@rushstack/rush-sdk'; import type { RushSession } from '@rushstack/rush-sdk'; import { WebClient } from '@rushstack/rush-sdk/lib/utilities/WebClient'; @@ -19,7 +18,7 @@ export class AmazonS3Client { // (undocumented) getObjectAsync(objectName: string): Promise; // (undocumented) - getObjectStreamAsync(objectName: string): Promise; + getObjectStreamAsync(objectName: string): Promise; // (undocumented) _getSha256Hmac(key: string | Buffer, data: string): Buffer; // (undocumented) @@ -28,7 +27,7 @@ export class AmazonS3Client { static tryDeserializeCredentials(credentialString: string | undefined): IAmazonS3Credentials | undefined; // (undocumented) uploadObjectAsync(objectName: string, objectBuffer: Buffer): Promise; - uploadObjectStreamAsync(objectName: string, objectStream: Readable): Promise; + uploadObjectStreamAsync(objectName: string, objectStream: NodeJS.ReadableStream): Promise; // (undocumented) static UriEncode(input: string): string; } diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index 5034164d89f..f452d1ab4a6 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -28,7 +28,6 @@ import { JsonObject } from '@rushstack/node-core-library'; import { LookupByPath } from '@rushstack/lookup-by-path'; import { PackageNameParser } from '@rushstack/node-core-library'; import type { PerformanceEntry as PerformanceEntry_2 } from 'node:perf_hooks'; -import type { Readable } from 'node:stream'; import type { StdioSummarizer } from '@rushstack/terminal'; import { SyncHook } from 'tapable'; import { SyncWaterfallHook } from 'tapable'; @@ -316,7 +315,7 @@ export class FileSystemBuildCacheProvider { getCacheEntryPath(cacheId: string): string; tryGetCacheEntryPathByIdAsync(terminal: ITerminal, cacheId: string): Promise; trySetCacheEntryBufferAsync(terminal: ITerminal, cacheId: string, entryBuffer: Buffer): Promise; - trySetCacheEntryStreamAsync(terminal: ITerminal, cacheId: string, entryStream: Readable): Promise; + trySetCacheEntryStreamAsync(terminal: ITerminal, cacheId: string, entryStream: NodeJS.ReadableStream): Promise; } // @internal @@ -349,10 +348,10 @@ export interface ICloudBuildCacheProvider { readonly isCacheWriteAllowed: boolean; // (undocumented) tryGetCacheEntryBufferByIdAsync(terminal: ITerminal, cacheId: string): Promise; - tryGetCacheEntryStreamByIdAsync?(terminal: ITerminal, cacheId: string): Promise; + tryGetCacheEntryStreamByIdAsync?(terminal: ITerminal, cacheId: string): Promise; // (undocumented) trySetCacheEntryBufferAsync(terminal: ITerminal, cacheId: string, entryBuffer: Buffer): Promise; - trySetCacheEntryStreamAsync?(terminal: ITerminal, cacheId: string, entryStream: Readable): Promise; + trySetCacheEntryStreamAsync?(terminal: ITerminal, cacheId: string, entryStream: NodeJS.ReadableStream): Promise; // (undocumented) updateCachedCredentialAsync(terminal: ITerminal, credential: string): Promise; // (undocumented) diff --git a/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts b/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts index 5b73a5916b6..7c0ee646489 100644 --- a/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts +++ b/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts @@ -3,7 +3,6 @@ import * as path from 'node:path'; import { pipeline } from 'node:stream/promises'; -import type { Readable } from 'node:stream'; import { FileSystem, type FileSystemWriteStream } from '@rushstack/node-core-library'; import type { ITerminal } from '@rushstack/terminal'; @@ -85,7 +84,7 @@ export class FileSystemBuildCacheProvider { public async trySetCacheEntryStreamAsync( terminal: ITerminal, cacheId: string, - entryStream: Readable + entryStream: NodeJS.ReadableStream ): Promise { const cacheEntryFilePath: string = this.getCacheEntryPath(cacheId); const writeStream: FileSystemWriteStream = await FileSystem.createWriteStreamAsync(cacheEntryFilePath, { diff --git a/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts b/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts index 9fee1f86125..571de7db90a 100644 --- a/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts +++ b/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import type { Readable } from 'node:stream'; - import type { ITerminal } from '@rushstack/terminal'; /** @@ -19,7 +17,10 @@ export interface ICloudBuildCacheProvider { * {@link ICloudBuildCacheProvider.tryGetCacheEntryBufferByIdAsync} to avoid loading the entire * cache entry into memory. */ - tryGetCacheEntryStreamByIdAsync?(terminal: ITerminal, cacheId: string): Promise; + tryGetCacheEntryStreamByIdAsync?( + terminal: ITerminal, + cacheId: string + ): Promise; /** * If implemented, the build cache will prefer to use this method over * {@link ICloudBuildCacheProvider.trySetCacheEntryBufferAsync} to avoid loading the entire @@ -33,7 +34,7 @@ export interface ICloudBuildCacheProvider { trySetCacheEntryStreamAsync?( terminal: ITerminal, cacheId: string, - entryStream: Readable + entryStream: NodeJS.ReadableStream ): Promise; updateCachedCredentialAsync(terminal: ITerminal, credential: string): Promise; diff --git a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts index 06796c95482..f26a381ddcf 100644 --- a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts +++ b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts @@ -3,7 +3,6 @@ import * as path from 'node:path'; import * as crypto from 'node:crypto'; -import type { Readable } from 'node:stream'; import { FileSystem, @@ -180,7 +179,7 @@ export class OperationBuildCache { this._cloudBuildCacheProvider.tryGetCacheEntryStreamByIdAsync ) { // Use streaming path to avoid loading the entire cache entry into memory - const cacheEntryStream: Readable | undefined = + const cacheEntryStream: NodeJS.ReadableStream | undefined = await this._cloudBuildCacheProvider.tryGetCacheEntryStreamByIdAsync(terminal, cacheId); if (cacheEntryStream) { try { diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts index 30afacb2f84..9305a070f36 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts @@ -187,7 +187,7 @@ export class AmazonS3BuildCacheProvider implements ICloudBuildCacheProvider { public async tryGetCacheEntryStreamByIdAsync( terminal: ITerminal, cacheId: string - ): Promise { + ): Promise { try { const client: AmazonS3Client = await this._getS3ClientAsync(terminal); return await client.getObjectStreamAsync(this._s3Prefix ? `${this._s3Prefix}/${cacheId}` : cacheId); @@ -200,7 +200,7 @@ export class AmazonS3BuildCacheProvider implements ICloudBuildCacheProvider { public async trySetCacheEntryStreamAsync( terminal: ITerminal, cacheId: string, - entryStream: Readable + entryStream: NodeJS.ReadableStream ): Promise { if (!this.isCacheWriteAllowed) { terminal.writeErrorLine('Writing to S3 cache is not allowed in the current configuration.'); @@ -213,7 +213,7 @@ export class AmazonS3BuildCacheProvider implements ICloudBuildCacheProvider { const client: AmazonS3Client = await this._getS3ClientAsync(terminal); await client.uploadObjectStreamAsync( this._s3Prefix ? `${this._s3Prefix}/${cacheId}` : cacheId, - entryStream + entryStream as Readable ); return true; } catch (e) { diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts index bada5bde201..ab85d24a41e 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts @@ -163,7 +163,7 @@ export class AmazonS3Client { }); } - public async getObjectStreamAsync(objectName: string): Promise { + public async getObjectStreamAsync(objectName: string): Promise { this._writeDebugLine('Reading object stream from S3'); return await this._sendCacheRequestWithRetriesAsync(async () => { const response: IWebClientStreamResponse = await this._makeSignedRequestAsync( @@ -193,7 +193,10 @@ export class AmazonS3Client { * does not use retry logic because the stream is consumed after the first attempt and cannot be * replayed. The caller should handle failures accordingly. */ - public async uploadObjectStreamAsync(objectName: string, objectStream: Readable): Promise { + public async uploadObjectStreamAsync( + objectName: string, + objectStream: NodeJS.ReadableStream + ): Promise { if (!this._credentials) { throw new Error('Credentials are required to upload objects to S3.'); } @@ -202,7 +205,7 @@ export class AmazonS3Client { const response: IWebClientStreamResponse = await this._makeSignedRequestAsync( 'PUT', objectName, - objectStream, + objectStream as Readable, true ); if (!response.ok) { diff --git a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts index 72925454eeb..cf7b5feccb9 100644 --- a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts +++ b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts @@ -196,22 +196,14 @@ export class AzureStorageBuildCacheProvider public async tryGetCacheEntryStreamByIdAsync( terminal: ITerminal, cacheId: string - ): Promise { + ): Promise { const blobClient: BlobClient = await this._getBlobClientForCacheIdAsync(cacheId, terminal); try { const blobExists: boolean = await blobClient.exists(); if (blobExists) { const downloadResponse: BlobDownloadResponseParsed = await blobClient.download(); - const readableStreamBody: NodeJS.ReadableStream | undefined = - downloadResponse.readableStreamBody; - if (readableStreamBody) { - // The Azure SDK types readableStreamBody as NodeJS.ReadableStream, which is a minimal - // interface. In practice, the underlying implementation is a Node.js Readable stream - // (http.IncomingMessage), so this assertion is safe. - return readableStreamBody as unknown as Readable; - } - return undefined; + return downloadResponse.readableStreamBody; } else { return undefined; } @@ -224,7 +216,7 @@ export class AzureStorageBuildCacheProvider public async trySetCacheEntryStreamAsync( terminal: ITerminal, cacheId: string, - entryStream: Readable + entryStream: NodeJS.ReadableStream ): Promise { if (!this.isCacheWriteAllowed) { terminal.writeErrorLine( @@ -258,7 +250,7 @@ export class AzureStorageBuildCacheProvider return true; } else { try { - await blockBlobClient.uploadStream(entryStream); + await blockBlobClient.uploadStream(entryStream as Readable); return true; } catch (e) { if ((e as IBlobError).statusCode === 409 /* conflict */) { diff --git a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts index 61c3a4adec7..92e73becac1 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts @@ -160,7 +160,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { public async tryGetCacheEntryStreamByIdAsync( terminal: ITerminal, cacheId: string - ): Promise { + ): Promise { try { const result: IWebClientStreamResponse | false = await this._makeHttpStreamRequestAsync({ terminal: terminal, @@ -181,7 +181,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { public async trySetCacheEntryStreamAsync( terminal: ITerminal, cacheId: string, - entryStream: Readable + entryStream: NodeJS.ReadableStream ): Promise { if (!this.isCacheWriteAllowed) { terminal.writeErrorLine('Writing to cache is not allowed in the current configuration.'); @@ -195,7 +195,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { terminal: terminal, relUrl: `${this._cacheKeyPrefix}${cacheId}`, method: this._uploadMethod, - body: entryStream, + body: entryStream as Readable, warningText: 'Could not write cache entry', // Streaming uploads cannot be retried because the stream is consumed maxAttempts: 1 From 81af1e19a3cb26d8ef27f6bfd584af8bef6c27d8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Apr 2026 00:16:53 +0000 Subject: [PATCH 18/45] DRY up Azure/HTTP cache providers, fix bridge plugin experiment config Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/b57084f4-36d7-42f3-8876-7298756b887e Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- .../src/AzureStorageBuildCacheProvider.ts | 174 ++++++------------ .../src/BridgeCachePlugin.ts | 4 +- .../src/HttpBuildCacheProvider.ts | 24 ++- 3 files changed, 77 insertions(+), 125 deletions(-) diff --git a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts index cf7b5feccb9..233a5a10205 100644 --- a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts +++ b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts @@ -78,60 +78,9 @@ export class AzureStorageBuildCacheProvider terminal: ITerminal, cacheId: string ): Promise { - const blobClient: BlobClient = await this._getBlobClientForCacheIdAsync(cacheId, terminal); - - try { - const blobExists: boolean = await blobClient.exists(); - if (blobExists) { - return await blobClient.downloadToBuffer(); - } else { - return undefined; - } - } catch (err) { - const e: IBlobError = err as IBlobError; - const errorMessage: string = - 'Error getting cache entry from Azure Storage: ' + - [e.name, e.message, e.response?.status, e.response?.parsedHeaders?.errorCode] - .filter((piece: string | undefined) => piece) - .join(' '); - - if (e.response?.parsedHeaders?.errorCode === 'PublicAccessNotPermitted') { - // This error means we tried to read the cache with no credentials, but credentials are required. - // We'll assume that the configuration of the cache is correct and the user has to take action. - terminal.writeWarningLine( - `${errorMessage}\n\n` + - `You need to configure Azure Storage SAS credentials to access the build cache.\n` + - `Update the credentials by running "rush ${RushConstants.updateCloudCredentialsCommandName}", \n` + - `or provide a SAS in the ` + - `${EnvironmentVariableNames.RUSH_BUILD_CACHE_CREDENTIAL} environment variable.` - ); - } else if (e.response?.parsedHeaders?.errorCode === 'AuthenticationFailed') { - // This error means the user's credentials are incorrect, but not expired normally. They might have - // gotten corrupted somehow, or revoked manually in Azure Portal. - terminal.writeWarningLine( - `${errorMessage}\n\n` + - `Your Azure Storage SAS credentials are not valid.\n` + - `Update the credentials by running "rush ${RushConstants.updateCloudCredentialsCommandName}", \n` + - `or provide a SAS in the ` + - `${EnvironmentVariableNames.RUSH_BUILD_CACHE_CREDENTIAL} environment variable.` - ); - } else if (e.response?.parsedHeaders?.errorCode === 'AuthorizationPermissionMismatch') { - // This error is not solvable by the user, so we'll assume it is a configuration error, and revert - // to providing likely next steps on configuration. (Hopefully this error is rare for a regular - // developer, more likely this error will appear while someone is configuring the cache for the - // first time.) - terminal.writeWarningLine( - `${errorMessage}\n\n` + - `Your Azure Storage SAS credentials are valid, but do not have permission to read the build cache.\n` + - `Make sure you have added the role 'Storage Blob Data Reader' to the appropriate user(s) or group(s)\n` + - `on your storage account in the Azure Portal.` - ); - } else { - // We don't know what went wrong, hopefully we'll print something useful. - terminal.writeWarningLine(errorMessage); - } - return undefined; - } + return await this._tryGetBlobDataAsync(terminal, cacheId, async (blobClient: BlobClient) => { + return await blobClient.downloadToBuffer(); + }); } public async trySetCacheEntryBufferAsync( @@ -139,84 +88,73 @@ export class AzureStorageBuildCacheProvider cacheId: string, entryStream: Buffer ): Promise { - if (!this.isCacheWriteAllowed) { - terminal.writeErrorLine( - 'Writing to Azure Blob Storage cache is not allowed in the current configuration.' - ); - return false; - } - - const blobClient: BlobClient = await this._getBlobClientForCacheIdAsync(cacheId, terminal); - const blockBlobClient: BlockBlobClient = blobClient.getBlockBlobClient(); - let blobAlreadyExists: boolean = false; - - try { - blobAlreadyExists = await blockBlobClient.exists(); - } catch (err) { - const e: IBlobError = err as IBlobError; - - // If RUSH_BUILD_CACHE_CREDENTIAL is set but is corrupted or has been rotated - // in Azure Portal, or the user's own cached credentials have been corrupted or - // invalidated, we'll print the error and continue (this way we don't fail the - // actual rush build). - const errorMessage: string = - 'Error checking if cache entry exists in Azure Storage: ' + - [e.name, e.message, e.response?.status, e.response?.parsedHeaders?.errorCode] - .filter((piece: string | undefined) => piece) - .join(' '); - - terminal.writeWarningLine(errorMessage); - } - - if (blobAlreadyExists) { - terminal.writeVerboseLine('Build cache entry blob already exists.'); - return true; - } else { - try { - await blockBlobClient.upload(entryStream, entryStream.length); - return true; - } catch (e) { - if ((e as IBlobError).statusCode === 409 /* conflict */) { - // If something else has written to the blob at the same time, - // it's probably a concurrent process that is attempting to write - // the same cache entry. That is an effective success. - terminal.writeVerboseLine( - 'Azure Storage returned status 409 (conflict). The cache entry has ' + - `probably already been set by another builder. Code: "${(e as IBlobError).code}".` - ); - return true; - } else { - terminal.writeWarningLine(`Error uploading cache entry to Azure Storage: ${e}`); - return false; - } - } - } + return await this._trySetBlobDataAsync(terminal, cacheId, async (blockBlobClient: BlockBlobClient) => { + await blockBlobClient.upload(entryStream, entryStream.length); + }); } public async tryGetCacheEntryStreamByIdAsync( terminal: ITerminal, cacheId: string ): Promise { + return await this._tryGetBlobDataAsync(terminal, cacheId, async (blobClient: BlobClient) => { + const downloadResponse: BlobDownloadResponseParsed = await blobClient.download(); + return downloadResponse.readableStreamBody; + }); + } + + public async trySetCacheEntryStreamAsync( + terminal: ITerminal, + cacheId: string, + entryStream: NodeJS.ReadableStream + ): Promise { + return await this._trySetBlobDataAsync( + terminal, + cacheId, + async (blockBlobClient: BlockBlobClient) => { + await blockBlobClient.uploadStream(entryStream as Readable); + }, + () => { + // Drain the incoming stream since we won't consume it + entryStream.resume(); + } + ); + } + + /** + * Shared logic for both buffer and stream GET operations. + * Checks if the blob exists, retrieves data via the provided callback, and handles errors. + */ + private async _tryGetBlobDataAsync( + terminal: ITerminal, + cacheId: string, + getBlobDataAsync: (blobClient: BlobClient) => Promise + ): Promise { const blobClient: BlobClient = await this._getBlobClientForCacheIdAsync(cacheId, terminal); try { const blobExists: boolean = await blobClient.exists(); if (blobExists) { - const downloadResponse: BlobDownloadResponseParsed = await blobClient.download(); - return downloadResponse.readableStreamBody; + return await getBlobDataAsync(blobClient); } else { return undefined; } } catch (err) { - this._logBlobError(terminal, err, 'Error getting cache entry stream from Azure Storage: '); + this._logBlobError(terminal, err, 'Error getting cache entry from Azure Storage: '); return undefined; } } - public async trySetCacheEntryStreamAsync( + /** + * Shared logic for both buffer and stream SET operations. + * Checks write permission, whether the blob already exists, uploads via the provided callback, + * and handles 409 conflict errors. + */ + private async _trySetBlobDataAsync( terminal: ITerminal, cacheId: string, - entryStream: NodeJS.ReadableStream + uploadAsync: (blockBlobClient: BlockBlobClient) => Promise, + onBlobAlreadyExists?: () => void ): Promise { if (!this.isCacheWriteAllowed) { terminal.writeErrorLine( @@ -234,6 +172,10 @@ export class AzureStorageBuildCacheProvider } catch (err) { const e: IBlobError = err as IBlobError; + // If RUSH_BUILD_CACHE_CREDENTIAL is set but is corrupted or has been rotated + // in Azure Portal, or the user's own cached credentials have been corrupted or + // invalidated, we'll print the error and continue (this way we don't fail the + // actual rush build). const errorMessage: string = 'Error checking if cache entry exists in Azure Storage: ' + [e.name, e.message, e.response?.status, e.response?.parsedHeaders?.errorCode] @@ -245,22 +187,24 @@ export class AzureStorageBuildCacheProvider if (blobAlreadyExists) { terminal.writeVerboseLine('Build cache entry blob already exists.'); - // Drain the incoming stream since we won't consume it - entryStream.resume(); + onBlobAlreadyExists?.(); return true; } else { try { - await blockBlobClient.uploadStream(entryStream as Readable); + await uploadAsync(blockBlobClient); return true; } catch (e) { if ((e as IBlobError).statusCode === 409 /* conflict */) { + // If something else has written to the blob at the same time, + // it's probably a concurrent process that is attempting to write + // the same cache entry. That is an effective success. terminal.writeVerboseLine( 'Azure Storage returned status 409 (conflict). The cache entry has ' + `probably already been set by another builder. Code: "${(e as IBlobError).code}".` ); return true; } else { - terminal.writeWarningLine(`Error uploading cache entry stream to Azure Storage: ${e}`); + terminal.writeWarningLine(`Error uploading cache entry to Azure Storage: ${e}`); return false; } } diff --git a/rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts b/rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts index fec5deebdb1..443a74da4f9 100644 --- a/rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts +++ b/rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts @@ -86,7 +86,7 @@ export class BridgeCachePlugin implements IRushPlugin { buildCacheConfiguration, rushConfiguration: { experimentsConfiguration: { - configuration: { omitAppleDoubleFilesFromBuildCache } + configuration: { omitAppleDoubleFilesFromBuildCache, useStreamingBuildCache } } } } = context; @@ -120,7 +120,7 @@ export class BridgeCachePlugin implements IRushPlugin { buildCacheConfiguration, terminal, excludeAppleDoubleFiles: !!omitAppleDoubleFilesFromBuildCache, - useStreamingBuildCache: false + useStreamingBuildCache: !!useStreamingBuildCache } ); diff --git a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts index 92e73becac1..c359f6ffb74 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts @@ -132,13 +132,10 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { cacheId: string, objectBuffer: Buffer ): Promise { - if (!this.isCacheWriteAllowed) { - terminal.writeErrorLine('Writing to cache is not allowed in the current configuration.'); + if (!this._validateWriteAllowed(terminal, cacheId)) { return false; } - terminal.writeDebugLine('Uploading object with cacheId: ', cacheId); - try { const result: boolean | Buffer = await this._makeHttpRequestAsync({ terminal: terminal, @@ -183,13 +180,10 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { cacheId: string, entryStream: NodeJS.ReadableStream ): Promise { - if (!this.isCacheWriteAllowed) { - terminal.writeErrorLine('Writing to cache is not allowed in the current configuration.'); + if (!this._validateWriteAllowed(terminal, cacheId)) { return false; } - terminal.writeDebugLine('Uploading object with cacheId: ', cacheId); - try { const result: IWebClientStreamResponse | false = await this._makeHttpStreamRequestAsync({ terminal: terminal, @@ -286,6 +280,20 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { return this.__credentialCacheId; } + /** + * Common validation for write operations. Returns `true` if writing is allowed, + * `false` if it is not (and logs an error to the terminal). + */ + private _validateWriteAllowed(terminal: ITerminal, cacheId: string): boolean { + if (!this.isCacheWriteAllowed) { + terminal.writeErrorLine('Writing to cache is not allowed in the current configuration.'); + return false; + } + + terminal.writeDebugLine('Uploading object with cacheId: ', cacheId); + return true; + } + private async _makeHttpRequestAsync(options: { terminal: ITerminal; relUrl: string; From 6654e21b0f0db02b28e389b74edff5190439222b Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 5 Apr 2026 16:14:47 -0700 Subject: [PATCH 19/45] Clean up FileSystemBuildCacheProvider. --- .../FileSystemBuildCacheProvider.ts | 47 ++++++++++++++----- 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts b/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts index 7c0ee646489..c37eb39fe09 100644 --- a/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts +++ b/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts @@ -36,9 +36,11 @@ export class FileSystemBuildCacheProvider { private readonly _cacheFolderPath: string; public constructor(options: IFileSystemBuildCacheProviderOptions) { - this._cacheFolderPath = - options.rushUserConfiguration.buildCacheFolder || - path.join(options.rushConfiguration.commonTempFolder, DEFAULT_BUILD_CACHE_FOLDER_NAME); + const { + rushUserConfiguration: { buildCacheFolder }, + rushConfiguration: { commonTempFolder } + } = options; + this._cacheFolderPath = buildCacheFolder || path.join(commonTempFolder, DEFAULT_BUILD_CACHE_FOLDER_NAME); } /** @@ -56,7 +58,8 @@ export class FileSystemBuildCacheProvider { cacheId: string ): Promise { const cacheEntryFilePath: string = this.getCacheEntryPath(cacheId); - if (await FileSystem.existsAsync(cacheEntryFilePath)) { + const cacheEntryExists: boolean = await FileSystem.existsAsync(cacheEntryFilePath); + if (cacheEntryExists) { return cacheEntryFilePath; } else { return undefined; @@ -71,10 +74,13 @@ export class FileSystemBuildCacheProvider { cacheId: string, entryBuffer: Buffer ): Promise { - const cacheEntryFilePath: string = this.getCacheEntryPath(cacheId); - await FileSystem.writeFileAsync(cacheEntryFilePath, entryBuffer, { ensureFolderExists: true }); - terminal.writeVerboseLine(`Wrote cache entry to "${cacheEntryFilePath}".`); - return cacheEntryFilePath; + return await this._setCacheEntryAsync( + terminal, + cacheId, + async (cacheEntryFilePath: string): Promise => { + await FileSystem.writeFileAsync(cacheEntryFilePath, entryBuffer, { ensureFolderExists: true }); + } + ); } /** @@ -85,12 +91,29 @@ export class FileSystemBuildCacheProvider { terminal: ITerminal, cacheId: string, entryStream: NodeJS.ReadableStream + ): Promise { + return await this._setCacheEntryAsync( + terminal, + cacheId, + async (cacheEntryFilePath: string): Promise => { + const writeStream: FileSystemWriteStream = await FileSystem.createWriteStreamAsync( + cacheEntryFilePath, + { + ensureFolderExists: true + } + ); + await pipeline(entryStream, writeStream); + } + ); + } + + private async _setCacheEntryAsync( + terminal: ITerminal, + cacheId: string, + setEntryCallbackAsync: (cacheEntryFilePath: string) => Promise ): Promise { const cacheEntryFilePath: string = this.getCacheEntryPath(cacheId); - const writeStream: FileSystemWriteStream = await FileSystem.createWriteStreamAsync(cacheEntryFilePath, { - ensureFolderExists: true - }); - await pipeline(entryStream, writeStream); + await setEntryCallbackAsync(cacheEntryFilePath); terminal.writeVerboseLine(`Wrote cache entry to "${cacheEntryFilePath}".`); return cacheEntryFilePath; } From 94d266ddd528b529e0df3990bb0c38682c1da764 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 5 Apr 2026 16:16:52 -0700 Subject: [PATCH 20/45] fixup! Fix CI: add missing useStreamingBuildCache to bridge plugin, fix WebClient private member type mismatch --- .../rush-bridge-cache-plugin/src/BridgeCachePlugin.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts b/rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts index 443a74da4f9..1dc8135d7a1 100644 --- a/rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts +++ b/rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts @@ -86,7 +86,10 @@ export class BridgeCachePlugin implements IRushPlugin { buildCacheConfiguration, rushConfiguration: { experimentsConfiguration: { - configuration: { omitAppleDoubleFilesFromBuildCache, useStreamingBuildCache } + configuration: { + omitAppleDoubleFilesFromBuildCache: excludeAppleDoubleFiles = false, + useStreamingBuildCache = false + } } } } = context; @@ -119,8 +122,8 @@ export class BridgeCachePlugin implements IRushPlugin { { buildCacheConfiguration, terminal, - excludeAppleDoubleFiles: !!omitAppleDoubleFilesFromBuildCache, - useStreamingBuildCache: !!useStreamingBuildCache + excludeAppleDoubleFiles, + useStreamingBuildCache } ); From 3ef0f43a981bc8ad72bfa3dc5c467222e71867b8 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 5 Apr 2026 16:31:14 -0700 Subject: [PATCH 21/45] Clean up WebClient. --- libraries/rush-lib/src/utilities/WebClient.ts | 164 +++++++++--------- 1 file changed, 79 insertions(+), 85 deletions(-) diff --git a/libraries/rush-lib/src/utilities/WebClient.ts b/libraries/rush-lib/src/utilities/WebClient.ts index c1dd9aa4794..0c1d43683b1 100644 --- a/libraries/rush-lib/src/utilities/WebClient.ts +++ b/libraries/rush-lib/src/utilities/WebClient.ts @@ -25,7 +25,7 @@ export interface IWebClientResponseBase { } /** - * For use with {@link WebClient}. + * A response from {@link WebClient.fetchAsync}. */ export interface IWebClientResponse extends IWebClientResponseBase { getTextAsync: () => Promise; @@ -117,7 +117,7 @@ function _makeRawRequestAsync( resolve: (result: TResponse | PromiseLike) => void, reject: (error: Error) => void ) => void, - selfFn: (url: string, options: IRequestOptions, isRedirect?: boolean) => Promise + requestFnAsync: (url: string, options: IRequestOptions, isRedirect?: boolean) => Promise ): Promise { const { body, redirect } = options; @@ -128,25 +128,25 @@ function _makeRawRequestAsync( parsedUrl.protocol === 'https:' ? httpsRequest : httpRequest; const req: ClientRequest = requestFunction(url, options, (response: IncomingMessage) => { - const statusCode: number | undefined = response.statusCode; - + const { + statusCode, + headers: { location: redirectUrl } + } = response; if (statusCode === 301 || statusCode === 302) { // Drain the redirect response before following response.resume(); switch (redirect) { case 'follow': { - const redirectUrl: string | string[] | undefined = response.headers.location; - if (typeof redirectUrl === 'string') { - resolve(selfFn(redirectUrl, options, true)); + if (redirectUrl) { + requestFnAsync(redirectUrl, options, true).then(resolve).catch(reject); } else { - reject( - new Error(`Received status code ${response.statusCode} with no location header: ${url}`) - ); + reject(new Error(`Received status code ${statusCode} with no location header: ${url}`)); } return; } + case 'error': - reject(new Error(`Received status code ${response.statusCode}: ${url}`)); + reject(new Error(`Received status code ${statusCode}: ${url}`)); return; } } @@ -188,10 +188,8 @@ const makeRequestAsync: FetchFn = async ( responseBuffers.push(Buffer.from(chunk)); }); response.on('end', () => { + const { statusCode: status = 0, statusMessage: statusText, headers } = response; const responseData: Buffer = Buffer.concat(responseBuffers); - const status: number = response.statusCode || 0; - const statusText: string | undefined = response.statusMessage; - const headers: Record = response.headers; let bodyString: string | undefined; let bodyJson: unknown | undefined; @@ -285,10 +283,7 @@ const makeStreamRequestAsync: StreamFetchFn = async ( wasRedirected: boolean, resolve: (result: IWebClientStreamResponse | PromiseLike) => void ): void => { - const status: number = response.statusCode || 0; - const statusText: string | undefined = response.statusMessage; - const headers: Record = response.headers; - + const { statusCode: status = 0, statusMessage: statusText, headers } = response; resolve({ ok: status >= 200 && status < 300, status, @@ -306,8 +301,8 @@ const makeStreamRequestAsync: StreamFetchFn = async ( * A helper for issuing HTTP requests. */ export class WebClient { - private static _requestFn: FetchFn = makeRequestAsync; - private static _streamRequestFn: StreamFetchFn = makeStreamRequestAsync; + private static _requestFnAsync: FetchFn = makeRequestAsync; + private static _streamRequestFnAsync: StreamFetchFn = makeStreamRequestAsync; public readonly standardHeaders: Record = {}; @@ -317,19 +312,19 @@ export class WebClient { public proxy: WebClientProxy = WebClientProxy.Detect; public static mockRequestFn(fn: FetchFn): void { - WebClient._requestFn = fn; + WebClient._requestFnAsync = fn; } public static resetMockRequestFn(): void { - WebClient._requestFn = makeRequestAsync; + WebClient._requestFnAsync = makeRequestAsync; } public static mockStreamRequestFn(fn: StreamFetchFn): void { - WebClient._streamRequestFn = fn; + WebClient._streamRequestFnAsync = fn; } public static resetMockStreamRequestFn(): void { - WebClient._streamRequestFn = makeStreamRequestAsync; + WebClient._streamRequestFnAsync = makeStreamRequestAsync; } public static mergeHeaders(target: Record, source: Record): void { @@ -347,8 +342,8 @@ export class WebClient { url: string, options?: IGetFetchOptions | IFetchOptionsWithBody ): Promise { - const requestInit: IRequestOptions = buildRequestOptions(this, options); - return await WebClient._requestFn(url, requestInit); + const requestInit: IRequestOptions = this._buildRequestOptions(options); + return await WebClient._requestFnAsync(url, requestInit); } /** @@ -359,71 +354,70 @@ export class WebClient { url: string, options?: IGetFetchOptions | IFetchOptionsWithBody ): Promise { - const requestInit: IRequestOptions = buildRequestOptions(this, options); - return await WebClient._streamRequestFn(url, requestInit); + const requestInit: IRequestOptions = this._buildRequestOptions(options); + return await WebClient._streamRequestFnAsync(url, requestInit); } -} -function buildRequestOptions( - client: WebClient, - options?: IGetFetchOptions | IFetchOptionsWithBody -): IRequestOptions { - const { - headers: optionsHeaders, - timeoutMs = 15 * 1000, - verb, - redirect, - body, - noDecode - } = (options as IFetchOptionsWithBody | undefined) ?? {}; - - const headers: Record = {}; - - WebClient.mergeHeaders(headers, client.standardHeaders); - - if (optionsHeaders) { - WebClient.mergeHeaders(headers, optionsHeaders); - } + private _buildRequestOptions(options?: IGetFetchOptions | IFetchOptionsWithBody): IRequestOptions { + const { + headers: optionsHeaders, + timeoutMs = 15 * 1000, + verb, + redirect, + body, + noDecode + } = (options as IFetchOptionsWithBody | undefined) ?? {}; - if (client.userAgent) { - headers[USER_AGENT_HEADER_NAME] = client.userAgent; - } + const headers: Record = {}; - if (client.accept) { - headers[ACCEPT_HEADER_NAME] = client.accept; - } + const { standardHeaders, userAgent, accept, proxy } = this; - let proxyUrl: string = ''; + WebClient.mergeHeaders(headers, standardHeaders); - switch (client.proxy) { - case WebClientProxy.Detect: - if (process.env.HTTPS_PROXY) { - proxyUrl = process.env.HTTPS_PROXY; - } else if (process.env.HTTP_PROXY) { - proxyUrl = process.env.HTTP_PROXY; - } - break; - - case WebClientProxy.Fiddler: - // For debugging, disable cert validation - // eslint-disable-next-line - process.env['NODE_TLS_REJECT_UNAUTHORIZED'] = '0'; - proxyUrl = 'http://localhost:8888/'; - break; - } + if (optionsHeaders) { + WebClient.mergeHeaders(headers, optionsHeaders); + } - let agent: HttpAgent | undefined = undefined; - if (proxyUrl) { - agent = createHttpsProxyAgent(proxyUrl); - } + if (userAgent) { + headers[USER_AGENT_HEADER_NAME] = userAgent; + } + + if (accept) { + headers[ACCEPT_HEADER_NAME] = accept; + } + + let proxyUrl: string = ''; - return { - method: verb, - headers, - agent, - timeout: timeoutMs, - redirect, - body, - noDecode - }; + switch (proxy) { + case WebClientProxy.Detect: + if (process.env.HTTPS_PROXY) { + proxyUrl = process.env.HTTPS_PROXY; + } else if (process.env.HTTP_PROXY) { + proxyUrl = process.env.HTTP_PROXY; + } + break; + + case WebClientProxy.Fiddler: + // For debugging, disable cert validation + // eslint-disable-next-line + process.env['NODE_TLS_REJECT_UNAUTHORIZED'] = '0'; + proxyUrl = 'http://localhost:8888/'; + break; + } + + let agent: HttpAgent | undefined = undefined; + if (proxyUrl) { + agent = createHttpsProxyAgent(proxyUrl); + } + + return { + method: verb, + headers, + agent, + timeout: timeoutMs, + redirect, + body, + noDecode + }; + } } From 1e7942a41cd7c330a9720761b9b89724f6e86665 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Apr 2026 01:19:28 +0000 Subject: [PATCH 22/45] Handle Content-Encoding decompression in streaming WebClient path Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/13978d72-ea9a-4463-a6a2-51ecfc72f3ff Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- libraries/rush-lib/src/utilities/WebClient.ts | 51 ++++++++++++++++++- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/libraries/rush-lib/src/utilities/WebClient.ts b/libraries/rush-lib/src/utilities/WebClient.ts index 0c1d43683b1..7fbb117c4e6 100644 --- a/libraries/rush-lib/src/utilities/WebClient.ts +++ b/libraries/rush-lib/src/utilities/WebClient.ts @@ -274,6 +274,8 @@ const makeStreamRequestAsync: StreamFetchFn = async ( options: IRequestOptions, redirected: boolean = false ) => { + const { noDecode } = options; + return _makeRawRequestAsync( url, options, @@ -284,14 +286,59 @@ const makeStreamRequestAsync: StreamFetchFn = async ( resolve: (result: IWebClientStreamResponse | PromiseLike) => void ): void => { const { statusCode: status = 0, statusMessage: statusText, headers } = response; - resolve({ + + const buildResult = (stream: Readable): IWebClientStreamResponse => ({ ok: status >= 200 && status < 300, status, statusText, redirected: wasRedirected, headers, - stream: response + stream }); + + // Handle Content-Encoding decompression for streaming responses, + // matching the buffer-based path's behavior in getBufferAsync() + let encodings: string | string[] | undefined; + if (!noDecode) { + encodings = headers[CONTENT_ENCODING_HEADER_NAME]; + } + + if (encodings !== undefined) { + // Resolve with a promise so we can lazily import zlib (same pattern as buffer path) + resolve( + (async () => { + const zlib: typeof import('zlib') = await import('node:zlib'); + if (!Array.isArray(encodings)) { + encodings = encodings!.split(','); + } + + let resultStream: Readable = response; + for (const encoding of encodings) { + switch (encoding.trim()) { + case DEFLATE_ENCODING: { + resultStream = resultStream.pipe(zlib.createInflate()); + break; + } + case GZIP_ENCODING: { + resultStream = resultStream.pipe(zlib.createGunzip()); + break; + } + case BROTLI_ENCODING: { + resultStream = resultStream.pipe(zlib.createBrotliDecompress()); + break; + } + default: { + throw new Error(`Unsupported content-encoding: ${encodings}`); + } + } + } + + return buildResult(resultStream); + })() + ); + } else { + resolve(buildResult(response)); + } }, makeStreamRequestAsync ); From 13562d9e02452d471bf960ae2d83bfbe05d55dba Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Apr 2026 01:23:43 +0000 Subject: [PATCH 23/45] Fix error message to show specific unsupported encoding value Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/13978d72-ea9a-4463-a6a2-51ecfc72f3ff Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- libraries/rush-lib/src/utilities/WebClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/rush-lib/src/utilities/WebClient.ts b/libraries/rush-lib/src/utilities/WebClient.ts index 7fbb117c4e6..3f5230c6056 100644 --- a/libraries/rush-lib/src/utilities/WebClient.ts +++ b/libraries/rush-lib/src/utilities/WebClient.ts @@ -328,7 +328,7 @@ const makeStreamRequestAsync: StreamFetchFn = async ( break; } default: { - throw new Error(`Unsupported content-encoding: ${encodings}`); + throw new Error(`Unsupported content-encoding: ${encoding.trim()}`); } } } From ab6f07eee9692e39297491e49ab916facb5244a7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Apr 2026 02:54:47 +0000 Subject: [PATCH 24/45] Fix CI: Move WebClient private members to module-level for rush-sdk type compatibility Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/d8f5e137-3cb0-4ed9-8391-50d7a33d804e Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- libraries/rush-lib/src/utilities/WebClient.ts | 169 ++++++++++-------- 1 file changed, 92 insertions(+), 77 deletions(-) diff --git a/libraries/rush-lib/src/utilities/WebClient.ts b/libraries/rush-lib/src/utilities/WebClient.ts index 3f5230c6056..d3daaa77080 100644 --- a/libraries/rush-lib/src/utilities/WebClient.ts +++ b/libraries/rush-lib/src/utilities/WebClient.ts @@ -344,13 +344,93 @@ const makeStreamRequestAsync: StreamFetchFn = async ( ); }; +// Module-level mutable state for mock injection. These must NOT be private members +// of WebClient because rush-sdk re-exports WebClient as a separate type declaration, +// and TypeScript's structural typing treats private members nominally, causing type +// incompatibility between the rush-lib and rush-sdk versions. +let _requestFnAsync: FetchFn = makeRequestAsync; +let _streamRequestFnAsync: StreamFetchFn = makeStreamRequestAsync; + +function _mergeHeaders(target: Record, source: Record): void { + for (const [name, value] of Object.entries(source)) { + target[name] = value; + } +} + +/** + * Builds the low-level IRequestOptions from WebClient instance state and caller-provided options. + * This is a module-level function (not a private method) to avoid the rush-sdk type mismatch. + */ +function buildRequestOptions( + webClient: WebClient, + options?: IGetFetchOptions | IFetchOptionsWithBody +): IRequestOptions { + const { + headers: optionsHeaders, + timeoutMs = 15 * 1000, + verb, + redirect, + body, + noDecode + } = (options as IFetchOptionsWithBody | undefined) ?? {}; + + const headers: Record = {}; + + const { standardHeaders, userAgent, accept, proxy } = webClient; + + _mergeHeaders(headers, standardHeaders); + + if (optionsHeaders) { + _mergeHeaders(headers, optionsHeaders); + } + + if (userAgent) { + headers[USER_AGENT_HEADER_NAME] = userAgent; + } + + if (accept) { + headers[ACCEPT_HEADER_NAME] = accept; + } + + let proxyUrl: string = ''; + + switch (proxy) { + case WebClientProxy.Detect: + if (process.env.HTTPS_PROXY) { + proxyUrl = process.env.HTTPS_PROXY; + } else if (process.env.HTTP_PROXY) { + proxyUrl = process.env.HTTP_PROXY; + } + break; + + case WebClientProxy.Fiddler: + // For debugging, disable cert validation + // eslint-disable-next-line + process.env['NODE_TLS_REJECT_UNAUTHORIZED'] = '0'; + proxyUrl = 'http://localhost:8888/'; + break; + } + + let agent: HttpAgent | undefined = undefined; + if (proxyUrl) { + agent = createHttpsProxyAgent(proxyUrl); + } + + return { + method: verb, + headers, + agent, + timeout: timeoutMs, + redirect, + body, + noDecode + }; +} + /** * A helper for issuing HTTP requests. */ export class WebClient { - private static _requestFnAsync: FetchFn = makeRequestAsync; - private static _streamRequestFnAsync: StreamFetchFn = makeStreamRequestAsync; - public readonly standardHeaders: Record = {}; public accept: string | undefined = '*/*'; @@ -359,25 +439,23 @@ export class WebClient { public proxy: WebClientProxy = WebClientProxy.Detect; public static mockRequestFn(fn: FetchFn): void { - WebClient._requestFnAsync = fn; + _requestFnAsync = fn; } public static resetMockRequestFn(): void { - WebClient._requestFnAsync = makeRequestAsync; + _requestFnAsync = makeRequestAsync; } public static mockStreamRequestFn(fn: StreamFetchFn): void { - WebClient._streamRequestFnAsync = fn; + _streamRequestFnAsync = fn; } public static resetMockStreamRequestFn(): void { - WebClient._streamRequestFnAsync = makeStreamRequestAsync; + _streamRequestFnAsync = makeStreamRequestAsync; } public static mergeHeaders(target: Record, source: Record): void { - for (const [name, value] of Object.entries(source)) { - target[name] = value; - } + _mergeHeaders(target, source); } public addBasicAuthHeader(userName: string, password: string): void { @@ -389,8 +467,8 @@ export class WebClient { url: string, options?: IGetFetchOptions | IFetchOptionsWithBody ): Promise { - const requestInit: IRequestOptions = this._buildRequestOptions(options); - return await WebClient._requestFnAsync(url, requestInit); + const requestInit: IRequestOptions = buildRequestOptions(this, options); + return await _requestFnAsync(url, requestInit); } /** @@ -401,70 +479,7 @@ export class WebClient { url: string, options?: IGetFetchOptions | IFetchOptionsWithBody ): Promise { - const requestInit: IRequestOptions = this._buildRequestOptions(options); - return await WebClient._streamRequestFnAsync(url, requestInit); - } - - private _buildRequestOptions(options?: IGetFetchOptions | IFetchOptionsWithBody): IRequestOptions { - const { - headers: optionsHeaders, - timeoutMs = 15 * 1000, - verb, - redirect, - body, - noDecode - } = (options as IFetchOptionsWithBody | undefined) ?? {}; - - const headers: Record = {}; - - const { standardHeaders, userAgent, accept, proxy } = this; - - WebClient.mergeHeaders(headers, standardHeaders); - - if (optionsHeaders) { - WebClient.mergeHeaders(headers, optionsHeaders); - } - - if (userAgent) { - headers[USER_AGENT_HEADER_NAME] = userAgent; - } - - if (accept) { - headers[ACCEPT_HEADER_NAME] = accept; - } - - let proxyUrl: string = ''; - - switch (proxy) { - case WebClientProxy.Detect: - if (process.env.HTTPS_PROXY) { - proxyUrl = process.env.HTTPS_PROXY; - } else if (process.env.HTTP_PROXY) { - proxyUrl = process.env.HTTP_PROXY; - } - break; - - case WebClientProxy.Fiddler: - // For debugging, disable cert validation - // eslint-disable-next-line - process.env['NODE_TLS_REJECT_UNAUTHORIZED'] = '0'; - proxyUrl = 'http://localhost:8888/'; - break; - } - - let agent: HttpAgent | undefined = undefined; - if (proxyUrl) { - agent = createHttpsProxyAgent(proxyUrl); - } - - return { - method: verb, - headers, - agent, - timeout: timeoutMs, - redirect, - body, - noDecode - }; + const requestInit: IRequestOptions = buildRequestOptions(this, options); + return await _streamRequestFnAsync(url, requestInit); } } From ba406852ba6ddae4aa29ae75d2d9fdf0d748f5a9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Apr 2026 03:17:19 +0000 Subject: [PATCH 25/45] Address review comments: fix response.resume race, stream retry bug, error logging, stream cleanup, and nits Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/d0190bf7-a346-4a71-93bf-d5375d98b552 Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- .../logic/buildCache/OperationBuildCache.ts | 13 ++++++---- libraries/rush-lib/src/utilities/WebClient.ts | 5 ++-- .../src/AzureStorageBuildCacheProvider.ts | 4 +-- .../src/HttpBuildCacheProvider.ts | 25 +++++++++++-------- 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts index f26a381ddcf..6e5b772afac 100644 --- a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts +++ b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts @@ -190,6 +190,7 @@ export class OperationBuildCache { ); updateLocalCacheSuccess = true; } catch (e) { + terminal.writeVerboseLine(`Failed to update local cache: ${e}`); updateLocalCacheSuccess = false; } } @@ -207,6 +208,7 @@ export class OperationBuildCache { ); updateLocalCacheSuccess = true; } catch (e) { + terminal.writeVerboseLine(`Failed to update local cache: ${e}`); updateLocalCacheSuccess = false; } } @@ -353,11 +355,12 @@ export class OperationBuildCache { ) { // Use streaming upload to avoid loading the entire cache entry into memory const entryStream: FileSystemReadStream = FileSystem.createReadStream(localCacheEntryPath); - setCloudCacheEntryPromise = this._cloudBuildCacheProvider.trySetCacheEntryStreamAsync( - terminal, - cacheId, - entryStream - ); + setCloudCacheEntryPromise = this._cloudBuildCacheProvider + .trySetCacheEntryStreamAsync(terminal, cacheId, entryStream) + .catch((e: Error) => { + entryStream.destroy(); + throw e; + }); } else { const cacheEntryBuffer: Buffer = await FileSystem.readFileToBufferAsync(localCacheEntryPath); setCloudCacheEntryPromise = this._cloudBuildCacheProvider.trySetCacheEntryBufferAsync( diff --git a/libraries/rush-lib/src/utilities/WebClient.ts b/libraries/rush-lib/src/utilities/WebClient.ts index d3daaa77080..20547a93428 100644 --- a/libraries/rush-lib/src/utilities/WebClient.ts +++ b/libraries/rush-lib/src/utilities/WebClient.ts @@ -133,10 +133,10 @@ function _makeRawRequestAsync( headers: { location: redirectUrl } } = response; if (statusCode === 301 || statusCode === 302) { - // Drain the redirect response before following - response.resume(); switch (redirect) { case 'follow': { + // Drain the redirect response since we're discarding it + response.resume(); if (redirectUrl) { requestFnAsync(redirectUrl, options, true).then(resolve).catch(reject); } else { @@ -146,6 +146,7 @@ function _makeRawRequestAsync( } case 'error': + response.resume(); reject(new Error(`Received status code ${statusCode}: ${url}`)); return; } diff --git a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts index 233a5a10205..32f6bc154d4 100644 --- a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts +++ b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts @@ -86,10 +86,10 @@ export class AzureStorageBuildCacheProvider public async trySetCacheEntryBufferAsync( terminal: ITerminal, cacheId: string, - entryStream: Buffer + entryBuffer: Buffer ): Promise { return await this._trySetBlobDataAsync(terminal, cacheId, async (blockBlobClient: BlockBlobClient) => { - await blockBlobClient.upload(entryStream, entryStream.length); + await blockBlobClient.upload(entryBuffer, entryBuffer.length); }); } diff --git a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts index c359f6ffb74..40a0688b19d 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts @@ -391,7 +391,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { headers: headers, body: body, redirect: 'follow', - timeoutMs: 0 // Use the default timeout + timeoutMs: 0 // Disable timeout for streaming transfers of large cache entries }; const response: IWebClientResponse | IWebClientStreamResponse = stream @@ -411,15 +411,20 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { typeof credentials !== 'string' && safeCredentialOptions === CredentialsOptions.Optional ) { - // If we don't already have credentials yet, and we got a response from the server - // that is a "normal" failure (4xx), then we assume that credentials are probably - // required. Re-attempt the request, requiring credentials this time. - // - // This counts as part of the "first attempt", so it is not included in the max attempts - return await this._makeHttpCoreRequestAsync({ - ...options, - credentialOptions: CredentialsOptions.Required - }); + // Skip credential fallback for stream bodies since the stream has already been consumed + // by the first attempt and cannot be replayed. + const isStreamBody: boolean = !!body && typeof (body as Readable).pipe === 'function'; + if (!isStreamBody) { + // If we don't already have credentials yet, and we got a response from the server + // that is a "normal" failure (4xx), then we assume that credentials are probably + // required. Re-attempt the request, requiring credentials this time. + // + // This counts as part of the "first attempt", so it is not included in the max attempts + return await this._makeHttpCoreRequestAsync({ + ...options, + credentialOptions: CredentialsOptions.Required + }); + } } if (options.maxAttempts > 1) { From fd1cb7554873c66fe1e7524c70cd88b9b09ab2cf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Apr 2026 04:23:17 +0000 Subject: [PATCH 26/45] Add unit tests for streaming cache APIs and fill buffer-based test gaps Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/86050f65-dd6c-45f4-ac41-95fdb860c053 Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com> --- .../src/test/AmazonS3Client.test.ts | 183 ++++++++++++ .../__snapshots__/AmazonS3Client.test.ts.snap | 52 ++++ .../src/test/HttpBuildCacheProvider.test.ts | 277 ++++++++++++++++++ 3 files changed, 512 insertions(+) diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts index 07b5eddfc73..6c518b1cfbe 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts @@ -634,4 +634,187 @@ describe(AmazonS3Client.name, () => { ); }); }); + + describe('Streaming requests', () => { + let realDate: typeof Date; + let realSetTimeout: typeof setTimeout; + beforeEach(() => { + // mock date + realDate = global.Date; + global.Date = MockedDate as typeof Date; + + // mock setTimeout + realSetTimeout = global.setTimeout; + global.setTimeout = ((callback: () => void, time: number) => { + return realSetTimeout(callback, 1); + }).bind(global) as typeof global.setTimeout; + }); + + afterEach(() => { + jest.restoreAllMocks(); + global.Date = realDate; + global.setTimeout = realSetTimeout.bind(global); + }); + + describe('Getting an object stream', () => { + async function makeStreamGetRequestAsync( + credentials: IAmazonS3Credentials | undefined, + options: IAmazonS3BuildCacheProviderOptionsAdvanced, + objectName: string, + status: number, + statusText?: string + ): Promise<{ result: NodeJS.ReadableStream | undefined; spy: jest.SpyInstance }> { + const { Readable } = await import('node:stream'); + const mockStream = new Readable({ read() {} }); + + const spy: jest.SpyInstance = jest + .spyOn(WebClient.prototype, 'fetchStreamAsync') + .mockReturnValue( + Promise.resolve({ + stream: mockStream, + headers: {}, + status, + statusText, + ok: status >= 200 && status < 300, + redirected: false + }) + ); + + const s3Client: AmazonS3Client = new AmazonS3Client(credentials, options, webClient, terminal); + const result = await s3Client.getObjectStreamAsync(objectName); + return { result, spy }; + } + + it('Can get an object stream', async () => { + const { result, spy } = await makeStreamGetRequestAsync( + { + accessKeyId: 'accessKeyId', + secretAccessKey: 'secretAccessKey', + sessionToken: undefined + }, + DUMMY_OPTIONS, + 'abc123', + 200 + ); + expect(result).toBeDefined(); + expect(spy).toHaveBeenCalledTimes(1); + expect(spy.mock.calls[0]).toMatchSnapshot(); + spy.mockRestore(); + }); + + it('Returns undefined for a 404 (missing) object stream', async () => { + const { result, spy } = await makeStreamGetRequestAsync( + { + accessKeyId: 'accessKeyId', + secretAccessKey: 'secretAccessKey', + sessionToken: undefined + }, + DUMMY_OPTIONS, + 'abc123', + 404, + 'Not Found' + ); + expect(result).toBeUndefined(); + expect(spy).toHaveBeenCalledTimes(1); + spy.mockRestore(); + }); + }); + + describe('Uploading an object stream', () => { + it('Throws an error if credentials are not provided', async () => { + const { Readable } = await import('node:stream'); + const s3Client: AmazonS3Client = new AmazonS3Client( + undefined, + { s3Endpoint: 'http://foo.bar.baz', ...DUMMY_OPTIONS_WITHOUT_ENDPOINT }, + webClient, + terminal + ); + + const mockStream = new Readable({ read() {} }); + try { + await s3Client.uploadObjectStreamAsync('temp', mockStream); + fail('Expected an exception to be thrown'); + } catch (e) { + expect(e).toMatchSnapshot(); + } + }); + + it('Uploads a stream successfully', async () => { + const { Readable } = await import('node:stream'); + const mockStream = new Readable({ read() {} }); + const responseStream = new Readable({ read() {} }); + + const spy: jest.SpyInstance = jest + .spyOn(WebClient.prototype, 'fetchStreamAsync') + .mockReturnValue( + Promise.resolve({ + stream: responseStream, + headers: {}, + status: 200, + statusText: 'OK', + ok: true, + redirected: false + }) + ); + + const s3Client: AmazonS3Client = new AmazonS3Client( + { + accessKeyId: 'accessKeyId', + secretAccessKey: 'secretAccessKey', + sessionToken: undefined + }, + DUMMY_OPTIONS, + webClient, + terminal + ); + + await s3Client.uploadObjectStreamAsync('abc123', mockStream); + + expect(spy).toHaveBeenCalledTimes(1); + expect(spy.mock.calls[0]).toMatchSnapshot(); + spy.mockRestore(); + }); + + it('Does not retry on failure (stream consumed)', async () => { + const { Readable } = await import('node:stream'); + const mockStream = new Readable({ read() {} }); + const responseStream = new Readable({ read() {} }); + + const spy: jest.SpyInstance = jest + .spyOn(WebClient.prototype, 'fetchStreamAsync') + .mockReturnValue( + Promise.resolve({ + stream: responseStream, + headers: {}, + status: 500, + statusText: 'InternalServerError', + ok: false, + redirected: false + }) + ); + + const s3Client: AmazonS3Client = new AmazonS3Client( + { + accessKeyId: 'accessKeyId', + secretAccessKey: 'secretAccessKey', + sessionToken: undefined + }, + DUMMY_OPTIONS, + webClient, + terminal + ); + + try { + await s3Client.uploadObjectStreamAsync('abc123', mockStream); + fail('Expected an exception to be thrown'); + } catch (e) { + expect((e as Error).message).toContain('500'); + } + + // Only 1 call - no retry for streams + expect(spy).toHaveBeenCalledTimes(1); + spy.mockRestore(); + }); + }); + }); }); diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/__snapshots__/AmazonS3Client.test.ts.snap b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/__snapshots__/AmazonS3Client.test.ts.snap index 9481b3d6983..b190f855e48 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/__snapshots__/AmazonS3Client.test.ts.snap +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/__snapshots__/AmazonS3Client.test.ts.snap @@ -548,3 +548,55 @@ exports[`AmazonS3Client Rejects invalid S3 endpoint values 9`] = `"Invalid S3 en exports[`AmazonS3Client Rejects invalid S3 endpoint values 10`] = `"Invalid S3 endpoint. Some part of the hostname contains invalid characters or is too long"`; exports[`AmazonS3Client Rejects invalid S3 endpoint values 11`] = `"Invalid S3 endpoint. Some part of the hostname contains invalid characters or is too long"`; + +exports[`AmazonS3Client Streaming requests Getting an object stream Can get an object stream 1`] = ` +Array [ + "http://localhost:9000/abc123", + Object { + "headers": Object { + "Authorization": "AWS4-HMAC-SHA256 Credential=accessKeyId/20200418/us-east-1/s3/aws4_request,SignedHeaders=host;x-amz-content-sha256;x-amz-date,Signature=194608e9e7ba6d8aa4a019b3b6fd237e6b09ef1f45ff7fa60cbb81c1875538be", + "x-amz-content-sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + "x-amz-date": "20200418T123242Z", + }, + "verb": "GET", + }, +] +`; + +exports[`AmazonS3Client Streaming requests Uploading an object stream Throws an error if credentials are not provided 1`] = `[Error: Credentials are required to upload objects to S3.]`; + +exports[`AmazonS3Client Streaming requests Uploading an object stream Uploads a stream successfully 1`] = ` +Array [ + "http://localhost:9000/abc123", + Object { + "body": Readable { + "_events": Object { + "close": undefined, + "data": undefined, + "end": undefined, + "error": undefined, + "readable": undefined, + }, + "_maxListeners": undefined, + "_read": [Function], + "_readableState": ReadableState { + "awaitDrainWriters": null, + "buffer": Array [], + "bufferIndex": 0, + "highWaterMark": 65536, + "length": 0, + "pipes": Array [], + Symbol(kState): 1052940, + }, + Symbol(shapeMode): true, + Symbol(kCapture): false, + }, + "headers": Object { + "Authorization": "AWS4-HMAC-SHA256 Credential=accessKeyId/20200418/us-east-1/s3/aws4_request,SignedHeaders=host;x-amz-content-sha256;x-amz-date,Signature=bfc5ee97ccfdb44b7f351c0b550a1ac9c2cdb661606dbba8a74c11be6b5b2b72", + "x-amz-content-sha256": "UNSIGNED-PAYLOAD", + "x-amz-date": "20200418T123242Z", + }, + "verb": "PUT", + }, +] +`; diff --git a/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts b/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts index a86ddb342de..dc572338b5e 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts @@ -5,6 +5,8 @@ jest.mock('@rushstack/rush-sdk/lib/utilities/WebClient', () => { return jest.requireActual('@microsoft/rush-lib/lib/utilities/WebClient'); }); +import { Readable } from 'node:stream'; + import { type RushSession, EnvironmentConfiguration } from '@rushstack/rush-sdk'; import { StringBufferTerminalProvider, Terminal } from '@rushstack/terminal'; import { WebClient } from '@rushstack/rush-sdk/lib/utilities/WebClient'; @@ -24,24 +26,37 @@ const EXAMPLE_OPTIONS: IHttpBuildCacheProviderOptions = { minHttpRetryDelayMs: 1 }; +const WRITE_ALLOWED_OPTIONS: IHttpBuildCacheProviderOptions = { + ...EXAMPLE_OPTIONS, + isCacheWriteAllowed: true +}; + type FetchFnType = Parameters[0]; +type StreamFetchFnType = Parameters[0]; describe('HttpBuildCacheProvider', () => { let terminalBuffer: StringBufferTerminalProvider; let terminal!: Terminal; let fetchFn: jest.Mock; + let streamFetchFn: jest.Mock; beforeEach(() => { terminalBuffer = new StringBufferTerminalProvider(); terminal = new Terminal(terminalBuffer); fetchFn = jest.fn(); + streamFetchFn = jest.fn(); WebClient.mockRequestFn(fetchFn as unknown as FetchFnType); + WebClient.mockStreamRequestFn(streamFetchFn as unknown as StreamFetchFnType); }); afterEach(() => { WebClient.resetMockRequestFn(); + WebClient.resetMockStreamRequestFn(); + jest.restoreAllMocks(); }); + // ── Buffer-based GET ────────────────────────────────────────────────────── + describe('tryGetCacheEntryBufferByIdAsync', () => { it('prints warning if read credentials are not available', async () => { jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue(undefined); @@ -138,5 +153,267 @@ Array [ ] `); }); + + it('returns a buffer on a successful response', async () => { + jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue('token123'); + + const session: RushSession = {} as RushSession; + const provider = new HttpBuildCacheProvider(EXAMPLE_OPTIONS, session); + const expectedBuffer = Buffer.from('cache-contents'); + + mocked(fetchFn).mockResolvedValue({ + status: 200, + statusText: 'OK', + ok: true, + redirected: false, + headers: {}, + getBufferAsync: () => Promise.resolve(expectedBuffer) + }); + + const result = await provider.tryGetCacheEntryBufferByIdAsync(terminal, 'some-key'); + expect(result).toEqual(expectedBuffer); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); + }); + + // ── Buffer-based SET ────────────────────────────────────────────────────── + + describe('trySetCacheEntryBufferAsync', () => { + it('returns false when cache write is not allowed', async () => { + const session: RushSession = {} as RushSession; + const provider = new HttpBuildCacheProvider(EXAMPLE_OPTIONS, session); // write not allowed + + const result = await provider.trySetCacheEntryBufferAsync( + terminal, + 'some-key', + Buffer.from('data') + ); + + expect(result).toBe(false); + expect(fetchFn).not.toHaveBeenCalled(); + }); + + it('uploads a buffer successfully', async () => { + jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue('token123'); + + const session: RushSession = {} as RushSession; + const provider = new HttpBuildCacheProvider(WRITE_ALLOWED_OPTIONS, session); + + mocked(fetchFn).mockResolvedValue({ + status: 200, + statusText: 'OK', + ok: true, + redirected: false, + headers: {} + }); + + const result = await provider.trySetCacheEntryBufferAsync( + terminal, + 'some-key', + Buffer.from('cache-data') + ); + + expect(result).toBe(true); + expect(fetchFn).toHaveBeenCalledTimes(1); + expect(fetchFn).toHaveBeenCalledWith( + 'https://buildcache.example.acme.com/some-key', + expect.objectContaining({ + method: 'POST' + }) + ); + }); + + it('retries up to 3 times on server error', async () => { + jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue('token123'); + + const session: RushSession = {} as RushSession; + const provider = new HttpBuildCacheProvider(WRITE_ALLOWED_OPTIONS, session); + + mocked(fetchFn).mockResolvedValue({ + status: 500, + statusText: 'InternalServerError', + ok: false + }); + + const result = await provider.trySetCacheEntryBufferAsync( + terminal, + 'some-key', + Buffer.from('data') + ); + + expect(result).toBe(false); + expect(fetchFn).toHaveBeenCalledTimes(3); + }); + }); + + // ── Stream-based GET ────────────────────────────────────────────────────── + + describe('tryGetCacheEntryStreamByIdAsync', () => { + it('returns a stream on a successful response', async () => { + jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue('token123'); + + const session: RushSession = {} as RushSession; + const provider = new HttpBuildCacheProvider(EXAMPLE_OPTIONS, session); + const mockStream = new Readable({ read() {} }); + + mocked(streamFetchFn).mockResolvedValue({ + status: 200, + statusText: 'OK', + ok: true, + redirected: false, + headers: {}, + stream: mockStream + }); + + const result = await provider.tryGetCacheEntryStreamByIdAsync(terminal, 'some-key'); + expect(result).toBe(mockStream); + expect(streamFetchFn).toHaveBeenCalledTimes(1); + expect(streamFetchFn).toHaveBeenCalledWith( + 'https://buildcache.example.acme.com/some-key', + expect.objectContaining({ + method: 'GET', + redirect: 'follow' + }) + ); + }); + + it('returns undefined on credential failure', async () => { + jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue(undefined); + + const session: RushSession = {} as RushSession; + const provider = new HttpBuildCacheProvider(EXAMPLE_OPTIONS, session); + const mockStream = new Readable({ read() {} }); + + mocked(streamFetchFn).mockResolvedValue({ + status: 401, + statusText: 'Unauthorized', + ok: false, + stream: mockStream + }); + + const result = await provider.tryGetCacheEntryStreamByIdAsync(terminal, 'some-key'); + expect(result).toBe(undefined); + }); + + it('retries up to 3 times on server error', async () => { + jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue(undefined); + + const session: RushSession = {} as RushSession; + const provider = new HttpBuildCacheProvider(EXAMPLE_OPTIONS, session); + const createMockStream = (): Readable => new Readable({ read() {} }); + + mocked(streamFetchFn).mockResolvedValueOnce({ + status: 500, + statusText: 'InternalServiceError', + ok: false, + stream: createMockStream() + }); + mocked(streamFetchFn).mockResolvedValueOnce({ + status: 503, + statusText: 'ServiceUnavailable', + ok: false, + stream: createMockStream() + }); + mocked(streamFetchFn).mockResolvedValueOnce({ + status: 504, + statusText: 'BadGateway', + ok: false, + stream: createMockStream() + }); + + const result = await provider.tryGetCacheEntryStreamByIdAsync(terminal, 'some-key'); + expect(result).toBe(undefined); + expect(streamFetchFn).toHaveBeenCalledTimes(3); + }); + }); + + // ── Stream-based SET ────────────────────────────────────────────────────── + + describe('trySetCacheEntryStreamAsync', () => { + it('returns false when cache write is not allowed', async () => { + const session: RushSession = {} as RushSession; + const provider = new HttpBuildCacheProvider(EXAMPLE_OPTIONS, session); // write not allowed + + const entryStream = new Readable({ read() {} }); + const result = await provider.trySetCacheEntryStreamAsync(terminal, 'some-key', entryStream); + + expect(result).toBe(false); + expect(streamFetchFn).not.toHaveBeenCalled(); + }); + + it('uploads a stream successfully', async () => { + jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue('token123'); + + const session: RushSession = {} as RushSession; + const provider = new HttpBuildCacheProvider(WRITE_ALLOWED_OPTIONS, session); + const entryStream = new Readable({ read() {} }); + const responseStream = new Readable({ read() {} }); + + mocked(streamFetchFn).mockResolvedValue({ + status: 200, + statusText: 'OK', + ok: true, + redirected: false, + headers: {}, + stream: responseStream + }); + + const result = await provider.trySetCacheEntryStreamAsync(terminal, 'some-key', entryStream); + + expect(result).toBe(true); + expect(streamFetchFn).toHaveBeenCalledTimes(1); + expect(streamFetchFn).toHaveBeenCalledWith( + 'https://buildcache.example.acme.com/some-key', + expect.objectContaining({ + method: 'POST' + }) + ); + }); + + it('does not retry on failure (stream consumed)', async () => { + jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue('token123'); + + const session: RushSession = {} as RushSession; + const provider = new HttpBuildCacheProvider(WRITE_ALLOWED_OPTIONS, session); + const entryStream = new Readable({ read() {} }); + const responseStream = new Readable({ read() {} }); + + mocked(streamFetchFn).mockResolvedValue({ + status: 500, + statusText: 'InternalServerError', + ok: false, + stream: responseStream + }); + + const result = await provider.trySetCacheEntryStreamAsync(terminal, 'some-key', entryStream); + + expect(result).toBe(false); + // maxAttempts is 1 for stream uploads, so only 1 call + expect(streamFetchFn).toHaveBeenCalledTimes(1); + }); + + it('skips credential fallback for stream bodies on 4xx', async () => { + jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue(undefined); + + const session: RushSession = {} as RushSession; + const provider = new HttpBuildCacheProvider(WRITE_ALLOWED_OPTIONS, session); + const entryStream = new Readable({ read() {} }); + const responseStream = new Readable({ read() {} }); + + mocked(streamFetchFn).mockResolvedValue({ + status: 401, + statusText: 'Unauthorized', + ok: false, + stream: responseStream + }); + + // Even though credentials are optional and we got a 4xx, the stream body + // should prevent the credential fallback retry since the stream is consumed + const result = await provider.trySetCacheEntryStreamAsync(terminal, 'some-key', entryStream); + + expect(result).toBe(false); + // Should only be called once (no credential fallback retry) + expect(streamFetchFn).toHaveBeenCalledTimes(1); + }); }); }); From bddd4ecdbcd52166e5aba941f78be01fdebd9fe7 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 5 Apr 2026 21:43:13 -0700 Subject: [PATCH 27/45] Fix buffer path error message to show specific unsupported encoding value The streaming path was fixed in 8becd082 but the buffer path still printed the entire encodings array instead of the individual value. Co-Authored-By: Claude Opus 4.6 (1M context) --- libraries/rush-lib/src/utilities/WebClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/rush-lib/src/utilities/WebClient.ts b/libraries/rush-lib/src/utilities/WebClient.ts index 20547a93428..dbcdf0ee969 100644 --- a/libraries/rush-lib/src/utilities/WebClient.ts +++ b/libraries/rush-lib/src/utilities/WebClient.ts @@ -246,7 +246,7 @@ const makeRequestAsync: FetchFn = async ( break; } default: { - throw new Error(`Unsupported content-encoding: ${encodings}`); + throw new Error(`Unsupported content-encoding: ${encoding.trim()}`); } } From 84304f80228f6d4aae8a661b30cdd6438ec63cd8 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 5 Apr 2026 21:49:45 -0700 Subject: [PATCH 28/45] Extract _getObjectName and _validateWriteAllowed helpers in S3 provider Deduplicate the S3 prefix logic (repeated 4 times) into a helper, and extract the write-permission guard (repeated in buffer and stream set methods) to match the HTTP provider's pattern. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/AmazonS3BuildCacheProvider.ts | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts index 9305a070f36..e1490903b3d 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts @@ -155,7 +155,7 @@ export class AmazonS3BuildCacheProvider implements ICloudBuildCacheProvider { ): Promise { try { const client: AmazonS3Client = await this._getS3ClientAsync(terminal); - return await client.getObjectAsync(this._s3Prefix ? `${this._s3Prefix}/${cacheId}` : cacheId); + return await client.getObjectAsync(this._getObjectName(cacheId)); } catch (e) { terminal.writeWarningLine(`Error getting cache entry from S3: ${e}`); return undefined; @@ -167,16 +167,13 @@ export class AmazonS3BuildCacheProvider implements ICloudBuildCacheProvider { cacheId: string, objectBuffer: Buffer ): Promise { - if (!this.isCacheWriteAllowed) { - terminal.writeErrorLine('Writing to S3 cache is not allowed in the current configuration.'); + if (!this._validateWriteAllowed(terminal, cacheId)) { return false; } - terminal.writeDebugLine('Uploading object with cacheId: ', cacheId); - try { const client: AmazonS3Client = await this._getS3ClientAsync(terminal); - await client.uploadObjectAsync(this._s3Prefix ? `${this._s3Prefix}/${cacheId}` : cacheId, objectBuffer); + await client.uploadObjectAsync(this._getObjectName(cacheId), objectBuffer); return true; } catch (e) { terminal.writeWarningLine(`Error uploading cache entry to S3: ${e}`); @@ -190,7 +187,7 @@ export class AmazonS3BuildCacheProvider implements ICloudBuildCacheProvider { ): Promise { try { const client: AmazonS3Client = await this._getS3ClientAsync(terminal); - return await client.getObjectStreamAsync(this._s3Prefix ? `${this._s3Prefix}/${cacheId}` : cacheId); + return await client.getObjectStreamAsync(this._getObjectName(cacheId)); } catch (e) { terminal.writeWarningLine(`Error getting cache entry stream from S3: ${e}`); return undefined; @@ -202,19 +199,13 @@ export class AmazonS3BuildCacheProvider implements ICloudBuildCacheProvider { cacheId: string, entryStream: NodeJS.ReadableStream ): Promise { - if (!this.isCacheWriteAllowed) { - terminal.writeErrorLine('Writing to S3 cache is not allowed in the current configuration.'); + if (!this._validateWriteAllowed(terminal, cacheId)) { return false; } - terminal.writeDebugLine('Uploading object stream with cacheId: ', cacheId); - try { const client: AmazonS3Client = await this._getS3ClientAsync(terminal); - await client.uploadObjectStreamAsync( - this._s3Prefix ? `${this._s3Prefix}/${cacheId}` : cacheId, - entryStream as Readable - ); + await client.uploadObjectStreamAsync(this._getObjectName(cacheId), entryStream as Readable); return true; } catch (e) { terminal.writeWarningLine(`Error uploading cache entry stream to S3: ${e}`); @@ -222,6 +213,20 @@ export class AmazonS3BuildCacheProvider implements ICloudBuildCacheProvider { } } + private _getObjectName(cacheId: string): string { + return this._s3Prefix ? `${this._s3Prefix}/${cacheId}` : cacheId; + } + + private _validateWriteAllowed(terminal: ITerminal, cacheId: string): boolean { + if (!this.isCacheWriteAllowed) { + terminal.writeErrorLine('Writing to S3 cache is not allowed in the current configuration.'); + return false; + } + + terminal.writeDebugLine('Uploading object with cacheId: ', cacheId); + return true; + } + public async updateCachedCredentialAsync(terminal: ITerminal, credential: string): Promise { await CredentialCache.usingAsync( { From 8aedb56ef0fab5c58c3fe2cefdca2d9e5774f548 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 5 Apr 2026 21:54:51 -0700 Subject: [PATCH 29/45] Extract _getContentEncodings helper to deduplicate encoding parsing Both the buffer and streaming response paths duplicated the same Content-Encoding header parsing logic. Extract into a shared helper that returns a parsed string array or undefined. Co-Authored-By: Claude Opus 4.6 (1M context) --- libraries/rush-lib/src/utilities/WebClient.ts | 37 +++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/libraries/rush-lib/src/utilities/WebClient.ts b/libraries/rush-lib/src/utilities/WebClient.ts index dbcdf0ee969..6b4e0e4140d 100644 --- a/libraries/rush-lib/src/utilities/WebClient.ts +++ b/libraries/rush-lib/src/utilities/WebClient.ts @@ -95,6 +95,22 @@ const ACCEPT_HEADER_NAME: 'accept' = 'accept'; const USER_AGENT_HEADER_NAME: 'user-agent' = 'user-agent'; const CONTENT_ENCODING_HEADER_NAME: 'content-encoding' = 'content-encoding'; +/** + * Parses the Content-Encoding header into an array of encoding names, + * or returns `undefined` if decoding should be skipped. + */ +function _getContentEncodings( + headers: Record, + noDecode: boolean | undefined +): string[] | undefined { + if (!noDecode) { + const encodings: string | string[] | undefined = headers[CONTENT_ENCODING_HEADER_NAME]; + if (encodings) { + return Array.isArray(encodings) ? encodings : encodings.split(','); + } + } +} + type StreamFetchFn = ( url: string, options: IRequestOptions, @@ -222,15 +238,12 @@ const makeRequestAsync: FetchFn = async ( getBufferAsync: async () => { // Determine if the buffer is compressed and decode it if necessary if (decodedBuffer === undefined) { - let encodings: string | string[] | undefined = headers[CONTENT_ENCODING_HEADER_NAME]; - if (!noDecode && encodings !== undefined) { + const contentEncodings: string[] | undefined = _getContentEncodings(headers, noDecode); + if (contentEncodings) { const zlib: typeof import('zlib') = await import('node:zlib'); - if (!Array.isArray(encodings)) { - encodings = encodings.split(','); - } let buffer: Buffer = responseData; - for (const encoding of encodings) { + for (const encoding of contentEncodings) { let decompressFn: (buffer: Buffer, callback: import('zlib').CompressCallback) => void; switch (encoding.trim()) { case DEFLATE_ENCODING: { @@ -299,22 +312,16 @@ const makeStreamRequestAsync: StreamFetchFn = async ( // Handle Content-Encoding decompression for streaming responses, // matching the buffer-based path's behavior in getBufferAsync() - let encodings: string | string[] | undefined; - if (!noDecode) { - encodings = headers[CONTENT_ENCODING_HEADER_NAME]; - } + const contentEncodings: string[] | undefined = _getContentEncodings(headers, noDecode); - if (encodings !== undefined) { + if (contentEncodings) { // Resolve with a promise so we can lazily import zlib (same pattern as buffer path) resolve( (async () => { const zlib: typeof import('zlib') = await import('node:zlib'); - if (!Array.isArray(encodings)) { - encodings = encodings!.split(','); - } let resultStream: Readable = response; - for (const encoding of encodings) { + for (const encoding of contentEncodings) { switch (encoding.trim()) { case DEFLATE_ENCODING: { resultStream = resultStream.pipe(zlib.createInflate()); From 15c0e113810642e50b977d3b76d7dbc036390f59 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 5 Apr 2026 22:00:12 -0700 Subject: [PATCH 30/45] Scope cacheEntryBuffer to its branch, use cloudCacheHit flag Replace the outer-scoped cacheEntryBuffer (which was only used as a boolean flag at the cache-miss check) with an explicit cloudCacheHit boolean. This scopes the buffer into the else branch and makes the streaming path set the flag consistently too. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../logic/buildCache/OperationBuildCache.ts | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts index 6e5b772afac..cc3ba63901d 100644 --- a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts +++ b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts @@ -167,21 +167,19 @@ export class OperationBuildCache { let localCacheEntryPath: string | undefined = await this._localBuildCacheProvider.tryGetCacheEntryPathByIdAsync(terminal, cacheId); - let cacheEntryBuffer: Buffer | undefined; + let cloudCacheHit: boolean = false; let updateLocalCacheSuccess: boolean | undefined; if (!localCacheEntryPath && this._cloudBuildCacheProvider) { terminal.writeVerboseLine( 'This project was not found in the local build cache. Querying the cloud build cache.' ); - if ( - this._useStreamingBuildCache && - this._cloudBuildCacheProvider.tryGetCacheEntryStreamByIdAsync - ) { + if (this._useStreamingBuildCache && this._cloudBuildCacheProvider.tryGetCacheEntryStreamByIdAsync) { // Use streaming path to avoid loading the entire cache entry into memory const cacheEntryStream: NodeJS.ReadableStream | undefined = await this._cloudBuildCacheProvider.tryGetCacheEntryStreamByIdAsync(terminal, cacheId); if (cacheEntryStream) { + cloudCacheHit = true; try { localCacheEntryPath = await this._localBuildCacheProvider.trySetCacheEntryStreamAsync( terminal, @@ -195,11 +193,10 @@ export class OperationBuildCache { } } } else { - cacheEntryBuffer = await this._cloudBuildCacheProvider.tryGetCacheEntryBufferByIdAsync( - terminal, - cacheId - ); + const cacheEntryBuffer: Buffer | undefined = + await this._cloudBuildCacheProvider.tryGetCacheEntryBufferByIdAsync(terminal, cacheId); if (cacheEntryBuffer) { + cloudCacheHit = true; try { localCacheEntryPath = await this._localBuildCacheProvider.trySetCacheEntryBufferAsync( terminal, @@ -215,7 +212,7 @@ export class OperationBuildCache { } } - if (!localCacheEntryPath && !cacheEntryBuffer) { + if (!localCacheEntryPath && !cloudCacheHit) { terminal.writeVerboseLine('This project was not found in the build cache.'); return false; } @@ -349,10 +346,7 @@ export class OperationBuildCache { throw new InternalError('Expected the local cache entry path to be set.'); } - if ( - this._useStreamingBuildCache && - this._cloudBuildCacheProvider.trySetCacheEntryStreamAsync - ) { + if (this._useStreamingBuildCache && this._cloudBuildCacheProvider.trySetCacheEntryStreamAsync) { // Use streaming upload to avoid loading the entire cache entry into memory const entryStream: FileSystemReadStream = FileSystem.createReadStream(localCacheEntryPath); setCloudCacheEntryPromise = this._cloudBuildCacheProvider From 91c0e4d90bea7f86c151b46bc3ade42938d0c32e Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 5 Apr 2026 22:02:02 -0700 Subject: [PATCH 31/45] Reuse a single WebClient instance in HttpBuildCacheProvider A new WebClient was being constructed on every call to _makeHttpCoreRequestAsync. Since the provider never configures any WebClient instance state, a single class member suffices. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/HttpBuildCacheProvider.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts index 40a0688b19d..ebfc096122f 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts @@ -73,6 +73,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { private readonly _cacheKeyPrefix: string; private readonly _tokenHandler: IHttpBuildCacheTokenHandler | undefined; private readonly _minHttpRetryDelayMs: number; + private readonly _webClient: WebClient = new WebClient(); private __credentialCacheId: string | undefined; public get isCacheWriteAllowed(): boolean { @@ -385,7 +386,6 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { terminal.writeDebugLine(`[http-build-cache] request: ${method} ${url} ${bodyLength} bytes`); - const webClient: WebClient = new WebClient(); const fetchOptions: IGetFetchOptions | IFetchOptionsWithBody = { verb: method, headers: headers, @@ -395,8 +395,8 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { }; const response: IWebClientResponse | IWebClientStreamResponse = stream - ? await webClient.fetchStreamAsync(url, fetchOptions) - : await webClient.fetchAsync(url, fetchOptions); + ? await this._webClient.fetchStreamAsync(url, fetchOptions) + : await this._webClient.fetchAsync(url, fetchOptions); if (!response.ok) { // Drain the response body on stream responses so the connection can be reused From 48aecb254da280401e26f70f3b04731ee235b0da Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 5 Apr 2026 22:02:58 -0700 Subject: [PATCH 32/45] Use property shorthand in HttpBuildCacheProvider Replace verbose `terminal: terminal`, `headers: headers`, `body: body`, and `credential: credential` with ES6 shorthand property syntax. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/HttpBuildCacheProvider.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts index ebfc096122f..38644a773e0 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts @@ -112,7 +112,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { ): Promise { try { const result: boolean | Buffer = await this._makeHttpRequestAsync({ - terminal: terminal, + terminal, relUrl: `${this._cacheKeyPrefix}${cacheId}`, method: 'GET', body: undefined, @@ -139,7 +139,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { try { const result: boolean | Buffer = await this._makeHttpRequestAsync({ - terminal: terminal, + terminal, relUrl: `${this._cacheKeyPrefix}${cacheId}`, method: this._uploadMethod, body: objectBuffer, @@ -161,7 +161,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { ): Promise { try { const result: IWebClientStreamResponse | false = await this._makeHttpStreamRequestAsync({ - terminal: terminal, + terminal, relUrl: `${this._cacheKeyPrefix}${cacheId}`, method: 'GET', body: undefined, @@ -187,7 +187,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { try { const result: IWebClientStreamResponse | false = await this._makeHttpStreamRequestAsync({ - terminal: terminal, + terminal, relUrl: `${this._cacheKeyPrefix}${cacheId}`, method: this._uploadMethod, body: entryStream as Readable, @@ -214,7 +214,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { }, async (credentialsCache: CredentialCache) => { credentialsCache.setCacheEntry(this._credentialCacheId, { - credential: credential + credential }); await credentialsCache.saveIfModifiedAsync(); } @@ -388,8 +388,8 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { const fetchOptions: IGetFetchOptions | IFetchOptionsWithBody = { verb: method, - headers: headers, - body: body, + headers, + body, redirect: 'follow', timeoutMs: 0 // Disable timeout for streaming transfers of large cache entries }; From d22938a99f1540efcd81b86426a5513186b30928 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Sun, 5 Apr 2026 23:15:11 -0700 Subject: [PATCH 33/45] Fix unit test issues in streaming cache tests - Fix credential fallback test: mock CredentialCache so cached credentials are available, making the test actually validate the stream-body guard (previously, the test passed trivially because _tryGetCredentialsAsync would throw before making a second request) - Fix 504 statusText from 'BadGateway' to 'Gateway Timeout' - Replace fragile S3 upload snapshot that captured Readable internals (breaks on Node.js upgrades) with targeted assertions on URL, verb, headers, and body identity - Replace fail() + try/catch with expect().rejects.toThrow() - Move Readable import to top of S3 test file instead of per-test dynamic import Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/test/AmazonS3Client.test.ts | 99 +++++++++---------- .../__snapshots__/AmazonS3Client.test.ts.snap | 37 ------- .../src/test/HttpBuildCacheProvider.test.ts | 33 ++++--- 3 files changed, 65 insertions(+), 104 deletions(-) diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts index 6c518b1cfbe..e889ffd007e 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts @@ -5,6 +5,8 @@ jest.mock('@rushstack/rush-sdk/lib/utilities/WebClient', () => { return jest.requireActual('@microsoft/rush-lib/lib/utilities/WebClient'); }); +import { Readable } from 'node:stream'; + import { ConsoleTerminalProvider, Terminal } from '@rushstack/terminal'; import { WebClient } from '@rushstack/rush-sdk/lib/utilities/WebClient'; @@ -664,21 +666,18 @@ describe(AmazonS3Client.name, () => { status: number, statusText?: string ): Promise<{ result: NodeJS.ReadableStream | undefined; spy: jest.SpyInstance }> { - const { Readable } = await import('node:stream'); const mockStream = new Readable({ read() {} }); - const spy: jest.SpyInstance = jest - .spyOn(WebClient.prototype, 'fetchStreamAsync') - .mockReturnValue( - Promise.resolve({ - stream: mockStream, - headers: {}, - status, - statusText, - ok: status >= 200 && status < 300, - redirected: false - }) - ); + const spy: jest.SpyInstance = jest.spyOn(WebClient.prototype, 'fetchStreamAsync').mockReturnValue( + Promise.resolve({ + stream: mockStream, + headers: {}, + status, + statusText, + ok: status >= 200 && status < 300, + redirected: false + }) + ); const s3Client: AmazonS3Client = new AmazonS3Client(credentials, options, webClient, terminal); const result = await s3Client.getObjectStreamAsync(objectName); @@ -722,7 +721,6 @@ describe(AmazonS3Client.name, () => { describe('Uploading an object stream', () => { it('Throws an error if credentials are not provided', async () => { - const { Readable } = await import('node:stream'); const s3Client: AmazonS3Client = new AmazonS3Client( undefined, { s3Endpoint: 'http://foo.bar.baz', ...DUMMY_OPTIONS_WITHOUT_ENDPOINT }, @@ -731,31 +729,25 @@ describe(AmazonS3Client.name, () => { ); const mockStream = new Readable({ read() {} }); - try { - await s3Client.uploadObjectStreamAsync('temp', mockStream); - fail('Expected an exception to be thrown'); - } catch (e) { - expect(e).toMatchSnapshot(); - } + await expect(s3Client.uploadObjectStreamAsync('temp', mockStream)).rejects.toThrow( + 'Credentials are required to upload objects to S3.' + ); }); it('Uploads a stream successfully', async () => { - const { Readable } = await import('node:stream'); const mockStream = new Readable({ read() {} }); const responseStream = new Readable({ read() {} }); - const spy: jest.SpyInstance = jest - .spyOn(WebClient.prototype, 'fetchStreamAsync') - .mockReturnValue( - Promise.resolve({ - stream: responseStream, - headers: {}, - status: 200, - statusText: 'OK', - ok: true, - redirected: false - }) - ); + const spy: jest.SpyInstance = jest.spyOn(WebClient.prototype, 'fetchStreamAsync').mockReturnValue( + Promise.resolve({ + stream: responseStream, + headers: {}, + status: 200, + statusText: 'OK', + ok: true, + redirected: false + }) + ); const s3Client: AmazonS3Client = new AmazonS3Client( { @@ -771,27 +763,33 @@ describe(AmazonS3Client.name, () => { await s3Client.uploadObjectStreamAsync('abc123', mockStream); expect(spy).toHaveBeenCalledTimes(1); - expect(spy.mock.calls[0]).toMatchSnapshot(); + // Assert on URL and request options without snapshotting Readable internals, + // which are fragile across Node.js versions + const [url, options] = spy.mock.calls[0]; + expect(url).toBe('http://localhost:9000/abc123'); + expect(options.verb).toBe('PUT'); + expect(options.body).toBe(mockStream); + expect(options.headers['x-amz-content-sha256']).toBe('UNSIGNED-PAYLOAD'); + expect(options.headers['x-amz-date']).toBe('20200418T123242Z'); + // eslint-disable-next-line dot-notation + expect(options.headers['Authorization']).toContain('AWS4-HMAC-SHA256'); spy.mockRestore(); }); it('Does not retry on failure (stream consumed)', async () => { - const { Readable } = await import('node:stream'); const mockStream = new Readable({ read() {} }); const responseStream = new Readable({ read() {} }); - const spy: jest.SpyInstance = jest - .spyOn(WebClient.prototype, 'fetchStreamAsync') - .mockReturnValue( - Promise.resolve({ - stream: responseStream, - headers: {}, - status: 500, - statusText: 'InternalServerError', - ok: false, - redirected: false - }) - ); + const spy: jest.SpyInstance = jest.spyOn(WebClient.prototype, 'fetchStreamAsync').mockReturnValue( + Promise.resolve({ + stream: responseStream, + headers: {}, + status: 500, + statusText: 'InternalServerError', + ok: false, + redirected: false + }) + ); const s3Client: AmazonS3Client = new AmazonS3Client( { @@ -804,12 +802,7 @@ describe(AmazonS3Client.name, () => { terminal ); - try { - await s3Client.uploadObjectStreamAsync('abc123', mockStream); - fail('Expected an exception to be thrown'); - } catch (e) { - expect((e as Error).message).toContain('500'); - } + await expect(s3Client.uploadObjectStreamAsync('abc123', mockStream)).rejects.toThrow('500'); // Only 1 call - no retry for streams expect(spy).toHaveBeenCalledTimes(1); diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/__snapshots__/AmazonS3Client.test.ts.snap b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/__snapshots__/AmazonS3Client.test.ts.snap index b190f855e48..4f842dd174d 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/__snapshots__/AmazonS3Client.test.ts.snap +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/__snapshots__/AmazonS3Client.test.ts.snap @@ -563,40 +563,3 @@ Array [ ] `; -exports[`AmazonS3Client Streaming requests Uploading an object stream Throws an error if credentials are not provided 1`] = `[Error: Credentials are required to upload objects to S3.]`; - -exports[`AmazonS3Client Streaming requests Uploading an object stream Uploads a stream successfully 1`] = ` -Array [ - "http://localhost:9000/abc123", - Object { - "body": Readable { - "_events": Object { - "close": undefined, - "data": undefined, - "end": undefined, - "error": undefined, - "readable": undefined, - }, - "_maxListeners": undefined, - "_read": [Function], - "_readableState": ReadableState { - "awaitDrainWriters": null, - "buffer": Array [], - "bufferIndex": 0, - "highWaterMark": 65536, - "length": 0, - "pipes": Array [], - Symbol(kState): 1052940, - }, - Symbol(shapeMode): true, - Symbol(kCapture): false, - }, - "headers": Object { - "Authorization": "AWS4-HMAC-SHA256 Credential=accessKeyId/20200418/us-east-1/s3/aws4_request,SignedHeaders=host;x-amz-content-sha256;x-amz-date,Signature=bfc5ee97ccfdb44b7f351c0b550a1ac9c2cdb661606dbba8a74c11be6b5b2b72", - "x-amz-content-sha256": "UNSIGNED-PAYLOAD", - "x-amz-date": "20200418T123242Z", - }, - "verb": "PUT", - }, -] -`; diff --git a/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts b/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts index dc572338b5e..691c6d2de00 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts @@ -8,6 +8,7 @@ jest.mock('@rushstack/rush-sdk/lib/utilities/WebClient', () => { import { Readable } from 'node:stream'; import { type RushSession, EnvironmentConfiguration } from '@rushstack/rush-sdk'; +import { type ICredentialCacheEntry, CredentialCache } from '@rushstack/credential-cache'; import { StringBufferTerminalProvider, Terminal } from '@rushstack/terminal'; import { WebClient } from '@rushstack/rush-sdk/lib/utilities/WebClient'; @@ -113,7 +114,7 @@ Array [ }); mocked(fetchFn).mockResolvedValueOnce({ status: 504, - statusText: 'BadGateway', + statusText: 'Gateway Timeout', ok: false }); @@ -149,7 +150,7 @@ Array [ "[ debug] [http-build-cache] request: GET https://buildcache.example.acme.com/some-key unknown bytes[n]", "[ debug] [http-build-cache] request: GET https://buildcache.example.acme.com/some-key unknown bytes[n]", "[ debug] [http-build-cache] request: GET https://buildcache.example.acme.com/some-key unknown bytes[n]", - "[warning] Could not get cache entry: HTTP 504: BadGateway[n]", + "[warning] Could not get cache entry: HTTP 504: Gateway Timeout[n]", ] `); }); @@ -183,11 +184,7 @@ Array [ const session: RushSession = {} as RushSession; const provider = new HttpBuildCacheProvider(EXAMPLE_OPTIONS, session); // write not allowed - const result = await provider.trySetCacheEntryBufferAsync( - terminal, - 'some-key', - Buffer.from('data') - ); + const result = await provider.trySetCacheEntryBufferAsync(terminal, 'some-key', Buffer.from('data')); expect(result).toBe(false); expect(fetchFn).not.toHaveBeenCalled(); @@ -235,11 +232,7 @@ Array [ ok: false }); - const result = await provider.trySetCacheEntryBufferAsync( - terminal, - 'some-key', - Buffer.from('data') - ); + const result = await provider.trySetCacheEntryBufferAsync(terminal, 'some-key', Buffer.from('data')); expect(result).toBe(false); expect(fetchFn).toHaveBeenCalledTimes(3); @@ -316,7 +309,7 @@ Array [ }); mocked(streamFetchFn).mockResolvedValueOnce({ status: 504, - statusText: 'BadGateway', + statusText: 'Gateway Timeout', ok: false, stream: createMockStream() }); @@ -393,7 +386,19 @@ Array [ }); it('skips credential fallback for stream bodies on 4xx', async () => { + // No credential in env for the first attempt jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue(undefined); + // But credentials ARE available in the credential cache — without the stream-body + // guard, the credential fallback would resolve these and make a second HTTP request + // with the already-consumed stream body. + jest + .spyOn(CredentialCache, 'usingAsync') + // eslint-disable-next-line @typescript-eslint/naming-convention + .mockImplementation(async (_options, fn) => { + await (fn as (cache: CredentialCache) => Promise)({ + tryGetCacheEntry: (): ICredentialCacheEntry => ({ credential: 'cached-token' }) + } as unknown as CredentialCache); + }); const session: RushSession = {} as RushSession; const provider = new HttpBuildCacheProvider(WRITE_ALLOWED_OPTIONS, session); @@ -412,7 +417,7 @@ Array [ const result = await provider.trySetCacheEntryStreamAsync(terminal, 'some-key', entryStream); expect(result).toBe(false); - // Should only be called once (no credential fallback retry) + // Should only be called once (no credential fallback retry with consumed stream) expect(streamFetchFn).toHaveBeenCalledTimes(1); }); }); From b44bc44aeb97f6ca5f661196b21c031c6c045e4a Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Mon, 6 Apr 2026 23:09:58 -0700 Subject: [PATCH 34/45] Rework streaming cache APIs to file-based APIs with S3 payload signing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace stream-based ICloudBuildCacheProvider methods with file-path-based alternatives that give providers full control over I/O: - tryGetCacheEntryStreamByIdAsync → tryGetCacheEntryToFileAsync - trySetCacheEntryStreamAsync → trySetCacheEntryFromFileAsync Key improvements: - S3 uploads now hash the tarball on disk before streaming, restoring AWS Signature V4 payload signing (removes UNSIGNED_PAYLOAD) - Azure provider uses SDK-native uploadFile/downloadToFile - HTTP provider uses FileSystem.createReadStream/createWriteStreamAsync - Providers that don't need pre-upload computation (HTTP, Azure) don't pay the cost of hashing - Experiment renamed to useDirectFileTransfersForBuildCache Co-Authored-By: Claude Opus 4.6 (1M context) --- .../rush-amazon-s3-build-cache-plugin.api.md | 5 +- common/reviews/api/rush-lib.api.md | 11 +- .../src/api/ExperimentsConfiguration.ts | 6 +- .../cli/scriptActions/PhasedScriptAction.ts | 4 +- .../FileSystemBuildCacheProvider.ts | 63 ++--------- .../buildCache/ICloudBuildCacheProvider.ts | 23 ++-- .../logic/buildCache/OperationBuildCache.ts | 80 +++++++------- .../test/OperationBuildCache.test.ts | 2 +- .../operations/CacheableOperationPlugin.ts | 18 +-- .../src/schemas/experiments.schema.json | 4 +- .../src/AmazonS3BuildCacheProvider.ts | 23 ++-- .../src/AmazonS3Client.ts | 103 +++++++++++------- .../src/test/AmazonS3Client.test.ts | 65 +++++++---- .../__snapshots__/AmazonS3Client.test.ts.snap | 2 +- .../src/AzureStorageBuildCacheProvider.ts | 40 +++---- .../src/BridgeCachePlugin.ts | 4 +- .../src/HttpBuildCacheProvider.ts | 35 ++++-- .../src/test/HttpBuildCacheProvider.test.ts | 51 +++++---- 18 files changed, 277 insertions(+), 262 deletions(-) diff --git a/common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md b/common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md index 1c390c5eae8..05745fddbb8 100644 --- a/common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md +++ b/common/reviews/api/rush-amazon-s3-build-cache-plugin.api.md @@ -15,11 +15,10 @@ import { WebClient } from '@rushstack/rush-sdk/lib/utilities/WebClient'; // @public export class AmazonS3Client { constructor(credentials: IAmazonS3Credentials | undefined, options: IAmazonS3BuildCacheProviderOptionsAdvanced, webClient: WebClient, terminal: ITerminal); + downloadObjectToFileAsync(objectName: string, localFilePath: string): Promise; // (undocumented) getObjectAsync(objectName: string): Promise; // (undocumented) - getObjectStreamAsync(objectName: string): Promise; - // (undocumented) _getSha256Hmac(key: string | Buffer, data: string): Buffer; // (undocumented) _getSha256Hmac(key: string | Buffer, data: string, encoding: 'hex'): string; @@ -27,7 +26,7 @@ export class AmazonS3Client { static tryDeserializeCredentials(credentialString: string | undefined): IAmazonS3Credentials | undefined; // (undocumented) uploadObjectAsync(objectName: string, objectBuffer: Buffer): Promise; - uploadObjectStreamAsync(objectName: string, objectStream: NodeJS.ReadableStream): Promise; + uploadObjectFromFileAsync(objectName: string, localFilePath: string): Promise; // (undocumented) static UriEncode(input: string): string; } diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index f452d1ab4a6..c0cc3cff49b 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -312,10 +312,9 @@ export class ExperimentsConfiguration { // @beta export class FileSystemBuildCacheProvider { constructor(options: IFileSystemBuildCacheProviderOptions); - getCacheEntryPath(cacheId: string): string; + readonly getCacheEntryPath: (cacheId: string) => string; tryGetCacheEntryPathByIdAsync(terminal: ITerminal, cacheId: string): Promise; trySetCacheEntryBufferAsync(terminal: ITerminal, cacheId: string, entryBuffer: Buffer): Promise; - trySetCacheEntryStreamAsync(terminal: ITerminal, cacheId: string, entryStream: NodeJS.ReadableStream): Promise; } // @internal @@ -348,10 +347,10 @@ export interface ICloudBuildCacheProvider { readonly isCacheWriteAllowed: boolean; // (undocumented) tryGetCacheEntryBufferByIdAsync(terminal: ITerminal, cacheId: string): Promise; - tryGetCacheEntryStreamByIdAsync?(terminal: ITerminal, cacheId: string): Promise; + tryGetCacheEntryToFileAsync?(terminal: ITerminal, cacheId: string, localFilePath: string): Promise; // (undocumented) trySetCacheEntryBufferAsync(terminal: ITerminal, cacheId: string, entryBuffer: Buffer): Promise; - trySetCacheEntryStreamAsync?(terminal: ITerminal, cacheId: string, entryStream: NodeJS.ReadableStream): Promise; + trySetCacheEntryFromFileAsync?(terminal: ITerminal, cacheId: string, localFilePath: string): Promise; // (undocumented) updateCachedCredentialAsync(terminal: ITerminal, credential: string): Promise; // (undocumented) @@ -484,12 +483,12 @@ export interface IExperimentsJson { omitImportersFromPreventManualShrinkwrapChanges?: boolean; printEventHooksOutputToConsole?: boolean; rushAlerts?: boolean; + useDirectFileTransfersForBuildCache?: boolean; useIPCScriptsInWatchMode?: boolean; usePnpmFrozenLockfileForRushInstall?: boolean; usePnpmLockfileOnlyThenFrozenLockfileForRushUpdate?: boolean; usePnpmPreferFrozenLockfileForRushUpdate?: boolean; usePnpmSyncForInjectedDependencies?: boolean; - useStreamingBuildCache?: boolean; } // @beta @@ -598,7 +597,7 @@ export interface _IOperationBuildCacheOptions { buildCacheConfiguration: BuildCacheConfiguration; excludeAppleDoubleFiles: boolean; terminal: ITerminal; - useStreamingBuildCache: boolean; + useDirectFileTransfersForBuildCache: boolean; } // @alpha diff --git a/libraries/rush-lib/src/api/ExperimentsConfiguration.ts b/libraries/rush-lib/src/api/ExperimentsConfiguration.ts index 75c4d654385..563917d1062 100644 --- a/libraries/rush-lib/src/api/ExperimentsConfiguration.ts +++ b/libraries/rush-lib/src/api/ExperimentsConfiguration.ts @@ -138,12 +138,12 @@ export interface IExperimentsJson { omitAppleDoubleFilesFromBuildCache?: boolean; /** - * If true, the build cache will use streaming APIs to transfer cache entries to and from cloud + * If true, the build cache will use file-based APIs to transfer cache entries to and from cloud * storage. This avoids loading the entire cache entry into memory, which can prevent out-of-memory * errors for large build outputs. The cloud cache provider plugin must implement the optional - * streaming methods for this to take effect; otherwise it falls back to the buffer-based approach. + * file-based methods for this to take effect; otherwise it falls back to the buffer-based approach. */ - useStreamingBuildCache?: boolean; + useDirectFileTransfersForBuildCache?: boolean; } const _EXPERIMENTS_JSON_SCHEMA: JsonSchema = JsonSchema.fromLoadedObject(schemaJson); diff --git a/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts b/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts index 09ecbbe410c..90b435d9095 100644 --- a/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts +++ b/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts @@ -522,7 +522,7 @@ export class PhasedScriptAction extends BaseScriptAction i buildCacheWithAllowWarningsInSuccessfulBuild = false, buildSkipWithAllowWarningsInSuccessfulBuild, omitAppleDoubleFilesFromBuildCache: excludeAppleDoubleFiles = false, - useStreamingBuildCache = false, + useDirectFileTransfersForBuildCache = false, usePnpmSyncForInjectedDependencies } }, @@ -536,7 +536,7 @@ export class PhasedScriptAction extends BaseScriptAction i cobuildConfiguration, terminal, excludeAppleDoubleFiles, - useStreamingBuildCache + useDirectFileTransfersForBuildCache }).apply(this.hooks); if (this._debugBuildCacheIdsParameter.value) { diff --git a/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts b/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts index c37eb39fe09..96af3eebb27 100644 --- a/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts +++ b/libraries/rush-lib/src/logic/buildCache/FileSystemBuildCacheProvider.ts @@ -1,10 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import * as path from 'node:path'; -import { pipeline } from 'node:stream/promises'; - -import { FileSystem, type FileSystemWriteStream } from '@rushstack/node-core-library'; +import { FileSystem } from '@rushstack/node-core-library'; import type { ITerminal } from '@rushstack/terminal'; import type { RushConfiguration } from '../../api/RushConfiguration'; @@ -33,21 +30,17 @@ const DEFAULT_BUILD_CACHE_FOLDER_NAME: string = 'build-cache'; * @beta */ export class FileSystemBuildCacheProvider { - private readonly _cacheFolderPath: string; + /** + * Returns the absolute disk path for the specified cache id. + */ + public readonly getCacheEntryPath: (cacheId: string) => string; public constructor(options: IFileSystemBuildCacheProviderOptions) { const { - rushUserConfiguration: { buildCacheFolder }, - rushConfiguration: { commonTempFolder } + rushConfiguration: { commonTempFolder }, + rushUserConfiguration: { buildCacheFolder = `${commonTempFolder}/${DEFAULT_BUILD_CACHE_FOLDER_NAME}` } } = options; - this._cacheFolderPath = buildCacheFolder || path.join(commonTempFolder, DEFAULT_BUILD_CACHE_FOLDER_NAME); - } - - /** - * Returns the absolute disk path for the specified cache id. - */ - public getCacheEntryPath(cacheId: string): string { - return path.join(this._cacheFolderPath, cacheId); + this.getCacheEntryPath = (cacheId: string) => `${buildCacheFolder}/${cacheId}`; } /** @@ -73,47 +66,9 @@ export class FileSystemBuildCacheProvider { terminal: ITerminal, cacheId: string, entryBuffer: Buffer - ): Promise { - return await this._setCacheEntryAsync( - terminal, - cacheId, - async (cacheEntryFilePath: string): Promise => { - await FileSystem.writeFileAsync(cacheEntryFilePath, entryBuffer, { ensureFolderExists: true }); - } - ); - } - - /** - * Writes the specified stream to the corresponding file system path for the cache id. - * This avoids loading the entire cache entry into memory. - */ - public async trySetCacheEntryStreamAsync( - terminal: ITerminal, - cacheId: string, - entryStream: NodeJS.ReadableStream - ): Promise { - return await this._setCacheEntryAsync( - terminal, - cacheId, - async (cacheEntryFilePath: string): Promise => { - const writeStream: FileSystemWriteStream = await FileSystem.createWriteStreamAsync( - cacheEntryFilePath, - { - ensureFolderExists: true - } - ); - await pipeline(entryStream, writeStream); - } - ); - } - - private async _setCacheEntryAsync( - terminal: ITerminal, - cacheId: string, - setEntryCallbackAsync: (cacheEntryFilePath: string) => Promise ): Promise { const cacheEntryFilePath: string = this.getCacheEntryPath(cacheId); - await setEntryCallbackAsync(cacheEntryFilePath); + await FileSystem.writeFileAsync(cacheEntryFilePath, entryBuffer, { ensureFolderExists: true }); terminal.writeVerboseLine(`Wrote cache entry to "${cacheEntryFilePath}".`); return cacheEntryFilePath; } diff --git a/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts b/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts index 571de7db90a..7499a7be25c 100644 --- a/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts +++ b/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts @@ -15,26 +15,25 @@ export interface ICloudBuildCacheProvider { /** * If implemented, the build cache will prefer to use this method over * {@link ICloudBuildCacheProvider.tryGetCacheEntryBufferByIdAsync} to avoid loading the entire - * cache entry into memory. + * cache entry into memory, if possible. The implementation should download the cache entry and write it + * to the specified local file path. + * + * @returns `true` if the cache entry was found and written to the file, `false` if it was + * not found. Throws on errors. */ - tryGetCacheEntryStreamByIdAsync?( - terminal: ITerminal, - cacheId: string - ): Promise; + tryGetCacheEntryToFileAsync?(terminal: ITerminal, cacheId: string, localFilePath: string): Promise; /** * If implemented, the build cache will prefer to use this method over * {@link ICloudBuildCacheProvider.trySetCacheEntryBufferAsync} to avoid loading the entire - * cache entry into memory. + * cache entry into memory, if possible. The implementation should read the cache entry from + * the specified local file path and upload it. * - * @remarks - * Because the provided stream can only be consumed once, implementations should not - * attempt to retry the upload using the same stream. If retry logic is needed, - * consider buffering internally or returning `false` so the caller can retry. + * @returns `true` if the cache entry was written to the cache, otherwise `false`. */ - trySetCacheEntryStreamAsync?( + trySetCacheEntryFromFileAsync?( terminal: ITerminal, cacheId: string, - entryStream: NodeJS.ReadableStream + localFilePath: string ): Promise; updateCachedCredentialAsync(terminal: ITerminal, credential: string): Promise; diff --git a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts index cc3ba63901d..9c1cd27f985 100644 --- a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts +++ b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts @@ -4,13 +4,7 @@ import * as path from 'node:path'; import * as crypto from 'node:crypto'; -import { - FileSystem, - type FileSystemReadStream, - type FolderItem, - InternalError, - Async -} from '@rushstack/node-core-library'; +import { FileSystem, type FolderItem, InternalError, Async } from '@rushstack/node-core-library'; import type { ITerminal } from '@rushstack/terminal'; import type { RushConfigurationProject } from '../../api/RushConfigurationProject'; @@ -39,10 +33,10 @@ export interface IOperationBuildCacheOptions { */ excludeAppleDoubleFiles: boolean; /** - * If true, use streaming APIs (when available) to transfer cache entries to and from the + * If true, use file-based APIs (when available) to transfer cache entries to and from the * cloud provider, avoiding buffering the entire entry in memory. */ - useStreamingBuildCache: boolean; + useDirectFileTransfersForBuildCache: boolean; } /** @@ -86,7 +80,7 @@ export class OperationBuildCache { private readonly _projectOutputFolderNames: ReadonlyArray; private readonly _cacheId: string | undefined; private readonly _excludeAppleDoubleFiles: boolean; - private readonly _useStreamingBuildCache: boolean; + private readonly _useDirectFileTransfersForBuildCache: boolean; private constructor(cacheId: string | undefined, options: IProjectBuildCacheOptions) { const { @@ -99,7 +93,7 @@ export class OperationBuildCache { project, projectOutputFolderNames, excludeAppleDoubleFiles, - useStreamingBuildCache + useDirectFileTransfersForBuildCache } = options; this._project = project; this._localBuildCacheProvider = localCacheProvider; @@ -109,7 +103,7 @@ export class OperationBuildCache { this._projectOutputFolderNames = projectOutputFolderNames || []; this._cacheId = cacheId; this._excludeAppleDoubleFiles = excludeAppleDoubleFiles && process.platform === 'darwin'; - this._useStreamingBuildCache = useStreamingBuildCache; + this._useDirectFileTransfersForBuildCache = useDirectFileTransfersForBuildCache; } private static _tryGetTarUtility(terminal: ITerminal): Promise { @@ -133,7 +127,12 @@ export class OperationBuildCache { executionResult: IOperationExecutionResult, options: IOperationBuildCacheOptions ): OperationBuildCache { - const { buildCacheConfiguration, terminal, excludeAppleDoubleFiles, useStreamingBuildCache } = options; + const { + buildCacheConfiguration, + terminal, + excludeAppleDoubleFiles, + useDirectFileTransfersForBuildCache + } = options; const outputFolders: string[] = [...(executionResult.operation.settings?.outputFolderNames ?? [])]; if (executionResult.metadataFolderPath) { outputFolders.push(executionResult.metadataFolderPath); @@ -147,7 +146,7 @@ export class OperationBuildCache { projectOutputFolderNames: outputFolders, operationStateHash: executionResult.getStateHash(), excludeAppleDoubleFiles, - useStreamingBuildCache + useDirectFileTransfersForBuildCache }; const cacheId: string | undefined = OperationBuildCache._getCacheId(buildCacheOptions); return new OperationBuildCache(cacheId, buildCacheOptions); @@ -174,23 +173,26 @@ export class OperationBuildCache { 'This project was not found in the local build cache. Querying the cloud build cache.' ); - if (this._useStreamingBuildCache && this._cloudBuildCacheProvider.tryGetCacheEntryStreamByIdAsync) { - // Use streaming path to avoid loading the entire cache entry into memory - const cacheEntryStream: NodeJS.ReadableStream | undefined = - await this._cloudBuildCacheProvider.tryGetCacheEntryStreamByIdAsync(terminal, cacheId); - if (cacheEntryStream) { - cloudCacheHit = true; - try { - localCacheEntryPath = await this._localBuildCacheProvider.trySetCacheEntryStreamAsync( - terminal, - cacheId, - cacheEntryStream - ); + if ( + this._useDirectFileTransfersForBuildCache && + this._cloudBuildCacheProvider.tryGetCacheEntryToFileAsync + ) { + // Use file-based path to avoid loading the entire cache entry into memory. + // The provider downloads directly to the local cache file. + const targetPath: string = this._localBuildCacheProvider.getCacheEntryPath(cacheId); + try { + cloudCacheHit = await this._cloudBuildCacheProvider.tryGetCacheEntryToFileAsync( + terminal, + cacheId, + targetPath + ); + if (cloudCacheHit) { + localCacheEntryPath = targetPath; updateLocalCacheSuccess = true; - } catch (e) { - terminal.writeVerboseLine(`Failed to update local cache: ${e}`); - updateLocalCacheSuccess = false; } + } catch (e) { + terminal.writeVerboseLine(`Failed to update local cache: ${e}`); + updateLocalCacheSuccess = false; } } else { const cacheEntryBuffer: Buffer | undefined = @@ -346,15 +348,17 @@ export class OperationBuildCache { throw new InternalError('Expected the local cache entry path to be set.'); } - if (this._useStreamingBuildCache && this._cloudBuildCacheProvider.trySetCacheEntryStreamAsync) { - // Use streaming upload to avoid loading the entire cache entry into memory - const entryStream: FileSystemReadStream = FileSystem.createReadStream(localCacheEntryPath); - setCloudCacheEntryPromise = this._cloudBuildCacheProvider - .trySetCacheEntryStreamAsync(terminal, cacheId, entryStream) - .catch((e: Error) => { - entryStream.destroy(); - throw e; - }); + if ( + this._useDirectFileTransfersForBuildCache && + this._cloudBuildCacheProvider.trySetCacheEntryFromFileAsync + ) { + // Use file-based upload to avoid loading the entire cache entry into memory. + // The provider reads from the local cache file directly. + setCloudCacheEntryPromise = this._cloudBuildCacheProvider.trySetCacheEntryFromFileAsync( + terminal, + cacheId, + localCacheEntryPath + ); } else { const cacheEntryBuffer: Buffer = await FileSystem.readFileToBufferAsync(localCacheEntryPath); setCloudCacheEntryPromise = this._cloudBuildCacheProvider.trySetCacheEntryBufferAsync( diff --git a/libraries/rush-lib/src/logic/buildCache/test/OperationBuildCache.test.ts b/libraries/rush-lib/src/logic/buildCache/test/OperationBuildCache.test.ts index c5e99901d60..c4f10daa36a 100644 --- a/libraries/rush-lib/src/logic/buildCache/test/OperationBuildCache.test.ts +++ b/libraries/rush-lib/src/logic/buildCache/test/OperationBuildCache.test.ts @@ -60,7 +60,7 @@ describe(OperationBuildCache.name, () => { terminal, phaseName: 'build', excludeAppleDoubleFiles: !!options.excludeAppleDoubleFiles, - useStreamingBuildCache: false + useDirectFileTransfersForBuildCache: false }); return subject; diff --git a/libraries/rush-lib/src/logic/operations/CacheableOperationPlugin.ts b/libraries/rush-lib/src/logic/operations/CacheableOperationPlugin.ts index 3ad8d73a170..5e7468fd8d6 100644 --- a/libraries/rush-lib/src/logic/operations/CacheableOperationPlugin.ts +++ b/libraries/rush-lib/src/logic/operations/CacheableOperationPlugin.ts @@ -77,7 +77,7 @@ export interface ICacheableOperationPluginOptions { cobuildConfiguration: CobuildConfiguration | undefined; terminal: ITerminal; excludeAppleDoubleFiles: boolean; - useStreamingBuildCache: boolean; + useDirectFileTransfersForBuildCache: boolean; } interface ITryGetOperationBuildCacheOptionsBase { @@ -85,7 +85,7 @@ interface ITryGetOperationBuildCacheOptionsBase { buildCacheConfiguration: BuildCacheConfiguration | undefined; terminal: ITerminal; excludeAppleDoubleFiles: boolean; - useStreamingBuildCache: boolean; + useDirectFileTransfersForBuildCache: boolean; record: TRecord; } @@ -111,7 +111,7 @@ export class CacheableOperationPlugin implements IPhasedCommandPlugin { buildCacheConfiguration, cobuildConfiguration, excludeAppleDoubleFiles, - useStreamingBuildCache + useDirectFileTransfersForBuildCache } = this._options; hooks.beforeExecuteOperations.tap( @@ -276,7 +276,7 @@ export class CacheableOperationPlugin implements IPhasedCommandPlugin { terminal: buildCacheTerminal, record, excludeAppleDoubleFiles, - useStreamingBuildCache + useDirectFileTransfersForBuildCache }); // Try to acquire the cobuild lock @@ -296,7 +296,7 @@ export class CacheableOperationPlugin implements IPhasedCommandPlugin { record, terminal: buildCacheTerminal, excludeAppleDoubleFiles, - useStreamingBuildCache + useDirectFileTransfersForBuildCache }); if (operationBuildCache) { buildCacheTerminal.writeVerboseLine( @@ -596,7 +596,7 @@ export class CacheableOperationPlugin implements IPhasedCommandPlugin { terminal, record, excludeAppleDoubleFiles, - useStreamingBuildCache + useDirectFileTransfersForBuildCache } = options; if (!buildCacheContext.operationBuildCache) { const { cacheDisabledReason } = buildCacheContext; @@ -614,7 +614,7 @@ export class CacheableOperationPlugin implements IPhasedCommandPlugin { buildCacheConfiguration, terminal, excludeAppleDoubleFiles, - useStreamingBuildCache + useDirectFileTransfersForBuildCache }); } @@ -632,7 +632,7 @@ export class CacheableOperationPlugin implements IPhasedCommandPlugin { record, terminal, excludeAppleDoubleFiles, - useStreamingBuildCache + useDirectFileTransfersForBuildCache } = options; if (!buildCacheConfiguration?.buildCacheEnabled) { @@ -664,7 +664,7 @@ export class CacheableOperationPlugin implements IPhasedCommandPlugin { operationStateHash, phaseName: associatedPhase.name, excludeAppleDoubleFiles, - useStreamingBuildCache + useDirectFileTransfersForBuildCache }); buildCacheContext.operationBuildCache = operationBuildCache; diff --git a/libraries/rush-lib/src/schemas/experiments.schema.json b/libraries/rush-lib/src/schemas/experiments.schema.json index 45e3d6fbcc8..d32934010ae 100644 --- a/libraries/rush-lib/src/schemas/experiments.schema.json +++ b/libraries/rush-lib/src/schemas/experiments.schema.json @@ -86,8 +86,8 @@ "description": "If true, when running on macOS, Rush will omit AppleDouble files (._*) from build cache archives when a companion file exists in the same directory. AppleDouble files are automatically created by macOS to store extended attributes on filesystems that don't support them, and should generally not be included in the shared build cache.", "type": "boolean" }, - "useStreamingBuildCache": { - "description": "If true, the build cache will use streaming APIs to transfer cache entries to and from cloud storage. This avoids loading the entire cache entry into memory, which can prevent out-of-memory errors for large build outputs. The cloud cache provider plugin must implement the optional streaming methods for this to take effect; otherwise it falls back to the buffer-based approach.", + "useDirectFileTransfersForBuildCache": { + "description": "If true, the build cache will use file-based APIs to transfer cache entries to and from cloud storage. This avoids loading the entire cache entry into memory, which can prevent out-of-memory errors for large build outputs. The cloud cache provider plugin must implement the optional file-based methods for this to take effect; otherwise it falls back to the buffer-based approach.", "type": "boolean" } }, diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts index e1490903b3d..6d7a129408c 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import type { Readable } from 'node:stream'; - import { type ICredentialCacheEntry, CredentialCache } from '@rushstack/credential-cache'; import type { ITerminal } from '@rushstack/terminal'; import { @@ -181,23 +179,24 @@ export class AmazonS3BuildCacheProvider implements ICloudBuildCacheProvider { } } - public async tryGetCacheEntryStreamByIdAsync( + public async tryGetCacheEntryToFileAsync( terminal: ITerminal, - cacheId: string - ): Promise { + cacheId: string, + localFilePath: string + ): Promise { try { const client: AmazonS3Client = await this._getS3ClientAsync(terminal); - return await client.getObjectStreamAsync(this._getObjectName(cacheId)); + return await client.downloadObjectToFileAsync(this._getObjectName(cacheId), localFilePath); } catch (e) { - terminal.writeWarningLine(`Error getting cache entry stream from S3: ${e}`); - return undefined; + terminal.writeWarningLine(`Error downloading cache entry from S3: ${e}`); + return false; } } - public async trySetCacheEntryStreamAsync( + public async trySetCacheEntryFromFileAsync( terminal: ITerminal, cacheId: string, - entryStream: NodeJS.ReadableStream + localFilePath: string ): Promise { if (!this._validateWriteAllowed(terminal, cacheId)) { return false; @@ -205,10 +204,10 @@ export class AmazonS3BuildCacheProvider implements ICloudBuildCacheProvider { try { const client: AmazonS3Client = await this._getS3ClientAsync(terminal); - await client.uploadObjectStreamAsync(this._getObjectName(cacheId), entryStream as Readable); + await client.uploadObjectFromFileAsync(this._getObjectName(cacheId), localFilePath); return true; } catch (e) { - terminal.writeWarningLine(`Error uploading cache entry stream to S3: ${e}`); + terminal.writeWarningLine(`Error uploading cache entry to S3: ${e}`); return false; } } diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts index ab85d24a41e..ac4b7bcb6aa 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts @@ -3,8 +3,14 @@ import * as crypto from 'node:crypto'; import type { Readable } from 'node:stream'; +import { pipeline } from 'node:stream/promises'; -import { Async } from '@rushstack/node-core-library'; +import { + Async, + FileSystem, + type FileSystemReadStream, + type FileSystemWriteStream +} from '@rushstack/node-core-library'; import { Colorize, type ITerminal } from '@rushstack/terminal'; import { type IGetFetchOptions, @@ -18,18 +24,12 @@ import { import type { IAmazonS3BuildCacheProviderOptionsAdvanced } from './AmazonS3BuildCacheProvider'; import { type IAmazonS3Credentials, fromRushEnv } from './AmazonS3Credentials'; -const CONTENT_HASH_HEADER_NAME: 'x-amz-content-sha256' = 'x-amz-content-sha256'; +const HASH_ALGORITHM: 'sha256' = 'sha256'; +const CONTENT_HASH_HEADER_NAME: `x-amz-content-${typeof HASH_ALGORITHM}` = `x-amz-content-${HASH_ALGORITHM}`; const DATE_HEADER_NAME: 'x-amz-date' = 'x-amz-date'; const HOST_HEADER_NAME: 'host' = 'host'; const SECURITY_TOKEN_HEADER_NAME: 'x-amz-security-token' = 'x-amz-security-token'; -/** - * AWS Signature V4 allows using this sentinel value as the content hash when the request - * payload is not signed. This is used for streaming uploads where the body cannot be hashed - * upfront. - */ -const UNSIGNED_PAYLOAD: 'UNSIGNED-PAYLOAD' = 'UNSIGNED-PAYLOAD'; - interface IIsoDateString { date: string; dateTime: string; @@ -69,6 +69,18 @@ const storageRetryOptions: IStorageRetryOptions = { retryPolicyType: StorageRetryPolicyType.EXPONENTIAL }; +/** + * Computes the SHA-256 hash of a file on disk using streaming reads. + */ +async function _hashFileAsync(filePath: string): Promise { + return await new Promise((resolve, reject) => { + const hash: crypto.Hash = crypto.createHash(HASH_ALGORITHM); + const stream: FileSystemReadStream = FileSystem.createReadStream(filePath); + stream.on('data', (chunk: string | Buffer) => hash.update(chunk)); + stream.on('end', () => resolve(hash.digest('hex'))); + stream.on('error', reject); + }); +} /** * A helper for reading and updating objects on Amazon S3 * @@ -163,20 +175,32 @@ export class AmazonS3Client { }); } - public async getObjectStreamAsync(objectName: string): Promise { - this._writeDebugLine('Reading object stream from S3'); - return await this._sendCacheRequestWithRetriesAsync(async () => { + /** + * Downloads an S3 object directly to a local file path, using streaming to avoid + * buffering the entire object in memory. Retries on transient network errors. + * + * @returns `true` if the object was found and written to the file, `false` if not found. + */ + public async downloadObjectToFileAsync(objectName: string, localFilePath: string): Promise { + this._writeDebugLine('Downloading object from S3 to file'); + const result: boolean | undefined = await this._sendCacheRequestWithRetriesAsync(async () => { const response: IWebClientStreamResponse = await this._makeSignedRequestAsync( 'GET', objectName, undefined, true ); - return this._handleGetResponseAsync( + return this._handleGetResponseAsync( response.status, response.statusText, response.ok, - () => response.stream, + async () => { + const writeStream: FileSystemWriteStream = await FileSystem.createWriteStreamAsync(localFilePath, { + ensureFolderExists: true + }); + await pipeline(response.stream, writeStream); + return true; + }, async () => { response.stream.resume(); return new Error( @@ -186,33 +210,35 @@ export class AmazonS3Client { () => response.stream.resume() ); }); + + return result ?? false; } /** - * Uploads a readable stream to S3. Unlike {@link AmazonS3Client.uploadObjectAsync}, this method - * does not use retry logic because the stream is consumed after the first attempt and cannot be - * replayed. The caller should handle failures accordingly. + * Uploads a local file to S3 using streaming, with the file's SHA-256 hash included in + * the AWS Signature V4 request for payload integrity verification. Does not retry + * because the stream is consumed after the first attempt. */ - public async uploadObjectStreamAsync( - objectName: string, - objectStream: NodeJS.ReadableStream - ): Promise { + public async uploadObjectFromFileAsync(objectName: string, localFilePath: string): Promise { if (!this._credentials) { throw new Error('Credentials are required to upload objects to S3.'); } + // Compute SHA-256 hash of the file before uploading so we can sign the payload + const contentHash: string = await _hashFileAsync(localFilePath); + const entryStream: FileSystemReadStream = FileSystem.createReadStream(localFilePath); + // Streaming uploads cannot be retried because the stream is consumed after the first attempt. const response: IWebClientStreamResponse = await this._makeSignedRequestAsync( 'PUT', objectName, - objectStream as Readable, - true + entryStream as Readable, + true, + contentHash ); if (!response.ok) { response.stream.resume(); - throw new Error( - `Amazon S3 responded with status code ${response.status} (${response.statusText})` - ); + throw new Error(`Amazon S3 responded with status code ${response.status} (${response.statusText})`); } response.stream.resume(); } @@ -260,10 +286,7 @@ export class AmazonS3Client { hasNetworkError: false, response: undefined }; - } else if ( - (status === 400 || status === 401 || status === 403) && - !this._credentials - ) { + } else if ((status === 400 || status === 401 || status === 403) && !this._credentials) { cleanup?.(); // unauthorized due to not providing credentials, // silence error for better DX when e.g. running locally without credentials @@ -299,19 +322,19 @@ export class AmazonS3Client { verb: 'GET' | 'PUT', objectName: string, body: Readable | undefined, - stream: true + stream: true, + contentHash?: string ): Promise; private async _makeSignedRequestAsync( verb: 'GET' | 'PUT', objectName: string, body?: Buffer | Readable, - stream?: boolean + stream?: boolean, + contentHash?: string ): Promise { - // For streaming uploads, the body cannot be hashed upfront, so we use UNSIGNED-PAYLOAD. - const isStreamBody: boolean = !!body && typeof (body as Readable).pipe === 'function'; - const bodyHash: string = isStreamBody - ? UNSIGNED_PAYLOAD - : this._getSha256(body as Buffer | undefined); + // Use the provided content hash if available (e.g. pre-computed from a file on disk), + // otherwise compute from the buffer body, or use the empty hash for GET requests. + const bodyHash: string = contentHash ?? this._getBufferSha256(Buffer.isBuffer(body) ? body : undefined); const { url, headers } = this._buildSignedRequest(verb, objectName, bodyHash); const webFetchOptions: IGetFetchOptions | IFetchOptionsWithBody = { @@ -401,7 +424,7 @@ export class AmazonS3Client { signedHeaderNamesString, bodyHash ].join('\n'); - const canonicalRequestHash: string = this._getSha256(canonicalRequest); + const canonicalRequestHash: string = this._getBufferSha256(canonicalRequest); const scope: string = `${isoDateString.date}/${this._s3Region}/s3/aws4_request`; // The string to sign looks like this: @@ -458,9 +481,9 @@ export class AmazonS3Client { } } - private _getSha256(data?: string | Buffer): string { + private _getBufferSha256(data?: string | Buffer): string { if (data) { - const hash: crypto.Hash = crypto.createHash('sha256'); + const hash: crypto.Hash = crypto.createHash(HASH_ALGORITHM); hash.update(data); return hash.digest('hex'); } else { diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts index e889ffd007e..bfcf6e254f5 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts @@ -5,9 +5,29 @@ jest.mock('@rushstack/rush-sdk/lib/utilities/WebClient', () => { return jest.requireActual('@microsoft/rush-lib/lib/utilities/WebClient'); }); +jest.mock('node:stream/promises', () => ({ + pipeline: jest.fn().mockResolvedValue(undefined) +})); + +jest.mock('node:fs', () => { + const { Readable: ReadableImpl } = jest.requireActual('node:stream'); + return { + createReadStream: jest.fn().mockImplementation(() => { + // Return a Readable that emits empty data then ends, so _hashFileAsync completes + return new ReadableImpl({ + read() { + this.push(null); + } + }); + }), + createWriteStream: jest.fn().mockReturnValue({}) + }; +}); + import { Readable } from 'node:stream'; import { ConsoleTerminalProvider, Terminal } from '@rushstack/terminal'; +import { FileSystem } from '@rushstack/node-core-library'; import { WebClient } from '@rushstack/rush-sdk/lib/utilities/WebClient'; import type { IAmazonS3BuildCacheProviderOptionsAdvanced } from '../AmazonS3BuildCacheProvider'; @@ -637,7 +657,7 @@ describe(AmazonS3Client.name, () => { }); }); - describe('Streaming requests', () => { + describe('File-based requests', () => { let realDate: typeof Date; let realSetTimeout: typeof setTimeout; beforeEach(() => { @@ -650,6 +670,8 @@ describe(AmazonS3Client.name, () => { global.setTimeout = ((callback: () => void, time: number) => { return realSetTimeout(callback, 1); }).bind(global) as typeof global.setTimeout; + + jest.spyOn(FileSystem, 'ensureFolderAsync').mockResolvedValue(); }); afterEach(() => { @@ -658,14 +680,14 @@ describe(AmazonS3Client.name, () => { global.setTimeout = realSetTimeout.bind(global); }); - describe('Getting an object stream', () => { - async function makeStreamGetRequestAsync( + describe('Downloading an object to file', () => { + async function makeFileGetRequestAsync( credentials: IAmazonS3Credentials | undefined, options: IAmazonS3BuildCacheProviderOptionsAdvanced, objectName: string, status: number, statusText?: string - ): Promise<{ result: NodeJS.ReadableStream | undefined; spy: jest.SpyInstance }> { + ): Promise<{ result: boolean; spy: jest.SpyInstance }> { const mockStream = new Readable({ read() {} }); const spy: jest.SpyInstance = jest.spyOn(WebClient.prototype, 'fetchStreamAsync').mockReturnValue( @@ -680,12 +702,12 @@ describe(AmazonS3Client.name, () => { ); const s3Client: AmazonS3Client = new AmazonS3Client(credentials, options, webClient, terminal); - const result = await s3Client.getObjectStreamAsync(objectName); + const result = await s3Client.downloadObjectToFileAsync(objectName, '/tmp/cache-entry'); return { result, spy }; } - it('Can get an object stream', async () => { - const { result, spy } = await makeStreamGetRequestAsync( + it('Can download an object to file', async () => { + const { result, spy } = await makeFileGetRequestAsync( { accessKeyId: 'accessKeyId', secretAccessKey: 'secretAccessKey', @@ -695,14 +717,14 @@ describe(AmazonS3Client.name, () => { 'abc123', 200 ); - expect(result).toBeDefined(); + expect(result).toBe(true); expect(spy).toHaveBeenCalledTimes(1); expect(spy.mock.calls[0]).toMatchSnapshot(); spy.mockRestore(); }); - it('Returns undefined for a 404 (missing) object stream', async () => { - const { result, spy } = await makeStreamGetRequestAsync( + it('Returns false for a 404 (missing) object', async () => { + const { result, spy } = await makeFileGetRequestAsync( { accessKeyId: 'accessKeyId', secretAccessKey: 'secretAccessKey', @@ -713,13 +735,13 @@ describe(AmazonS3Client.name, () => { 404, 'Not Found' ); - expect(result).toBeUndefined(); + expect(result).toBe(false); expect(spy).toHaveBeenCalledTimes(1); spy.mockRestore(); }); }); - describe('Uploading an object stream', () => { + describe('Uploading an object from file', () => { it('Throws an error if credentials are not provided', async () => { const s3Client: AmazonS3Client = new AmazonS3Client( undefined, @@ -728,14 +750,12 @@ describe(AmazonS3Client.name, () => { terminal ); - const mockStream = new Readable({ read() {} }); - await expect(s3Client.uploadObjectStreamAsync('temp', mockStream)).rejects.toThrow( + await expect(s3Client.uploadObjectFromFileAsync('temp', '/tmp/cache-entry')).rejects.toThrow( 'Credentials are required to upload objects to S3.' ); }); - it('Uploads a stream successfully', async () => { - const mockStream = new Readable({ read() {} }); + it('Uploads from file with signed payload hash', async () => { const responseStream = new Readable({ read() {} }); const spy: jest.SpyInstance = jest.spyOn(WebClient.prototype, 'fetchStreamAsync').mockReturnValue( @@ -760,16 +780,14 @@ describe(AmazonS3Client.name, () => { terminal ); - await s3Client.uploadObjectStreamAsync('abc123', mockStream); + await s3Client.uploadObjectFromFileAsync('abc123', '/tmp/cache-entry'); expect(spy).toHaveBeenCalledTimes(1); - // Assert on URL and request options without snapshotting Readable internals, - // which are fragile across Node.js versions const [url, options] = spy.mock.calls[0]; expect(url).toBe('http://localhost:9000/abc123'); expect(options.verb).toBe('PUT'); - expect(options.body).toBe(mockStream); - expect(options.headers['x-amz-content-sha256']).toBe('UNSIGNED-PAYLOAD'); + // Verify the content hash is a real SHA-256 hex string, NOT UNSIGNED-PAYLOAD + expect(options.headers['x-amz-content-sha256']).toMatch(/^[0-9a-f]{64}$/); expect(options.headers['x-amz-date']).toBe('20200418T123242Z'); // eslint-disable-next-line dot-notation expect(options.headers['Authorization']).toContain('AWS4-HMAC-SHA256'); @@ -777,7 +795,6 @@ describe(AmazonS3Client.name, () => { }); it('Does not retry on failure (stream consumed)', async () => { - const mockStream = new Readable({ read() {} }); const responseStream = new Readable({ read() {} }); const spy: jest.SpyInstance = jest.spyOn(WebClient.prototype, 'fetchStreamAsync').mockReturnValue( @@ -802,9 +819,9 @@ describe(AmazonS3Client.name, () => { terminal ); - await expect(s3Client.uploadObjectStreamAsync('abc123', mockStream)).rejects.toThrow('500'); + await expect(s3Client.uploadObjectFromFileAsync('abc123', '/tmp/cache-entry')).rejects.toThrow('500'); - // Only 1 call - no retry for streams + // Only 1 call - no retry for file-based uploads expect(spy).toHaveBeenCalledTimes(1); spy.mockRestore(); }); diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/__snapshots__/AmazonS3Client.test.ts.snap b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/__snapshots__/AmazonS3Client.test.ts.snap index 4f842dd174d..d6297a81131 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/__snapshots__/AmazonS3Client.test.ts.snap +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/__snapshots__/AmazonS3Client.test.ts.snap @@ -549,7 +549,7 @@ exports[`AmazonS3Client Rejects invalid S3 endpoint values 10`] = `"Invalid S3 e exports[`AmazonS3Client Rejects invalid S3 endpoint values 11`] = `"Invalid S3 endpoint. Some part of the hostname contains invalid characters or is too long"`; -exports[`AmazonS3Client Streaming requests Getting an object stream Can get an object stream 1`] = ` +exports[`AmazonS3Client File-based requests Downloading an object to file Can download an object to file 1`] = ` Array [ "http://localhost:9000/abc123", Object { diff --git a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts index 32f6bc154d4..550c032cd6f 100644 --- a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts +++ b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts @@ -1,11 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import type { Readable } from 'node:stream'; - import { type BlobClient, - type BlobDownloadResponseParsed, BlobServiceClient, type BlockBlobClient, type ContainerClient @@ -93,32 +90,31 @@ export class AzureStorageBuildCacheProvider }); } - public async tryGetCacheEntryStreamByIdAsync( - terminal: ITerminal, - cacheId: string - ): Promise { - return await this._tryGetBlobDataAsync(terminal, cacheId, async (blobClient: BlobClient) => { - const downloadResponse: BlobDownloadResponseParsed = await blobClient.download(); - return downloadResponse.readableStreamBody; - }); - } - - public async trySetCacheEntryStreamAsync( + public async tryGetCacheEntryToFileAsync( terminal: ITerminal, cacheId: string, - entryStream: NodeJS.ReadableStream + localFilePath: string ): Promise { - return await this._trySetBlobDataAsync( + const result: boolean | undefined = await this._tryGetBlobDataAsync( terminal, cacheId, - async (blockBlobClient: BlockBlobClient) => { - await blockBlobClient.uploadStream(entryStream as Readable); - }, - () => { - // Drain the incoming stream since we won't consume it - entryStream.resume(); + async (blobClient: BlobClient) => { + await blobClient.downloadToFile(localFilePath); + return true; } ); + + return result ?? false; + } + + public async trySetCacheEntryFromFileAsync( + terminal: ITerminal, + cacheId: string, + localFilePath: string + ): Promise { + return await this._trySetBlobDataAsync(terminal, cacheId, async (blockBlobClient: BlockBlobClient) => { + await blockBlobClient.uploadFile(localFilePath); + }); } /** diff --git a/rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts b/rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts index 1dc8135d7a1..edfcb890b97 100644 --- a/rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts +++ b/rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts @@ -88,7 +88,7 @@ export class BridgeCachePlugin implements IRushPlugin { experimentsConfiguration: { configuration: { omitAppleDoubleFilesFromBuildCache: excludeAppleDoubleFiles = false, - useStreamingBuildCache = false + useDirectFileTransfersForBuildCache = false } } } @@ -123,7 +123,7 @@ export class BridgeCachePlugin implements IRushPlugin { buildCacheConfiguration, terminal, excludeAppleDoubleFiles, - useStreamingBuildCache + useDirectFileTransfersForBuildCache } ); diff --git a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts index 38644a773e0..d6f1d50518e 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts @@ -3,9 +3,16 @@ import type { SpawnSyncReturns } from 'node:child_process'; import type { Readable } from 'node:stream'; +import { pipeline } from 'node:stream/promises'; import { type ICredentialCacheEntry, CredentialCache } from '@rushstack/credential-cache'; -import { Executable, Async } from '@rushstack/node-core-library'; +import { + Executable, + Async, + FileSystem, + type FileSystemWriteStream, + type FileSystemReadStream +} from '@rushstack/node-core-library'; import type { ITerminal } from '@rushstack/terminal'; import { type ICloudBuildCacheProvider, @@ -155,10 +162,11 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { } } - public async tryGetCacheEntryStreamByIdAsync( + public async tryGetCacheEntryToFileAsync( terminal: ITerminal, - cacheId: string - ): Promise { + cacheId: string, + localFilePath: string + ): Promise { try { const result: IWebClientStreamResponse | false = await this._makeHttpStreamRequestAsync({ terminal, @@ -169,28 +177,37 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { maxAttempts: MAX_HTTP_CACHE_ATTEMPTS }); - return result !== false ? result.stream : undefined; + if (result === false) { + return false; + } + + const writeStream: FileSystemWriteStream = await FileSystem.createWriteStreamAsync(localFilePath, { + ensureFolderExists: true + }); + await pipeline(result.stream, writeStream); + return true; } catch (e) { terminal.writeWarningLine(`Error getting cache entry: ${e}`); - return undefined; + return false; } } - public async trySetCacheEntryStreamAsync( + public async trySetCacheEntryFromFileAsync( terminal: ITerminal, cacheId: string, - entryStream: NodeJS.ReadableStream + localFilePath: string ): Promise { if (!this._validateWriteAllowed(terminal, cacheId)) { return false; } try { + const entryStream: FileSystemReadStream = FileSystem.createReadStream(localFilePath); const result: IWebClientStreamResponse | false = await this._makeHttpStreamRequestAsync({ terminal, relUrl: `${this._cacheKeyPrefix}${cacheId}`, method: this._uploadMethod, - body: entryStream as Readable, + body: entryStream, warningText: 'Could not write cache entry', // Streaming uploads cannot be retried because the stream is consumed maxAttempts: 1 diff --git a/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts b/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts index 691c6d2de00..1bef9056e83 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts @@ -5,10 +5,20 @@ jest.mock('@rushstack/rush-sdk/lib/utilities/WebClient', () => { return jest.requireActual('@microsoft/rush-lib/lib/utilities/WebClient'); }); +jest.mock('node:stream/promises', () => ({ + pipeline: jest.fn().mockResolvedValue(undefined) +})); + +jest.mock('node:fs', () => ({ + createReadStream: jest.fn().mockReturnValue({ pipe: jest.fn() }), + createWriteStream: jest.fn().mockReturnValue({}) +})); + import { Readable } from 'node:stream'; import { type RushSession, EnvironmentConfiguration } from '@rushstack/rush-sdk'; import { type ICredentialCacheEntry, CredentialCache } from '@rushstack/credential-cache'; +import { FileSystem } from '@rushstack/node-core-library'; import { StringBufferTerminalProvider, Terminal } from '@rushstack/terminal'; import { WebClient } from '@rushstack/rush-sdk/lib/utilities/WebClient'; @@ -48,6 +58,7 @@ describe('HttpBuildCacheProvider', () => { streamFetchFn = jest.fn(); WebClient.mockRequestFn(fetchFn as unknown as FetchFnType); WebClient.mockStreamRequestFn(streamFetchFn as unknown as StreamFetchFnType); + jest.spyOn(FileSystem, 'ensureFolderAsync').mockResolvedValue(); }); afterEach(() => { @@ -239,10 +250,10 @@ Array [ }); }); - // ── Stream-based GET ────────────────────────────────────────────────────── + // ── File-based GET ────────────────────────────────────────────────────── - describe('tryGetCacheEntryStreamByIdAsync', () => { - it('returns a stream on a successful response', async () => { + describe('tryGetCacheEntryToFileAsync', () => { + it('downloads to file on a successful response', async () => { jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue('token123'); const session: RushSession = {} as RushSession; @@ -258,8 +269,8 @@ Array [ stream: mockStream }); - const result = await provider.tryGetCacheEntryStreamByIdAsync(terminal, 'some-key'); - expect(result).toBe(mockStream); + const result = await provider.tryGetCacheEntryToFileAsync(terminal, 'some-key', '/tmp/cache-entry'); + expect(result).toBe(true); expect(streamFetchFn).toHaveBeenCalledTimes(1); expect(streamFetchFn).toHaveBeenCalledWith( 'https://buildcache.example.acme.com/some-key', @@ -270,7 +281,7 @@ Array [ ); }); - it('returns undefined on credential failure', async () => { + it('returns false on credential failure', async () => { jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue(undefined); const session: RushSession = {} as RushSession; @@ -284,8 +295,8 @@ Array [ stream: mockStream }); - const result = await provider.tryGetCacheEntryStreamByIdAsync(terminal, 'some-key'); - expect(result).toBe(undefined); + const result = await provider.tryGetCacheEntryToFileAsync(terminal, 'some-key', '/tmp/cache-entry'); + expect(result).toBe(false); }); it('retries up to 3 times on server error', async () => { @@ -314,32 +325,30 @@ Array [ stream: createMockStream() }); - const result = await provider.tryGetCacheEntryStreamByIdAsync(terminal, 'some-key'); - expect(result).toBe(undefined); + const result = await provider.tryGetCacheEntryToFileAsync(terminal, 'some-key', '/tmp/cache-entry'); + expect(result).toBe(false); expect(streamFetchFn).toHaveBeenCalledTimes(3); }); }); - // ── Stream-based SET ────────────────────────────────────────────────────── + // ── File-based SET ────────────────────────────────────────────────────── - describe('trySetCacheEntryStreamAsync', () => { + describe('trySetCacheEntryFromFileAsync', () => { it('returns false when cache write is not allowed', async () => { const session: RushSession = {} as RushSession; const provider = new HttpBuildCacheProvider(EXAMPLE_OPTIONS, session); // write not allowed - const entryStream = new Readable({ read() {} }); - const result = await provider.trySetCacheEntryStreamAsync(terminal, 'some-key', entryStream); + const result = await provider.trySetCacheEntryFromFileAsync(terminal, 'some-key', '/tmp/cache-entry'); expect(result).toBe(false); expect(streamFetchFn).not.toHaveBeenCalled(); }); - it('uploads a stream successfully', async () => { + it('uploads from file successfully', async () => { jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue('token123'); const session: RushSession = {} as RushSession; const provider = new HttpBuildCacheProvider(WRITE_ALLOWED_OPTIONS, session); - const entryStream = new Readable({ read() {} }); const responseStream = new Readable({ read() {} }); mocked(streamFetchFn).mockResolvedValue({ @@ -351,7 +360,7 @@ Array [ stream: responseStream }); - const result = await provider.trySetCacheEntryStreamAsync(terminal, 'some-key', entryStream); + const result = await provider.trySetCacheEntryFromFileAsync(terminal, 'some-key', '/tmp/cache-entry'); expect(result).toBe(true); expect(streamFetchFn).toHaveBeenCalledTimes(1); @@ -368,7 +377,6 @@ Array [ const session: RushSession = {} as RushSession; const provider = new HttpBuildCacheProvider(WRITE_ALLOWED_OPTIONS, session); - const entryStream = new Readable({ read() {} }); const responseStream = new Readable({ read() {} }); mocked(streamFetchFn).mockResolvedValue({ @@ -378,10 +386,10 @@ Array [ stream: responseStream }); - const result = await provider.trySetCacheEntryStreamAsync(terminal, 'some-key', entryStream); + const result = await provider.trySetCacheEntryFromFileAsync(terminal, 'some-key', '/tmp/cache-entry'); expect(result).toBe(false); - // maxAttempts is 1 for stream uploads, so only 1 call + // maxAttempts is 1 for file-based uploads, so only 1 call expect(streamFetchFn).toHaveBeenCalledTimes(1); }); @@ -402,7 +410,6 @@ Array [ const session: RushSession = {} as RushSession; const provider = new HttpBuildCacheProvider(WRITE_ALLOWED_OPTIONS, session); - const entryStream = new Readable({ read() {} }); const responseStream = new Readable({ read() {} }); mocked(streamFetchFn).mockResolvedValue({ @@ -414,7 +421,7 @@ Array [ // Even though credentials are optional and we got a 4xx, the stream body // should prevent the credential fallback retry since the stream is consumed - const result = await provider.trySetCacheEntryStreamAsync(terminal, 'some-key', entryStream); + const result = await provider.trySetCacheEntryFromFileAsync(terminal, 'some-key', '/tmp/cache-entry'); expect(result).toBe(false); // Should only be called once (no credential fallback retry with consumed stream) From b474fdaaabc4fc17f419706bcf2b4f6ee052e73f Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Mon, 6 Apr 2026 23:25:18 -0700 Subject: [PATCH 35/45] Rename file-based cache APIs to tryDownload/tryUpload for clarity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - tryGetCacheEntryToFileAsync → tryDownloadCacheEntryToFileAsync - trySetCacheEntryFromFileAsync → tryUploadCacheEntryFromFileAsync Co-Authored-By: Claude Opus 4.6 (1M context) --- common/reviews/api/rush-lib.api.md | 4 +- .../buildCache/ICloudBuildCacheProvider.ts | 8 +++- .../logic/buildCache/OperationBuildCache.ts | 8 ++-- .../src/AmazonS3BuildCacheProvider.ts | 4 +- .../src/AzureStorageBuildCacheProvider.ts | 4 +- .../src/HttpBuildCacheProvider.ts | 4 +- .../src/test/HttpBuildCacheProvider.test.ts | 46 +++++++++++++++---- 7 files changed, 55 insertions(+), 23 deletions(-) diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index c0cc3cff49b..9532ccf52e6 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -345,12 +345,12 @@ export interface ICloudBuildCacheProvider { deleteCachedCredentialsAsync(terminal: ITerminal): Promise; // (undocumented) readonly isCacheWriteAllowed: boolean; + tryDownloadCacheEntryToFileAsync?(terminal: ITerminal, cacheId: string, localFilePath: string): Promise; // (undocumented) tryGetCacheEntryBufferByIdAsync(terminal: ITerminal, cacheId: string): Promise; - tryGetCacheEntryToFileAsync?(terminal: ITerminal, cacheId: string, localFilePath: string): Promise; // (undocumented) trySetCacheEntryBufferAsync(terminal: ITerminal, cacheId: string, entryBuffer: Buffer): Promise; - trySetCacheEntryFromFileAsync?(terminal: ITerminal, cacheId: string, localFilePath: string): Promise; + tryUploadCacheEntryFromFileAsync?(terminal: ITerminal, cacheId: string, localFilePath: string): Promise; // (undocumented) updateCachedCredentialAsync(terminal: ITerminal, credential: string): Promise; // (undocumented) diff --git a/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts b/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts index 7499a7be25c..a10e1019282 100644 --- a/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts +++ b/libraries/rush-lib/src/logic/buildCache/ICloudBuildCacheProvider.ts @@ -21,7 +21,11 @@ export interface ICloudBuildCacheProvider { * @returns `true` if the cache entry was found and written to the file, `false` if it was * not found. Throws on errors. */ - tryGetCacheEntryToFileAsync?(terminal: ITerminal, cacheId: string, localFilePath: string): Promise; + tryDownloadCacheEntryToFileAsync?( + terminal: ITerminal, + cacheId: string, + localFilePath: string + ): Promise; /** * If implemented, the build cache will prefer to use this method over * {@link ICloudBuildCacheProvider.trySetCacheEntryBufferAsync} to avoid loading the entire @@ -30,7 +34,7 @@ export interface ICloudBuildCacheProvider { * * @returns `true` if the cache entry was written to the cache, otherwise `false`. */ - trySetCacheEntryFromFileAsync?( + tryUploadCacheEntryFromFileAsync?( terminal: ITerminal, cacheId: string, localFilePath: string diff --git a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts index 9c1cd27f985..0748d63d83d 100644 --- a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts +++ b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts @@ -175,13 +175,13 @@ export class OperationBuildCache { if ( this._useDirectFileTransfersForBuildCache && - this._cloudBuildCacheProvider.tryGetCacheEntryToFileAsync + this._cloudBuildCacheProvider.tryDownloadCacheEntryToFileAsync ) { // Use file-based path to avoid loading the entire cache entry into memory. // The provider downloads directly to the local cache file. const targetPath: string = this._localBuildCacheProvider.getCacheEntryPath(cacheId); try { - cloudCacheHit = await this._cloudBuildCacheProvider.tryGetCacheEntryToFileAsync( + cloudCacheHit = await this._cloudBuildCacheProvider.tryDownloadCacheEntryToFileAsync( terminal, cacheId, targetPath @@ -350,11 +350,11 @@ export class OperationBuildCache { if ( this._useDirectFileTransfersForBuildCache && - this._cloudBuildCacheProvider.trySetCacheEntryFromFileAsync + this._cloudBuildCacheProvider.tryUploadCacheEntryFromFileAsync ) { // Use file-based upload to avoid loading the entire cache entry into memory. // The provider reads from the local cache file directly. - setCloudCacheEntryPromise = this._cloudBuildCacheProvider.trySetCacheEntryFromFileAsync( + setCloudCacheEntryPromise = this._cloudBuildCacheProvider.tryUploadCacheEntryFromFileAsync( terminal, cacheId, localCacheEntryPath diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts index 6d7a129408c..1410294476b 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3BuildCacheProvider.ts @@ -179,7 +179,7 @@ export class AmazonS3BuildCacheProvider implements ICloudBuildCacheProvider { } } - public async tryGetCacheEntryToFileAsync( + public async tryDownloadCacheEntryToFileAsync( terminal: ITerminal, cacheId: string, localFilePath: string @@ -193,7 +193,7 @@ export class AmazonS3BuildCacheProvider implements ICloudBuildCacheProvider { } } - public async trySetCacheEntryFromFileAsync( + public async tryUploadCacheEntryFromFileAsync( terminal: ITerminal, cacheId: string, localFilePath: string diff --git a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts index 550c032cd6f..c04542ff611 100644 --- a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts +++ b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts @@ -90,7 +90,7 @@ export class AzureStorageBuildCacheProvider }); } - public async tryGetCacheEntryToFileAsync( + public async tryDownloadCacheEntryToFileAsync( terminal: ITerminal, cacheId: string, localFilePath: string @@ -107,7 +107,7 @@ export class AzureStorageBuildCacheProvider return result ?? false; } - public async trySetCacheEntryFromFileAsync( + public async tryUploadCacheEntryFromFileAsync( terminal: ITerminal, cacheId: string, localFilePath: string diff --git a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts index d6f1d50518e..ff06e0d18e4 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts @@ -162,7 +162,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { } } - public async tryGetCacheEntryToFileAsync( + public async tryDownloadCacheEntryToFileAsync( terminal: ITerminal, cacheId: string, localFilePath: string @@ -192,7 +192,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { } } - public async trySetCacheEntryFromFileAsync( + public async tryUploadCacheEntryFromFileAsync( terminal: ITerminal, cacheId: string, localFilePath: string diff --git a/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts b/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts index 1bef9056e83..ba40d11451c 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts @@ -252,7 +252,7 @@ Array [ // ── File-based GET ────────────────────────────────────────────────────── - describe('tryGetCacheEntryToFileAsync', () => { + describe('tryDownloadCacheEntryToFileAsync', () => { it('downloads to file on a successful response', async () => { jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue('token123'); @@ -269,7 +269,11 @@ Array [ stream: mockStream }); - const result = await provider.tryGetCacheEntryToFileAsync(terminal, 'some-key', '/tmp/cache-entry'); + const result = await provider.tryDownloadCacheEntryToFileAsync( + terminal, + 'some-key', + '/tmp/cache-entry' + ); expect(result).toBe(true); expect(streamFetchFn).toHaveBeenCalledTimes(1); expect(streamFetchFn).toHaveBeenCalledWith( @@ -295,7 +299,11 @@ Array [ stream: mockStream }); - const result = await provider.tryGetCacheEntryToFileAsync(terminal, 'some-key', '/tmp/cache-entry'); + const result = await provider.tryDownloadCacheEntryToFileAsync( + terminal, + 'some-key', + '/tmp/cache-entry' + ); expect(result).toBe(false); }); @@ -325,7 +333,11 @@ Array [ stream: createMockStream() }); - const result = await provider.tryGetCacheEntryToFileAsync(terminal, 'some-key', '/tmp/cache-entry'); + const result = await provider.tryDownloadCacheEntryToFileAsync( + terminal, + 'some-key', + '/tmp/cache-entry' + ); expect(result).toBe(false); expect(streamFetchFn).toHaveBeenCalledTimes(3); }); @@ -333,12 +345,16 @@ Array [ // ── File-based SET ────────────────────────────────────────────────────── - describe('trySetCacheEntryFromFileAsync', () => { + describe('tryUploadCacheEntryFromFileAsync', () => { it('returns false when cache write is not allowed', async () => { const session: RushSession = {} as RushSession; const provider = new HttpBuildCacheProvider(EXAMPLE_OPTIONS, session); // write not allowed - const result = await provider.trySetCacheEntryFromFileAsync(terminal, 'some-key', '/tmp/cache-entry'); + const result = await provider.tryUploadCacheEntryFromFileAsync( + terminal, + 'some-key', + '/tmp/cache-entry' + ); expect(result).toBe(false); expect(streamFetchFn).not.toHaveBeenCalled(); @@ -360,7 +376,11 @@ Array [ stream: responseStream }); - const result = await provider.trySetCacheEntryFromFileAsync(terminal, 'some-key', '/tmp/cache-entry'); + const result = await provider.tryUploadCacheEntryFromFileAsync( + terminal, + 'some-key', + '/tmp/cache-entry' + ); expect(result).toBe(true); expect(streamFetchFn).toHaveBeenCalledTimes(1); @@ -386,7 +406,11 @@ Array [ stream: responseStream }); - const result = await provider.trySetCacheEntryFromFileAsync(terminal, 'some-key', '/tmp/cache-entry'); + const result = await provider.tryUploadCacheEntryFromFileAsync( + terminal, + 'some-key', + '/tmp/cache-entry' + ); expect(result).toBe(false); // maxAttempts is 1 for file-based uploads, so only 1 call @@ -421,7 +445,11 @@ Array [ // Even though credentials are optional and we got a 4xx, the stream body // should prevent the credential fallback retry since the stream is consumed - const result = await provider.trySetCacheEntryFromFileAsync(terminal, 'some-key', '/tmp/cache-entry'); + const result = await provider.tryUploadCacheEntryFromFileAsync( + terminal, + 'some-key', + '/tmp/cache-entry' + ); expect(result).toBe(false); // Should only be called once (no credential fallback retry with consumed stream) From e8007857b85aa62636cc8d2295989914a8157d0d Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Mon, 6 Apr 2026 23:50:08 -0700 Subject: [PATCH 36/45] Replace jest.mock('node:fs') with FileSystem spies in cache tests Mock FileSystem.createReadStream, createWriteStreamAsync, and ensureFolderAsync via jest.spyOn instead of module-level fs mocks. This is more targeted, less brittle, and consistent with the production code's use of the FileSystem abstraction. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/test/AmazonS3Client.test.ts | 26 ++++++++----------- .../src/test/HttpBuildCacheProvider.test.ts | 11 ++++---- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts index bfcf6e254f5..ba73593610a 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts @@ -9,21 +9,6 @@ jest.mock('node:stream/promises', () => ({ pipeline: jest.fn().mockResolvedValue(undefined) })); -jest.mock('node:fs', () => { - const { Readable: ReadableImpl } = jest.requireActual('node:stream'); - return { - createReadStream: jest.fn().mockImplementation(() => { - // Return a Readable that emits empty data then ends, so _hashFileAsync completes - return new ReadableImpl({ - read() { - this.push(null); - } - }); - }), - createWriteStream: jest.fn().mockReturnValue({}) - }; -}); - import { Readable } from 'node:stream'; import { ConsoleTerminalProvider, Terminal } from '@rushstack/terminal'; @@ -672,6 +657,17 @@ describe(AmazonS3Client.name, () => { }).bind(global) as typeof global.setTimeout; jest.spyOn(FileSystem, 'ensureFolderAsync').mockResolvedValue(); + jest + .spyOn(FileSystem, 'createWriteStreamAsync') + .mockResolvedValue({} as unknown as Awaited>); + // Return a Readable that immediately ends, so _hashFileAsync completes with the null hash + jest.spyOn(FileSystem, 'createReadStream').mockReturnValue( + new Readable({ + read() { + this.push(null); + } + }) as unknown as ReturnType + ); }); afterEach(() => { diff --git a/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts b/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts index ba40d11451c..372b7aac0fd 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts @@ -9,11 +9,6 @@ jest.mock('node:stream/promises', () => ({ pipeline: jest.fn().mockResolvedValue(undefined) })); -jest.mock('node:fs', () => ({ - createReadStream: jest.fn().mockReturnValue({ pipe: jest.fn() }), - createWriteStream: jest.fn().mockReturnValue({}) -})); - import { Readable } from 'node:stream'; import { type RushSession, EnvironmentConfiguration } from '@rushstack/rush-sdk'; @@ -58,6 +53,12 @@ describe('HttpBuildCacheProvider', () => { streamFetchFn = jest.fn(); WebClient.mockRequestFn(fetchFn as unknown as FetchFnType); WebClient.mockStreamRequestFn(streamFetchFn as unknown as StreamFetchFnType); + jest + .spyOn(FileSystem, 'createReadStream') + .mockReturnValue({ pipe: jest.fn() } as unknown as ReturnType); + jest + .spyOn(FileSystem, 'createWriteStreamAsync') + .mockResolvedValue({} as unknown as Awaited>); jest.spyOn(FileSystem, 'ensureFolderAsync').mockResolvedValue(); }); From caf3fecaefedfd3b82c60ae4623df27312cea51e Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Tue, 7 Apr 2026 00:54:08 -0700 Subject: [PATCH 37/45] Clean up partial file on failed cache download If tryDownloadCacheEntryToFileAsync throws mid-download, a corrupt partial file could be left at the target path. On the next build, tryGetCacheEntryPathByIdAsync would find it and try to untar it. Delete the file in the catch block to prevent this. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/logic/buildCache/OperationBuildCache.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts index 0748d63d83d..a4070c52aed 100644 --- a/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts +++ b/libraries/rush-lib/src/logic/buildCache/OperationBuildCache.ts @@ -191,7 +191,15 @@ export class OperationBuildCache { updateLocalCacheSuccess = true; } } catch (e) { - terminal.writeVerboseLine(`Failed to update local cache: ${e}`); + terminal.writeVerboseLine(`Failed to download cache entry to local cache: ${e}`); + // Clean up any partial file left by the failed download so it isn't + // mistaken for a valid cache entry on the next build. + try { + await FileSystem.deleteFileAsync(targetPath); + } catch { + // Ignore cleanup errors (file may not have been created) + } + updateLocalCacheSuccess = false; } } else { From e8949807725e5d2857bec1e36e638a82ecc66a27 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Tue, 7 Apr 2026 00:57:13 -0700 Subject: [PATCH 38/45] Ensure parent directory exists before Azure downloadToFile The HTTP and S3 providers both ensure the parent directory exists before writing the cache entry file. The Azure provider was missing this, which would cause failures on a fresh machine where the build cache folder hasn't been created yet. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/AzureStorageBuildCacheProvider.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts index c04542ff611..1f4c3c2fe38 100644 --- a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts +++ b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. +import * as path from 'node:path'; + import { type BlobClient, BlobServiceClient, @@ -9,6 +11,7 @@ import { } from '@azure/storage-blob'; import { AzureAuthorityHosts } from '@azure/identity'; +import { FileSystem } from '@rushstack/node-core-library'; import type { ITerminal } from '@rushstack/terminal'; import { type ICloudBuildCacheProvider, @@ -99,6 +102,7 @@ export class AzureStorageBuildCacheProvider terminal, cacheId, async (blobClient: BlobClient) => { + await FileSystem.ensureFolderAsync(path.dirname(localFilePath)); await blobClient.downloadToFile(localFilePath); return true; } From cebdcee04d43c2e418c8be1d0928e5fd080da591 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Tue, 7 Apr 2026 00:57:47 -0700 Subject: [PATCH 39/45] fixup! Ensure parent directory exists before Azure downloadToFile --- .../src/AzureStorageBuildCacheProvider.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts index 1f4c3c2fe38..94b9e7ccf40 100644 --- a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts +++ b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts @@ -102,6 +102,7 @@ export class AzureStorageBuildCacheProvider terminal, cacheId, async (blobClient: BlobClient) => { + // TODO: Determine if this is necessary, or if the Azure Storage SDK handles this internally. await FileSystem.ensureFolderAsync(path.dirname(localFilePath)); await blobClient.downloadToFile(localFilePath); return true; From 53f378eed68f12473c62e50dd0d14e0f8ae376f9 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Tue, 7 Apr 2026 00:59:49 -0700 Subject: [PATCH 40/45] Update change file descriptions, Azure JSDoc, and test names - Rush change file: replace "streaming APIs" with file-based transfer API names and mention the experiment flag - node-core-library change file: fix broken backtick formatting - Azure provider: update JSDoc from "stream" to "file-based" - HTTP test names: replace "stream consumed" / "stream bodies" with file-based language Co-Authored-By: Claude Opus 4.6 (1M context) --- ...t-stream-cache-entry-for-http-plugin_2026-04-05-03-56.json | 2 +- .../src/AzureStorageBuildCacheProvider.ts | 4 ++-- .../src/test/HttpBuildCacheProvider.test.ts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/common/changes/@microsoft/rush/copilot-stream-cache-entry-for-http-plugin_2026-04-05-03-56.json b/common/changes/@microsoft/rush/copilot-stream-cache-entry-for-http-plugin_2026-04-05-03-56.json index 7942e4c0699..fa3539c5131 100644 --- a/common/changes/@microsoft/rush/copilot-stream-cache-entry-for-http-plugin_2026-04-05-03-56.json +++ b/common/changes/@microsoft/rush/copilot-stream-cache-entry-for-http-plugin_2026-04-05-03-56.json @@ -1,7 +1,7 @@ { "changes": [ { - "comment": "Add optional streaming APIs to `ICloudBuildCacheProvider` and `FileSystemBuildCacheProvider`, allowing cache plugins to stream entries to and from the cloud cache without buffering entire contents in memory. Implement streaming in `@rushstack/rush-http-build-cache-plugin`, `@rushstack/rush-amazon-s3-build-cache-plugin`, and `@rushstack/rush-azure-storage-build-cache-plugin`.", + "comment": "Add optional file-based transfer APIs (`tryDownloadCacheEntryToFileAsync`, `tryUploadCacheEntryFromFileAsync`) to `ICloudBuildCacheProvider`, allowing cache plugins to transfer cache entries directly to and from files on disk without buffering entire contents in memory. Implement in `@rushstack/rush-http-build-cache-plugin`, `@rushstack/rush-amazon-s3-build-cache-plugin`, and `@rushstack/rush-azure-storage-build-cache-plugin`. Gated behind the `useDirectFileTransfersForBuildCache` experiment.", "type": "none", "packageName": "@microsoft/rush" } diff --git a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts index 94b9e7ccf40..00d08c6f153 100644 --- a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts +++ b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts @@ -123,7 +123,7 @@ export class AzureStorageBuildCacheProvider } /** - * Shared logic for both buffer and stream GET operations. + * Shared logic for both buffer-based and file-based GET operations. * Checks if the blob exists, retrieves data via the provided callback, and handles errors. */ private async _tryGetBlobDataAsync( @@ -147,7 +147,7 @@ export class AzureStorageBuildCacheProvider } /** - * Shared logic for both buffer and stream SET operations. + * Shared logic for both buffer-based and file-based SET operations. * Checks write permission, whether the blob already exists, uploads via the provided callback, * and handles 409 conflict errors. */ diff --git a/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts b/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts index 372b7aac0fd..5b155437a08 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts @@ -393,7 +393,7 @@ Array [ ); }); - it('does not retry on failure (stream consumed)', async () => { + it('does not retry on failure (file stream already consumed)', async () => { jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue('token123'); const session: RushSession = {} as RushSession; @@ -418,7 +418,7 @@ Array [ expect(streamFetchFn).toHaveBeenCalledTimes(1); }); - it('skips credential fallback for stream bodies on 4xx', async () => { + it('skips credential fallback for file-based uploads on 4xx', async () => { // No credential in env for the first attempt jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue(undefined); // But credentials ARE available in the credential cache — without the stream-body From d81d8567c4ec3215c8c4dbf3fcb7bc04e47db7ca Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Tue, 7 Apr 2026 01:00:34 -0700 Subject: [PATCH 41/45] Fix S3 retry delay log unit from seconds to milliseconds The delay variable is in milliseconds but the log message said "s". This produced misleading output like "Will retry request in 4000s...". Co-Authored-By: Claude Opus 4.6 (1M context) --- .../rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts index ac4b7bcb6aa..89fefa89cad 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/AmazonS3Client.ts @@ -600,7 +600,7 @@ export class AmazonS3Client { } delay = Math.min(maxRetryDelayInMs, delay); - log(`Will retry request in ${delay}s...`); + log(`Will retry request in ${delay}ms...`); await Async.sleepAsync(delay); const retryResponse: RetryableRequestResponse = await sendRequest(); From 3a322ed9d12327f1577db19cfc6a4c78edcf8581 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Tue, 7 Apr 2026 01:01:22 -0700 Subject: [PATCH 42/45] fixup! Rework streaming cache APIs to file-based APIs with S3 payload signing Remove unused onBlobAlreadyExists parameter from _trySetBlobDataAsync This callback was previously used to drain incoming streams when the blob already existed. With the switch to file-based APIs, no callers pass this parameter anymore. --- .../src/AzureStorageBuildCacheProvider.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts index 00d08c6f153..cfc91b25ffa 100644 --- a/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts +++ b/rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts @@ -154,8 +154,7 @@ export class AzureStorageBuildCacheProvider private async _trySetBlobDataAsync( terminal: ITerminal, cacheId: string, - uploadAsync: (blockBlobClient: BlockBlobClient) => Promise, - onBlobAlreadyExists?: () => void + uploadAsync: (blockBlobClient: BlockBlobClient) => Promise ): Promise { if (!this.isCacheWriteAllowed) { terminal.writeErrorLine( @@ -188,7 +187,6 @@ export class AzureStorageBuildCacheProvider if (blobAlreadyExists) { terminal.writeVerboseLine('Build cache entry blob already exists.'); - onBlobAlreadyExists?.(); return true; } else { try { From 7c8e7c3a75ac016672ee0570765325c8bad4139b Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Tue, 7 Apr 2026 01:06:17 -0700 Subject: [PATCH 43/45] Add missing test coverage for file-based cache APIs - HTTP: add 404 cache miss test for tryDownloadCacheEntryToFileAsync - HTTP: add pipeline assertion in download success test - S3: add retry test for downloadObjectToFileAsync on transient 5xx - S3: add pipeline assertions in download success/miss tests Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/test/AmazonS3Client.test.ts | 48 +++++++++++++++++++ .../src/test/HttpBuildCacheProvider.test.ts | 25 ++++++++++ 2 files changed, 73 insertions(+) diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts index ba73593610a..43e0739cf5a 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts @@ -10,6 +10,7 @@ jest.mock('node:stream/promises', () => ({ })); import { Readable } from 'node:stream'; +import { pipeline } from 'node:stream/promises'; import { ConsoleTerminalProvider, Terminal } from '@rushstack/terminal'; import { FileSystem } from '@rushstack/node-core-library'; @@ -716,6 +717,7 @@ describe(AmazonS3Client.name, () => { expect(result).toBe(true); expect(spy).toHaveBeenCalledTimes(1); expect(spy.mock.calls[0]).toMatchSnapshot(); + expect(pipeline).toHaveBeenCalled(); spy.mockRestore(); }); @@ -733,6 +735,52 @@ describe(AmazonS3Client.name, () => { ); expect(result).toBe(false); expect(spy).toHaveBeenCalledTimes(1); + expect(pipeline).not.toHaveBeenCalled(); + spy.mockRestore(); + }); + + it('Retries on transient server errors', async () => { + let callCount: number = 0; + const spy: jest.SpyInstance = jest + .spyOn(WebClient.prototype, 'fetchStreamAsync') + .mockImplementation(async () => { + callCount++; + const mockStream = new Readable({ read() {} }); + if (callCount < 3) { + return { + stream: mockStream, + headers: {}, + status: 500, + statusText: 'InternalServerError', + ok: false, + redirected: false + }; + } + return { + stream: mockStream, + headers: {}, + status: 200, + statusText: 'OK', + ok: true, + redirected: false + }; + }); + + const s3Client: AmazonS3Client = new AmazonS3Client( + { + accessKeyId: 'accessKeyId', + secretAccessKey: 'secretAccessKey', + sessionToken: undefined + }, + DUMMY_OPTIONS, + webClient, + terminal + ); + + const result = await s3Client.downloadObjectToFileAsync('abc123', '/tmp/cache-entry'); + expect(result).toBe(true); + // First two attempts fail with 500, third succeeds + expect(spy).toHaveBeenCalledTimes(3); spy.mockRestore(); }); }); diff --git a/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts b/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts index 5b155437a08..b8608bb875e 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts @@ -10,6 +10,7 @@ jest.mock('node:stream/promises', () => ({ })); import { Readable } from 'node:stream'; +import { pipeline } from 'node:stream/promises'; import { type RushSession, EnvironmentConfiguration } from '@rushstack/rush-sdk'; import { type ICredentialCacheEntry, CredentialCache } from '@rushstack/credential-cache'; @@ -284,6 +285,30 @@ Array [ redirect: 'follow' }) ); + expect(pipeline).toHaveBeenCalledWith(mockStream, expect.anything()); + }); + + it('returns false on 404 cache miss', async () => { + jest.spyOn(EnvironmentConfiguration, 'buildCacheCredential', 'get').mockReturnValue('token123'); + + const session: RushSession = {} as RushSession; + const provider = new HttpBuildCacheProvider(EXAMPLE_OPTIONS, session); + const mockStream = new Readable({ read() {} }); + + mocked(streamFetchFn).mockResolvedValue({ + status: 404, + statusText: 'Not Found', + ok: false, + stream: mockStream + }); + + const result = await provider.tryDownloadCacheEntryToFileAsync( + terminal, + 'some-key', + '/tmp/cache-entry' + ); + expect(result).toBe(false); + expect(pipeline).not.toHaveBeenCalled(); }); it('returns false on credential failure', async () => { From 5abf492a3ded9fec7dd8422d98dcd45c1e2053b0 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Tue, 7 Apr 2026 01:07:26 -0700 Subject: [PATCH 44/45] Fix "unknown bytes" debug log wording in HttpBuildCacheProvider For stream-body requests, the log read "unknown bytes" which is awkward. Change to "unknown length" for clarity. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/HttpBuildCacheProvider.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts index ff06e0d18e4..511a3b586fa 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts @@ -399,9 +399,9 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { } } - const bodyLength: number | string = Buffer.isBuffer(body) ? body.length : 'unknown'; + const bodyLengthDesc: string = Buffer.isBuffer(body) ? `${body.length} bytes` : 'unknown length'; - terminal.writeDebugLine(`[http-build-cache] request: ${method} ${url} ${bodyLength} bytes`); + terminal.writeDebugLine(`[http-build-cache] request: ${method} ${url} ${bodyLengthDesc}`); const fetchOptions: IGetFetchOptions | IFetchOptionsWithBody = { verb: method, From f8fecd4ef639d99e712b161ef0bde3d598677405 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 8 Apr 2026 15:23:45 -0700 Subject: [PATCH 45/45] Address dmichon-msft review comments - Add clarifying comment on maxAttempts: 1 for uploads explaining why the parameter exists (shared between download with retries and upload without) - Replace S3 download snapshot containing auth headers with explicit field assertions, avoiding credential-looking strings in snapshots - Update inline snapshot for "unknown length" wording change Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/test/AmazonS3Client.test.ts | 8 +++++++- .../test/__snapshots__/AmazonS3Client.test.ts.snap | 14 -------------- .../src/HttpBuildCacheProvider.ts | 4 +++- .../src/test/HttpBuildCacheProvider.test.ts | 8 ++++---- 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts index 43e0739cf5a..095d3284846 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/AmazonS3Client.test.ts @@ -716,7 +716,13 @@ describe(AmazonS3Client.name, () => { ); expect(result).toBe(true); expect(spy).toHaveBeenCalledTimes(1); - expect(spy.mock.calls[0]).toMatchSnapshot(); + const [url, options] = spy.mock.calls[0]; + expect(url).toBe('http://localhost:9000/abc123'); + expect(options.verb).toBe('GET'); + expect(options.headers['x-amz-content-sha256']).toMatch(/^[0-9a-f]{64}$/); + expect(options.headers['x-amz-date']).toBe('20200418T123242Z'); + // eslint-disable-next-line dot-notation + expect(options.headers['Authorization']).toContain('AWS4-HMAC-SHA256'); expect(pipeline).toHaveBeenCalled(); spy.mockRestore(); }); diff --git a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/__snapshots__/AmazonS3Client.test.ts.snap b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/__snapshots__/AmazonS3Client.test.ts.snap index d6297a81131..de18d3fbc4b 100644 --- a/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/__snapshots__/AmazonS3Client.test.ts.snap +++ b/rush-plugins/rush-amazon-s3-build-cache-plugin/src/test/__snapshots__/AmazonS3Client.test.ts.snap @@ -549,17 +549,3 @@ exports[`AmazonS3Client Rejects invalid S3 endpoint values 10`] = `"Invalid S3 e exports[`AmazonS3Client Rejects invalid S3 endpoint values 11`] = `"Invalid S3 endpoint. Some part of the hostname contains invalid characters or is too long"`; -exports[`AmazonS3Client File-based requests Downloading an object to file Can download an object to file 1`] = ` -Array [ - "http://localhost:9000/abc123", - Object { - "headers": Object { - "Authorization": "AWS4-HMAC-SHA256 Credential=accessKeyId/20200418/us-east-1/s3/aws4_request,SignedHeaders=host;x-amz-content-sha256;x-amz-date,Signature=194608e9e7ba6d8aa4a019b3b6fd237e6b09ef1f45ff7fa60cbb81c1875538be", - "x-amz-content-sha256": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", - "x-amz-date": "20200418T123242Z", - }, - "verb": "GET", - }, -] -`; - diff --git a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts index 511a3b586fa..9c7ba8a081c 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts @@ -209,7 +209,9 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { method: this._uploadMethod, body: entryStream, warningText: 'Could not write cache entry', - // Streaming uploads cannot be retried because the stream is consumed + // maxAttempts is 1 because the file read stream is consumed after the first attempt + // and cannot be replayed. Downloads use MAX_HTTP_CACHE_ATTEMPTS since each retry + // issues a fresh GET with no request body. maxAttempts: 1 }); diff --git a/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts b/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts index b8608bb875e..49a04580f8a 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/test/HttpBuildCacheProvider.test.ts @@ -97,7 +97,7 @@ describe('HttpBuildCacheProvider', () => { ); expect(terminalBuffer.getAllOutputAsChunks({ asLines: true })).toMatchInlineSnapshot(` Array [ - "[ debug] [http-build-cache] request: GET https://buildcache.example.acme.com/some-key unknown bytes[n]", + "[ debug] [http-build-cache] request: GET https://buildcache.example.acme.com/some-key unknown length[n]", "[warning] Error getting cache entry: Error: Credentials for https://buildcache.example.acme.com/ have not been provided.[n]", "[warning] In CI, verify that RUSH_BUILD_CACHE_CREDENTIAL contains a valid Authorization header value.[n]", "[warning] [n]", @@ -160,9 +160,9 @@ Array [ ); expect(terminalBuffer.getAllOutputAsChunks({ asLines: true })).toMatchInlineSnapshot(` Array [ - "[ debug] [http-build-cache] request: GET https://buildcache.example.acme.com/some-key unknown bytes[n]", - "[ debug] [http-build-cache] request: GET https://buildcache.example.acme.com/some-key unknown bytes[n]", - "[ debug] [http-build-cache] request: GET https://buildcache.example.acme.com/some-key unknown bytes[n]", + "[ debug] [http-build-cache] request: GET https://buildcache.example.acme.com/some-key unknown length[n]", + "[ debug] [http-build-cache] request: GET https://buildcache.example.acme.com/some-key unknown length[n]", + "[ debug] [http-build-cache] request: GET https://buildcache.example.acme.com/some-key unknown length[n]", "[warning] Could not get cache entry: HTTP 504: Gateway Timeout[n]", ] `);