Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rush] Update rush-http-build-cache-plugin to use WebClient instead of node-fetch #5027

Merged
merged 3 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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"
}
6 changes: 0 additions & 6 deletions common/config/subspaces/default/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 2 additions & 4 deletions rush-plugins/rush-http-build-cache-plugin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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:*"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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<string, string>;
cacheKeyPrefix?: string;
Expand All @@ -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<string, string>;
private readonly _cacheKeyPrefix: string;
private readonly _tokenHandler: IHttpBuildCacheTokenHandler | undefined;
Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -392,7 +403,7 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider {
private _reportFailure(
terminal: ITerminal,
requestMethod: string,
response: Response,
response: WebClientResponse,
isRedirect: boolean,
message: string
): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion rush-plugins/rush-http-build-cache-plugin/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
import { RushHttpBuildCachePlugin } from './RushHttpBuildCachePlugin';

export default RushHttpBuildCachePlugin;
export type { IHttpBuildCacheProviderOptions } from './HttpBuildCacheProvider';
export type { IHttpBuildCacheProviderOptions, UploadMethod } from './HttpBuildCacheProvider';
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -24,13 +24,22 @@ const EXAMPLE_OPTIONS: IHttpBuildCacheProviderOptions = {
minHttpRetryDelayMs: 1
};

type FetchFnType = Parameters<typeof WebClient.mockRequestFn>[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', () => {
Expand All @@ -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]"`
);
Expand All @@ -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]"`
);
Expand Down
Loading