From 8916b4974e9eec66af313afbe5a4c75bcc72cd35 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Wed, 4 Dec 2024 11:15:17 -0800 Subject: [PATCH] [rush] Update rush-http-build-cache-plugin to use WebClient instead of node-fetch (#5027) * Update rush-http-build-cache-plugin to use WebClient instead of node-fetch. * fixup! Update rush-http-build-cache-plugin to use WebClient instead of node-fetch. * Rush update. --- ...fetch-http-from-http_2024-12-04-02-57.json | 10 ++ .../config/subspaces/default/pnpm-lock.yaml | 6 - .../rush-http-build-cache-plugin/package.json | 6 +- .../src/HttpBuildCacheProvider.ts | 31 +++-- .../src/RushHttpBuildCachePlugin.ts | 4 +- .../rush-http-build-cache-plugin/src/index.ts | 2 +- .../src/test/HttpBuildCacheProvider.test.ts | 113 ++++++++++-------- 7 files changed, 99 insertions(+), 73 deletions(-) create mode 100644 common/changes/@microsoft/rush/remove-node-fetch-http-from-http_2024-12-04-02-57.json diff --git a/common/changes/@microsoft/rush/remove-node-fetch-http-from-http_2024-12-04-02-57.json b/common/changes/@microsoft/rush/remove-node-fetch-http-from-http_2024-12-04-02-57.json new file mode 100644 index 00000000000..89398fbc189 --- /dev/null +++ b/common/changes/@microsoft/rush/remove-node-fetch-http-from-http_2024-12-04-02-57.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Remove the `node-fetch` dependency from `@rushstack/rush-http-build-cache-plugin`.", + "type": "none" + } + ], + "packageName": "@microsoft/rush" +} \ No newline at end of file diff --git a/common/config/subspaces/default/pnpm-lock.yaml b/common/config/subspaces/default/pnpm-lock.yaml index 2ea25f66e04..857f4d0da73 100644 --- a/common/config/subspaces/default/pnpm-lock.yaml +++ b/common/config/subspaces/default/pnpm-lock.yaml @@ -4051,9 +4051,6 @@ importers: https-proxy-agent: specifier: ~5.0.0 version: 5.0.1 - node-fetch: - specifier: 2.6.7 - version: 2.6.7 devDependencies: '@microsoft/rush-lib': specifier: workspace:* @@ -4064,9 +4061,6 @@ importers: '@rushstack/terminal': specifier: workspace:* version: link:../../libraries/terminal - '@types/node-fetch': - specifier: 2.6.2 - version: 2.6.2 local-node-rig: specifier: workspace:* version: link:../../rigs/local-node-rig diff --git a/rush-plugins/rush-http-build-cache-plugin/package.json b/rush-plugins/rush-http-build-cache-plugin/package.json index a4695921fca..983c31a5960 100644 --- a/rush-plugins/rush-http-build-cache-plugin/package.json +++ b/rush-plugins/rush-http-build-cache-plugin/package.json @@ -21,14 +21,12 @@ "dependencies": { "@rushstack/node-core-library": "workspace:*", "@rushstack/rush-sdk": "workspace:*", - "https-proxy-agent": "~5.0.0", - "node-fetch": "2.6.7" + "https-proxy-agent": "~5.0.0" }, "devDependencies": { "@microsoft/rush-lib": "workspace:*", "@rushstack/heft": "workspace:*", "@rushstack/terminal": "workspace:*", - "local-node-rig": "workspace:*", - "@types/node-fetch": "2.6.2" + "local-node-rig": "workspace:*" } } 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 43f39b065c4..f7ec565a154 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts @@ -10,7 +10,7 @@ import { type RushSession, EnvironmentConfiguration } from '@rushstack/rush-sdk'; -import fetch, { type BodyInit, type Response } from 'node-fetch'; +import { WebClient, type WebClientResponse } from '@rushstack/rush-sdk/lib/utilities/WebClient'; import type { SpawnSyncReturns } from 'child_process'; enum CredentialsOptions { @@ -32,13 +32,18 @@ export interface IHttpBuildCacheTokenHandler { args?: string[]; } +/** + * @public + */ +export type UploadMethod = 'PUT' | 'POST' | 'PATCH'; + /** * @public */ export interface IHttpBuildCacheProviderOptions { url: string; tokenHandler?: IHttpBuildCacheTokenHandler; - uploadMethod?: string; + uploadMethod?: UploadMethod; minHttpRetryDelayMs?: number; headers?: Record; cacheKeyPrefix?: string; @@ -56,7 +61,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { private readonly _environmentCredential: string | undefined; private readonly _isCacheWriteAllowedByConfiguration: boolean; private readonly _url: URL; - private readonly _uploadMethod: string; + private readonly _uploadMethod: UploadMethod; private readonly _headers: Record; private readonly _cacheKeyPrefix: string; private readonly _tokenHandler: IHttpBuildCacheTokenHandler | undefined; @@ -210,8 +215,8 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { private async _makeHttpRequestAsync(options: { terminal: ITerminal; relUrl: string; - method: string; - body: BodyInit | undefined; + method: 'GET' | UploadMethod; + body: Buffer | undefined; warningText: string; readBody: boolean; maxAttempts: number; @@ -237,11 +242,13 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { terminal.writeDebugLine(`[http-build-cache] request: ${method} ${url} ${bodyLength} bytes`); - const response: Response = await fetch(url, { - method: method, + const webClient: WebClient = new WebClient(); + const response: WebClientResponse = await webClient.fetchAsync(url, { + verb: method, headers: headers, body: body, - redirect: 'follow' + redirect: 'follow', + timeoutMs: 0 // Use the default timeout }); if (!response.ok) { @@ -342,7 +349,11 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { } } - private _getFailureType(requestMethod: string, response: Response, isRedirect: boolean): FailureType { + private _getFailureType( + requestMethod: string, + response: WebClientResponse, + isRedirect: boolean + ): FailureType { if (response.ok) { return FailureType.None; } @@ -392,7 +403,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider { private _reportFailure( terminal: ITerminal, requestMethod: string, - response: Response, + response: WebClientResponse, isRedirect: boolean, message: string ): void { diff --git a/rush-plugins/rush-http-build-cache-plugin/src/RushHttpBuildCachePlugin.ts b/rush-plugins/rush-http-build-cache-plugin/src/RushHttpBuildCachePlugin.ts index a06a55409c8..c1577afb1f6 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/RushHttpBuildCachePlugin.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/RushHttpBuildCachePlugin.ts @@ -2,7 +2,7 @@ // See LICENSE in the project root for license information. import type { IRushPlugin, RushSession, RushConfiguration } from '@rushstack/rush-sdk'; -import type { IHttpBuildCacheProviderOptions } from './HttpBuildCacheProvider'; +import type { IHttpBuildCacheProviderOptions, UploadMethod } from './HttpBuildCacheProvider'; const PLUGIN_NAME: string = 'HttpBuildCachePlugin'; @@ -18,7 +18,7 @@ export interface IRushHttpBuildCachePluginConfig { /** * The HTTP method to use when writing to the cache (defaults to PUT). */ - uploadMethod?: string; + uploadMethod?: UploadMethod; /** * An optional set of HTTP headers to pass to the cache server. diff --git a/rush-plugins/rush-http-build-cache-plugin/src/index.ts b/rush-plugins/rush-http-build-cache-plugin/src/index.ts index dd9e088bac9..77b82280924 100644 --- a/rush-plugins/rush-http-build-cache-plugin/src/index.ts +++ b/rush-plugins/rush-http-build-cache-plugin/src/index.ts @@ -4,4 +4,4 @@ import { RushHttpBuildCachePlugin } from './RushHttpBuildCachePlugin'; export default RushHttpBuildCachePlugin; -export type { IHttpBuildCacheProviderOptions } from './HttpBuildCacheProvider'; +export type { IHttpBuildCacheProviderOptions, UploadMethod } from './HttpBuildCacheProvider'; 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 fe93acce8c4..463c238d2e6 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 @@ -1,13 +1,13 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -jest.mock('node-fetch', function () { - return Object.assign(jest.fn(), jest.requireActual('node-fetch')); +jest.mock('@rushstack/rush-sdk/lib/utilities/WebClient', () => { + return jest.requireActual('@microsoft/rush-lib/lib/utilities/WebClient'); }); -import fetch, { Response } from 'node-fetch'; import { type RushSession, EnvironmentConfiguration } from '@rushstack/rush-sdk'; import { StringBufferTerminalProvider, Terminal } from '@rushstack/terminal'; +import { WebClient } from '@rushstack/rush-sdk/lib/utilities/WebClient'; import { HttpBuildCacheProvider, type IHttpBuildCacheProviderOptions } from '../HttpBuildCacheProvider'; @@ -24,13 +24,22 @@ const EXAMPLE_OPTIONS: IHttpBuildCacheProviderOptions = { minHttpRetryDelayMs: 1 }; +type FetchFnType = Parameters[0]; + describe('HttpBuildCacheProvider', () => { let terminalBuffer: StringBufferTerminalProvider; let terminal!: Terminal; + let fetchFn: jest.Mock; beforeEach(() => { terminalBuffer = new StringBufferTerminalProvider(); terminal = new Terminal(terminalBuffer); + fetchFn = jest.fn(); + WebClient.mockRequestFn(fetchFn as unknown as FetchFnType); + }); + + afterEach(() => { + WebClient.resetMockRequestFn(); }); describe('tryGetCacheEntryBufferByIdAsync', () => { @@ -40,22 +49,23 @@ describe('HttpBuildCacheProvider', () => { const session: RushSession = {} as RushSession; const provider = new HttpBuildCacheProvider(EXAMPLE_OPTIONS, session); - mocked(fetch).mockResolvedValue( - new Response('Unauthorized', { - status: 401, - statusText: 'Unauthorized' - }) - ); + mocked(fetchFn).mockResolvedValue({ + status: 401, + statusText: 'Unauthorized', + ok: false + }); const result = await provider.tryGetCacheEntryBufferByIdAsync(terminal, 'some-key'); expect(result).toBe(undefined); - expect(fetch).toHaveBeenCalledTimes(1); - expect(fetch).toHaveBeenNthCalledWith(1, 'https://buildcache.example.acme.com/some-key', { - body: undefined, - headers: {}, - method: 'GET', - redirect: 'follow' - }); + expect(fetchFn).toHaveBeenCalledTimes(1); + expect(fetchFn).toHaveBeenNthCalledWith( + 1, + 'https://buildcache.example.acme.com/some-key', + expect.objectContaining({ + method: 'GET', + redirect: 'follow' + }) + ); expect(terminalBuffer.getWarningOutput()).toMatchInlineSnapshot( `"Error getting cache entry: Error: Credentials for https://buildcache.example.acme.com/ have not been provided.[n]In CI, verify that RUSH_BUILD_CACHE_CREDENTIAL contains a valid Authorization header value.[n][n]For local developers, run:[n][n] rush update-cloud-credentials --interactive[n][n]"` ); @@ -67,46 +77,49 @@ describe('HttpBuildCacheProvider', () => { const session: RushSession = {} as RushSession; const provider = new HttpBuildCacheProvider(EXAMPLE_OPTIONS, session); - mocked(fetch).mockResolvedValueOnce( - new Response('InternalServiceError', { - status: 500, - statusText: 'InternalServiceError' + mocked(fetchFn).mockResolvedValueOnce({ + status: 500, + statusText: 'InternalServiceError', + ok: false + }); + mocked(fetchFn).mockResolvedValueOnce({ + status: 503, + statusText: 'ServiceUnavailable', + ok: false + }); + mocked(fetchFn).mockResolvedValueOnce({ + status: 504, + statusText: 'BadGateway', + ok: false + }); + + const result = await provider.tryGetCacheEntryBufferByIdAsync(terminal, 'some-key'); + expect(result).toBe(undefined); + expect(fetchFn).toHaveBeenCalledTimes(3); + expect(fetchFn).toHaveBeenNthCalledWith( + 1, + 'https://buildcache.example.acme.com/some-key', + expect.objectContaining({ + method: 'GET', + redirect: 'follow' }) ); - mocked(fetch).mockResolvedValueOnce( - new Response('ServiceUnavailable', { - status: 503, - statusText: 'ServiceUnavailable' + expect(fetchFn).toHaveBeenNthCalledWith( + 2, + 'https://buildcache.example.acme.com/some-key', + expect.objectContaining({ + method: 'GET', + redirect: 'follow' }) ); - mocked(fetch).mockResolvedValueOnce( - new Response('BadGateway', { - status: 504, - statusText: 'BadGateway' + expect(fetchFn).toHaveBeenNthCalledWith( + 3, + 'https://buildcache.example.acme.com/some-key', + expect.objectContaining({ + method: 'GET', + redirect: 'follow' }) ); - - const result = await provider.tryGetCacheEntryBufferByIdAsync(terminal, 'some-key'); - expect(result).toBe(undefined); - expect(fetch).toHaveBeenCalledTimes(3); - expect(fetch).toHaveBeenNthCalledWith(1, 'https://buildcache.example.acme.com/some-key', { - body: undefined, - headers: {}, - method: 'GET', - redirect: 'follow' - }); - expect(fetch).toHaveBeenNthCalledWith(2, 'https://buildcache.example.acme.com/some-key', { - body: undefined, - headers: {}, - method: 'GET', - redirect: 'follow' - }); - expect(fetch).toHaveBeenNthCalledWith(3, 'https://buildcache.example.acme.com/some-key', { - body: undefined, - headers: {}, - method: 'GET', - redirect: 'follow' - }); expect(terminalBuffer.getWarningOutput()).toMatchInlineSnapshot( `"Could not get cache entry: HTTP 504: BadGateway[n]"` );