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

feat!: use fetch #365

Merged
merged 38 commits into from
Feb 11, 2024
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
8fa4a3c
refactor: start fetch
merceyz Jan 28, 2024
d1e38dc
feat: proxy support
merceyz Jan 28, 2024
38bdb99
chore: format
merceyz Jan 28, 2024
afc7f65
refactor: rename to getProxyAgent
merceyz Jan 28, 2024
d67d7f3
chore: add lint rule
merceyz Jan 28, 2024
25c7147
ci: use lts for nock files
merceyz Jan 28, 2024
de216b4
fix: use globalThis.fetch
merceyz Jan 28, 2024
b8d4813
feat: mocks
merceyz Jan 28, 2024
2d7a867
refactor: use fetch
merceyz Jan 28, 2024
4c7540d
build: set NODE_ENV
merceyz Jan 28, 2024
3ecb82b
chore: remove dependencies
merceyz Jan 28, 2024
8118258
chore: remove old request recording
merceyz Jan 28, 2024
ac99ee9
fix: correct nock path
merceyz Jan 28, 2024
7d88498
chore: remove nocks
merceyz Jan 28, 2024
6271b1b
test: remove tests mocking functions
merceyz Jan 28, 2024
cf4456e
chore: add new nocks
merceyz Jan 28, 2024
0b1831b
refactor: use else if
merceyz Jan 28, 2024
2fb9142
fix: move network access check
merceyz Jan 28, 2024
55c0408
chore: define `NOCK_ENV` as well
merceyz Jan 28, 2024
2ff3ea7
refactor: remove `NODE_ENV`
merceyz Jan 28, 2024
4c12241
chore: remove `fetchJSON`
merceyz Jan 30, 2024
8a5bb71
refactor: move request recording back to `tests/recordRequests.js`
merceyz Feb 3, 2024
ed68f53
refactor: use `v8.serialize`
merceyz Feb 3, 2024
9831441
test: copy body
merceyz Feb 3, 2024
11f70d2
test: store as string
merceyz Feb 3, 2024
a1b9145
refactor: reduce diff
merceyz Feb 7, 2024
c345721
refactor: improve mock loading
merceyz Feb 7, 2024
b87dbc6
refactor: simplify adding new headers
merceyz Feb 7, 2024
b584310
test: store as base64
merceyz Feb 7, 2024
6d0bdca
docs: add some information about the use of base64
merceyz Feb 10, 2024
9f6cf1c
refactor: restore `fetchAsJson` and `fetchUrlStream`
merceyz Feb 11, 2024
1bc41bd
chore: restore `tests/npmRegistryUtils.test.ts`
merceyz Feb 11, 2024
5167cdc
Merge branch 'main' into merceyz/refactor/fetch
merceyz Feb 11, 2024
5163ee8
Update sources/httpUtils.ts
merceyz Feb 11, 2024
67a40c7
chore: revert micro-optimisation
merceyz Feb 11, 2024
59039ad
Revert "Update sources/httpUtils.ts"
merceyz Feb 11, 2024
c8818a3
Apply suggestions from code review
merceyz Feb 11, 2024
574745b
Merge branch 'main' into merceyz/refactor/fetch
merceyz Feb 11, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,11 @@ module.exports = {
extends: [
`@yarnpkg`,
],
rules: {
// eslint-disable-next-line @typescript-eslint/naming-convention
'no-restricted-globals': [`error`, {
name: `fetch`,
message: `Use fetch from sources/httpUtils.ts`,
}],
},
};
2 changes: 1 addition & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
"eslint.nodePath": ".yarn/sdks",
"typescript.enablePromptUseWorkspaceTsdk": true,
"editor.codeActionsOnSave": {
"source.fixAll.eslint": true
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
"source.fixAll.eslint": "explicit"
}
}
8 changes: 3 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"@types/debug": "^4.1.5",
"@types/jest": "^29.0.0",
"@types/node": "^20.4.6",
"@types/proxy-from-env": "^1",
"@types/semver": "^7.1.0",
"@types/tar": "^6.0.0",
"@types/which": "^3.0.0",
Expand All @@ -40,13 +41,13 @@
"eslint": "^8.0.0",
"eslint-plugin-arca": "^0.16.0",
"jest": "^29.0.0",
"nock": "^13.0.4",
"proxy-agent": "^6.2.2",
"proxy-from-env": "^1.1.0",
"semver": "^7.5.2",
"supports-color": "^9.0.0",
"tar": "^6.0.1",
"ts-node": "^10.0.0",
"typescript": "^5.0.4",
"undici": "^6.4.0",
"v8-compile-cache": "^2.3.0",
"which": "^4.0.0"
},
Expand Down Expand Up @@ -94,8 +95,5 @@
"./shims/yarnpkg",
"./shims/yarnpkg.ps1"
]
},
"resolutions": {
"vm2": "portal:./vm2"
}
}
18 changes: 13 additions & 5 deletions sources/corepackUtils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import assert from 'assert';
import {createHash} from 'crypto';
import {once} from 'events';
import {FileHandle} from 'fs/promises';
Expand All @@ -6,6 +7,7 @@ import type {Dir} from 'fs';
import Module from 'module';
import path from 'path';
import semver from 'semver';
import {Readable} from 'stream';

import * as engine from './Engine';
import * as debugUtils from './debugUtils';
Expand All @@ -28,7 +30,8 @@ export async function fetchLatestStableVersion(spec: RegistrySpec): Promise<stri
return await npmRegistryUtils.fetchLatestStableVersion(spec.package);
}
case `url`: {
const data = await httpUtils.fetchAsJson(spec.url);
const response = await httpUtils.fetch(spec.url);
const data: any = await response.json();
return data[spec.fields.tags].stable;
}
default: {
Expand All @@ -43,7 +46,8 @@ export async function fetchAvailableTags(spec: RegistrySpec): Promise<Record<str
return await npmRegistryUtils.fetchAvailableTags(spec.package);
}
case `url`: {
const data = await httpUtils.fetchAsJson(spec.url);
const response = await httpUtils.fetch(spec.url);
const data: any = await response.json();
return data[spec.fields.tags];
}
default: {
Expand All @@ -58,7 +62,8 @@ export async function fetchAvailableVersions(spec: RegistrySpec): Promise<Array<
return await npmRegistryUtils.fetchAvailableVersions(spec.package);
}
case `url`: {
const data = await httpUtils.fetchAsJson(spec.url);
const response = await httpUtils.fetch(spec.url);
const data: any = await response.json();
const field = data[spec.fields.versions];
return Array.isArray(field) ? field : Object.keys(field);
}
Expand Down Expand Up @@ -139,9 +144,12 @@ export async function installVersion(installTarget: string, locator: Locator, {s

const tmpFolder = folderUtils.getTemporaryFolder(installTarget);
debugUtils.log(`Installing ${locator.name}@${version} from ${url} to ${tmpFolder}`);
const stream = await httpUtils.fetchUrlStream(url);

const parsedUrl = new URL(url);
const response = await httpUtils.fetch(parsedUrl);
const webStream = response.body;
assert(webStream, `Expected stream to be set`);
const stream = Readable.fromWeb(webStream);

const ext = path.posix.extname(parsedUrl.pathname);

let outputFile: string | null = null;
Expand Down
97 changes: 36 additions & 61 deletions sources/httpUtils.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,15 @@
import {UsageError} from 'clipanion';
import {once} from 'events';
import type {RequestOptions} from 'https';
import type {IncomingMessage, ClientRequest} from 'http';
import {stderr, stdin} from 'process';
import {UsageError} from 'clipanion';
import {once} from 'events';
import {stderr, stdin} from 'process';

export async function fetchUrlStream(url: string, options: RequestOptions = {}) {
export async function fetch(input: string | URL, init?: RequestInit) {
if (process.env.COREPACK_ENABLE_NETWORK === `0`)
throw new UsageError(`Network access disabled by the environment; can't reach ${url}`);
throw new UsageError(`Network access disabled by the environment; can't reach ${input}`);

const {default: https} = await import(`https`);

const {ProxyAgent} = await import(`proxy-agent`);

const proxyAgent = new ProxyAgent();
const agent = await getProxyAgent(input);

if (process.env.COREPACK_ENABLE_DOWNLOAD_PROMPT === `1`) {
console.error(`Corepack is about to download ${url}.`);
console.error(`Corepack is about to download ${input}.`);
if (stdin.isTTY && !process.env.CI) {
stderr.write(`\nDo you want to continue? [Y/n] `);
stdin.resume();
Expand All @@ -30,60 +24,41 @@ export async function fetchUrlStream(url: string, options: RequestOptions = {})
}
}

return new Promise<IncomingMessage>((resolve, reject) => {
const createRequest = (url: string) => {
const request: ClientRequest = https.get(url, {...options, agent: proxyAgent}, response => {
const statusCode = response.statusCode;

if ([301, 302, 307, 308].includes(statusCode as number) && response.headers.location)
return createRequest(response.headers.location as string);

if (statusCode != null && statusCode >= 200 && statusCode < 300)
return resolve(response);

return reject(new Error(`Server answered with HTTP ${statusCode} when performing the request to ${url}; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting`));
});
let response;
try {
response = await globalThis.fetch(input, {
...init,
dispatcher: agent,
});
} catch (error) {
throw new Error(
`Error when performing the request to ${input}; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting`,
{cause: error},
);
}

request.on(`error`, err => {
reject(new Error(`Error when performing the request to ${url}; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting`));
});
};
if (!response.ok) {
await response.arrayBuffer();
throw new Error(
`Server answered with HTTP ${response.status} when performing the request to ${input}; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting`,
);
}

createRequest(url);
});
return response;
}

export async function fetchAsBuffer(url: string, options?: RequestOptions) {
const response = await fetchUrlStream(url, options);
async function getProxyAgent(input: string | URL) {
const {getProxyForUrl} = await import(`proxy-from-env`);

return new Promise<Buffer>((resolve, reject) => {
const chunks: Array<Buffer> = [];
const proxy = getProxyForUrl(input.toString());
merceyz marked this conversation as resolved.
Show resolved Hide resolved
merceyz marked this conversation as resolved.
Show resolved Hide resolved

response.on(`data`, chunk => {
chunks.push(chunk);
});

response.on(`error`, error => {
reject(error);
});

response.on(`end`, () => {
resolve(Buffer.concat(chunks));
});
});
}
if (!proxy) return undefined;

export async function fetchAsJson(url: string, options?: RequestOptions) {
const buffer = await fetchAsBuffer(url, options);
const asText = buffer.toString();
// Doing a deep import here since undici isn't tree-shakeable
const {default: ProxyAgent} = (await import(
// @ts-expect-error No types for this specific file
`undici/lib/proxy-agent.js`
)) as { default: typeof import('undici').ProxyAgent };

try {
return JSON.parse(asText);
} catch (error) {
const truncated = asText.length > 30
? `${asText.slice(0, 30)}...`
: asText;

throw new Error(`Couldn't parse JSON data: ${JSON.stringify(truncated)}`);
}
return new ProxyAgent(proxy);
}
11 changes: 6 additions & 5 deletions sources/npmRegistryUtils.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import {UsageError} from 'clipanion';
import {OutgoingHttpHeaders} from 'http2';
import {UsageError} from 'clipanion';

import * as httpUtils from './httpUtils';
import * as httpUtils from './httpUtils';

// load abbreviated metadata as that's all we need for these calls
// see: https://github.com/npm/registry/blob/cfe04736f34db9274a780184d1cdb2fb3e4ead2a/docs/responses/package-metadata.md
export const DEFAULT_HEADERS: OutgoingHttpHeaders = {
export const DEFAULT_HEADERS: Record<string, string> = {
[`Accept`]: `application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8`,
};
export const DEFAULT_NPM_REGISTRY_URL = `https://registry.npmjs.org`;
Expand All @@ -26,7 +25,9 @@ export async function fetchAsJson(packageName: string) {
headers.authorization = `Basic ${encodedCreds}`;
}

return httpUtils.fetchAsJson(`${npmRegistryUrl}/${packageName}`, {headers});
const response = await httpUtils.fetch(`${npmRegistryUrl}/${packageName}`, {headers});
const data: any = await response.json();
return data;
merceyz marked this conversation as resolved.
Show resolved Hide resolved
}

export async function fetchLatestStableVersion(packageName: string) {
Expand Down
95 changes: 0 additions & 95 deletions tests/httpUtils.test.ts

This file was deleted.