diff --git a/.yarn/versions/7111fix0.yml b/.yarn/versions/7111fix0.yml new file mode 100644 index 000000000000..d67cc3d44af3 --- /dev/null +++ b/.yarn/versions/7111fix0.yml @@ -0,0 +1,37 @@ +releases: + "@yarnpkg/cli": patch + "@yarnpkg/core": patch + "@yarnpkg/plugin-essentials": patch + "@yarnpkg/plugin-typescript": patch + +declined: + - "@yarnpkg/extensions" + - "@yarnpkg/plugin-catalog" + - "@yarnpkg/plugin-compat" + - "@yarnpkg/plugin-constraints" + - "@yarnpkg/plugin-dlx" + - "@yarnpkg/plugin-exec" + - "@yarnpkg/plugin-file" + - "@yarnpkg/plugin-git" + - "@yarnpkg/plugin-github" + - "@yarnpkg/plugin-http" + - "@yarnpkg/plugin-init" + - "@yarnpkg/plugin-interactive-tools" + - "@yarnpkg/plugin-jsr" + - "@yarnpkg/plugin-link" + - "@yarnpkg/plugin-nm" + - "@yarnpkg/plugin-npm" + - "@yarnpkg/plugin-npm-cli" + - "@yarnpkg/plugin-pack" + - "@yarnpkg/plugin-patch" + - "@yarnpkg/plugin-pnp" + - "@yarnpkg/plugin-pnpm" + - "@yarnpkg/plugin-stage" + - "@yarnpkg/plugin-version" + - "@yarnpkg/plugin-workspace-tools" + - "@yarnpkg/builder" + - "@yarnpkg/doctor" + - "@yarnpkg/nm" + - "@yarnpkg/pnp" + - "@yarnpkg/pnpify" + - "@yarnpkg/sdks" diff --git a/packages/acceptance-tests/pkg-tests-specs/sources/plugins/plugin-typescript.test.ts b/packages/acceptance-tests/pkg-tests-specs/sources/plugins/plugin-typescript.test.ts index 779683b5e066..c21038a8b8ba 100644 --- a/packages/acceptance-tests/pkg-tests-specs/sources/plugins/plugin-typescript.test.ts +++ b/packages/acceptance-tests/pkg-tests-specs/sources/plugins/plugin-typescript.test.ts @@ -1,6 +1,9 @@ import {Manifest} from '@yarnpkg/core'; import {PortablePath, ppath, xfs} from '@yarnpkg/fslib'; import {merge} from 'es-toolkit/compat'; +import events from 'events'; +import {AddressInfo} from 'net'; +import net from 'net'; import {fs, yarn} from 'pkg-tests-core'; const {unpackToDirectory} = fs; @@ -66,6 +69,52 @@ describe(`Plugins`, () => { }), ); + test( + `it should warn and add the package without @types when the Algolia index can't be reached`, + makeTemporaryEnv({}, async ({path, run, source}) => { + // Accepts the connection then immediately drops it, just like a + // firewall sitting between Yarn and Algolia would + const blackhole = net.createServer(socket => socket.destroy()); + + blackhole.listen(0, `127.0.0.1`); + await events.once(blackhole, `listening`); + + try { + const {port} = blackhole.address() as AddressInfo; + + await xfs.writeFilePromise(ppath.join(path, `tsconfig.json`), ``); + + // Only the Algolia lookup goes through the blackhole; the registry + // is configured through the environment and stays reachable + await xfs.writeFilePromise(ppath.join(path, `.yarnrc.yml`), [ + `httpRetry: 0`, + `networkSettings:`, + ` "*.algolia.net":`, + ` httpsProxy: "http://127.0.0.1:${port}"`, + ` "*.algolianet.com":`, + ` httpsProxy: "http://127.0.0.1:${port}"`, + ].join(`\n`)); + + const {stdout} = await run(`add`, `is-number`); + + expect(stdout).toMatch(/Couldn't query Algolia's npm-search index/); + + const manifest = await readManifest(path); + + expect(manifest).toMatchObject({ + dependencies: { + [`is-number`]: `^2.0.0`, + }, + }); + + expect(manifest).not.toHaveProperty(`devDependencies`); + } finally { + blackhole.close(); + await events.once(blackhole, `close`); + } + }), + ); + test( `it should automatically enable automatic @types insertion in the current workspace when tsEnableAutoTypes is set to true`, makeTemporaryMonorepoEnv({ diff --git a/packages/plugin-essentials/sources/commands/add.ts b/packages/plugin-essentials/sources/commands/add.ts index 522c77131eef..44c87ab130c3 100644 --- a/packages/plugin-essentials/sources/commands/add.ts +++ b/packages/plugin-essentials/sources/commands/add.ts @@ -1,6 +1,6 @@ import {BaseCommand, WorkspaceRequiredError} from '@yarnpkg/cli'; import {Cache, Configuration, Descriptor, formatUtils, LightReport, MessageName} from '@yarnpkg/core'; -import {Project, Workspace, Ident, InstallMode} from '@yarnpkg/core'; +import {Project, StreamReport, Workspace, Ident, InstallMode} from '@yarnpkg/core'; import {structUtils} from '@yarnpkg/core'; import {PortablePath} from '@yarnpkg/fslib'; import {Command, Option, Usage, UsageError} from 'clipanion'; @@ -331,15 +331,28 @@ export default class AddCommand extends BaseCommand { } } - await configuration.triggerMultipleHooks( - (hooks: Hooks) => hooks.afterWorkspaceDependencyAddition, - afterWorkspaceDependencyAdditionList, - ); + // Those hooks may report warnings (eg. plugin-typescript when it can't + // reach Algolia); without a report around them Node would print them as + // raw process warnings rather than as regular Yarn messages + const hookReport = await StreamReport.start({ + configuration, + includeFooter: false, + json: this.json, + stdout: this.context.stdout, + }, async () => { + await configuration.triggerMultipleHooks( + (hooks: Hooks) => hooks.afterWorkspaceDependencyAddition, + afterWorkspaceDependencyAdditionList, + ); + + await configuration.triggerMultipleHooks( + (hooks: Hooks) => hooks.afterWorkspaceDependencyReplacement, + afterWorkspaceDependencyReplacementList, + ); + }); - await configuration.triggerMultipleHooks( - (hooks: Hooks) => hooks.afterWorkspaceDependencyReplacement, - afterWorkspaceDependencyReplacementList, - ); + if (hookReport.hasErrors()) + return hookReport.exitCode(); if (askedQuestions) this.context.stdout.write(`\n`); diff --git a/packages/plugin-typescript/sources/typescriptUtils.ts b/packages/plugin-typescript/sources/typescriptUtils.ts index bb4f55784b25..e13a3ed5dcb8 100644 --- a/packages/plugin-typescript/sources/typescriptUtils.ts +++ b/packages/plugin-typescript/sources/typescriptUtils.ts @@ -1,13 +1,20 @@ -import {Request, Requester, Response} from '@algolia/requester-common'; -import {Configuration, Descriptor} from '@yarnpkg/core'; -import {httpUtils, structUtils} from '@yarnpkg/core'; -import algoliasearch from 'algoliasearch'; +import {Request, Requester, Response} from '@algolia/requester-common'; +import {Configuration, Descriptor} from '@yarnpkg/core'; +import {formatUtils, httpUtils, structUtils} from '@yarnpkg/core'; +import algoliasearch from 'algoliasearch'; // Note that the appId and appKey are specific to Yarn's plugin-typescript - please // don't use them anywhere else without asking Algolia's permission const ALGOLIA_API_KEY = `e8e1bd300d860104bb8c58453ffa1eb4`; const ALGOLIA_APP_ID = `OFCNCOG2CU`; +// Maximum time (in milliseconds) we're willing to wait for Algolia to tell us +// whether a package ships its types through DefinitelyTyped. Without this cap a +// restricted network (eg. a corporate proxy that silently drops the request) +// would make `yarn add` hang indefinitely. +// See https://github.com/yarnpkg/berry/issues/7111 +const ALGOLIA_TIMEOUT = 10000; + interface AlgoliaObj { types?: { ts?: string; @@ -19,25 +26,55 @@ export const hasDefinitelyTyped = async ( configuration: Configuration, ) => { const stringifiedIdent = structUtils.stringifyIdent(descriptor); - const algoliaClient = createAlgoliaClient(configuration); + const abortController = new AbortController(); + const algoliaClient = createAlgoliaClient(configuration, abortController.signal); const index = algoliaClient.initIndex(`npm-search`); + // Note that we can't use `AbortSignal.timeout` here: its timer lives in Node's + // internals rather than on the global `setTimeout`, so tests can't advance it. + const timeout = setTimeout(() => { + abortController.abort(new Error(`Timed out after ${ALGOLIA_TIMEOUT}ms`)); + }, ALGOLIA_TIMEOUT); + try { const packageInfo = await index.getObject(stringifiedIdent, {attributesToRetrieve: [`types`]}); return packageInfo.types?.ts === `definitely-typed`; - } catch { + } catch (error) { + // A timeout or a network error (eg. a proxy blocking the request) shouldn't + // prevent the package from being added - we just can't tell whether it needs + // a matching `@types` package, so we let the user know and carry on. + if (abortController.signal.aborted || error?.name === `RetryError`) + reportAutoTypesError(configuration, descriptor, error); + return false; + } finally { + clearTimeout(timeout); } }; -const createAlgoliaClient = (configuration: Configuration) => { +const reportAutoTypesError = (configuration: Configuration, descriptor: Descriptor, error: Error) => { + const prettyIdent = structUtils.prettyIdent(configuration, descriptor); + + // Reported as two warnings rather than one multi-line message, as reports + // only prefix the first line of what they're given + process.emitWarning( + `Couldn't query Algolia's npm-search index to check whether ${prettyIdent} needs a matching @types package (${error.message}); the package will be added without it.`, + ); + + process.emitWarning( + `You can disable this lookup by setting ${formatUtils.pretty(configuration, `tsEnableAutoTypes`, formatUtils.Type.SETTING)} to false in your .yarnrc.yml (or by setting the YARN_TS_ENABLE_AUTO_TYPES="false" environment variable).`, + ); +}; + +const createAlgoliaClient = (configuration: Configuration, signal: AbortSignal) => { const requester: Requester = { async send(request: Request): Promise { try { const response = await httpUtils.request(request.url, request.data || null, { configuration, headers: request.headers, + signal, }); return { @@ -46,10 +83,16 @@ const createAlgoliaClient = (configuration: Configuration) => { status: response.statusCode, }; } catch (error) { + if (signal.aborted) + throw signal.reason; + + // Connection errors (eg. a proxy refusing the request) don't always + // carry a `response`, so we have to guard against it to avoid throwing + // an unrelated `TypeError` from within the requester itself. return { - content: error.response.body, - isTimedOut: false, - status: error.response.statusCode, + content: error.response?.body, + isTimedOut: error.code === `ETIMEDOUT`, + status: error.response?.statusCode ?? 0, }; } }}; diff --git a/packages/plugin-typescript/tests/typescriptUtils.test.ts b/packages/plugin-typescript/tests/typescriptUtils.test.ts new file mode 100644 index 000000000000..b9ad85e82d66 --- /dev/null +++ b/packages/plugin-typescript/tests/typescriptUtils.test.ts @@ -0,0 +1,91 @@ +import {Configuration, Hooks, Plugin, httpUtils, structUtils} from '@yarnpkg/core'; +import {PortablePath} from '@yarnpkg/fslib'; + +import {hasDefinitelyTyped} from '../sources/typescriptUtils'; +import plugin from '../sources'; + +const requestMock = jest.fn(); + +const descriptor = structUtils.makeDescriptor( + structUtils.makeIdent(null, `is-number`), + `unknown`, +); + +const makeConfiguration = (executeRequest: (signal: AbortSignal) => Promise) => { + const testPlugin: Plugin = { + hooks: { + wrapNetworkRequest: async (_executor, {signal}) => { + if (typeof signal === `undefined`) + throw new Error(`Expected the Algolia request to receive an abort signal`); + + requestMock(signal); + + return () => executeRequest(signal); + }, + }, + }; + + return Configuration.create(PortablePath.root, new Map([ + [`@yarnpkg/plugin-typescript`, plugin], + [`test-plugin`, testPlugin], + ])); +}; + +const flushPromises = async () => { + for (let t = 0; t < 10; t++) { + await Promise.resolve(); + } +}; + +afterEach(() => { + jest.useRealTimers(); + jest.restoreAllMocks(); + requestMock.mockReset(); +}); + +describe(`typescriptUtils`, () => { + describe(`hasDefinitelyTyped`, () => { + it(`aborts the Algolia request when the lookup times out`, async () => { + jest.useFakeTimers(); + + const emitWarning = jest.spyOn(process, `emitWarning`).mockImplementation(() => {}); + const configuration = makeConfiguration(signal => { + return new Promise((_resolve, reject) => { + if (signal.aborted) { + reject(signal.reason); + } else { + signal.addEventListener(`abort`, () => { + reject(signal.reason); + }, {once: true}); + } + }); + }); + + const result = hasDefinitelyTyped(descriptor, configuration); + + await flushPromises(); + expect(requestMock).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(10_000); + + await expect(result).resolves.toBe(false); + await flushPromises(); + + expect(requestMock).toHaveBeenCalledTimes(1); + expect(requestMock.mock.calls[0][0].aborted).toBe(true); + expect(emitWarning).toHaveBeenCalledWith(expect.stringContaining(`Couldn't query Algolia's npm-search index`)); + }); + + it(`warns and returns false when all Algolia hosts are unreachable`, async () => { + const emitWarning = jest.spyOn(process, `emitWarning`).mockImplementation(() => {}); + const configuration = makeConfiguration(async () => { + throw new Error(`Network unavailable`); + }); + + await expect(hasDefinitelyTyped(descriptor, configuration)).resolves.toBe(false); + + expect(requestMock).toHaveBeenCalledTimes(4); + expect(emitWarning).toHaveBeenCalledWith(expect.stringContaining(`Couldn't query Algolia's npm-search index`)); + }); + }); +}); diff --git a/packages/yarnpkg-core/sources/httpUtils.ts b/packages/yarnpkg-core/sources/httpUtils.ts index e6c03d22a424..ef30cdf0b389 100644 --- a/packages/yarnpkg-core/sources/httpUtils.ts +++ b/packages/yarnpkg-core/sources/httpUtils.ts @@ -168,11 +168,12 @@ export type Options = { jsonRequest?: boolean; jsonResponse?: boolean; method?: Method; + signal?: AbortSignal; wrapNetworkRequest?: (executor: () => Promise, extra: WrapNetworkRequestInfo) => Promise<() => Promise>; }; -export async function request(target: string | URL, body: Body, {configuration, headers, jsonRequest, jsonResponse, method = Method.GET, wrapNetworkRequest}: Omit) { - const options = {target, body, configuration, headers, jsonRequest, jsonResponse, method}; +export async function request(target: string | URL, body: Body, {configuration, headers, jsonRequest, jsonResponse, method = Method.GET, signal, wrapNetworkRequest}: Omit) { + const options = {target, body, configuration, headers, jsonRequest, jsonResponse, method, signal}; const realRequest = async () => await requestImpl(target, body, options); @@ -187,13 +188,14 @@ export async function request(target: string | URL, body: Body, {configuration, return await executor(); } -export async function get(target: string, {configuration, jsonResponse, customErrorMessage, wrapNetworkRequest, ...rest}: Options) { - const runRequest = () => prettyNetworkError(request(target, null, {configuration, wrapNetworkRequest, ...rest}), {configuration, customErrorMessage}) +export async function get(target: string, {configuration, jsonResponse, customErrorMessage, signal, wrapNetworkRequest, ...rest}: Options) { + const runRequest = () => prettyNetworkError(request(target, null, {configuration, signal, wrapNetworkRequest, ...rest}), {configuration, customErrorMessage}) .then(response => response.body); - // We cannot cache responses when wrapNetworkRequest is used, as it can differ between calls + // We cannot cache responses when wrapNetworkRequest is used, as it can differ between calls. + // Requests with a signal must also stay independent so aborting one doesn't cancel another. const entry = await ( - typeof wrapNetworkRequest !== `undefined` + typeof wrapNetworkRequest !== `undefined` || typeof signal !== `undefined` ? runRequest() : miscUtils.getFactoryWithDefault(cache, target, () => { return runRequest().then(body => { @@ -228,7 +230,7 @@ export async function del(target: string, {customErrorMessage, ...options}: Opti return response.body; } -async function requestImpl(target: string | URL, body: Body, {configuration, headers, jsonRequest, jsonResponse, method = Method.GET}: Omit): Promise { +async function requestImpl(target: string | URL, body: Body, {configuration, headers, jsonRequest, jsonResponse, method = Method.GET, signal}: Omit): Promise { const url = typeof target === `string` ? new URL(target) : target; const networkConfig = getNetworkSettings(url, {configuration}); @@ -276,6 +278,7 @@ async function requestImpl(target: string | URL, body: Body, {configuration, hea ca: certificateAuthority, cert: certificate, key, + signal, }; const agent = { @@ -311,7 +314,18 @@ async function requestImpl(target: string | URL, body: Body, {configuration, hea ...gotOptions, }); - return configuration.getLimit(`networkConcurrency`)(() => { - return gotClient(url); + return configuration.getLimit(`networkConcurrency`)(async () => { + signal?.throwIfAborted(); + + const request = gotClient(url); + const cancelRequest = () => request.cancel(); + + signal?.addEventListener(`abort`, cancelRequest, {once: true}); + + try { + return await request; + } finally { + signal?.removeEventListener(`abort`, cancelRequest); + } }); } diff --git a/packages/yarnpkg-core/tests/httpUtils.test.ts b/packages/yarnpkg-core/tests/httpUtils.test.ts index 2f14a845f1a7..b6ca2f92089d 100644 --- a/packages/yarnpkg-core/tests/httpUtils.test.ts +++ b/packages/yarnpkg-core/tests/httpUtils.test.ts @@ -1,5 +1,10 @@ import {Configuration, Plugin, httpUtils} from '@yarnpkg/core'; import {npath} from '@yarnpkg/fslib'; +import events from 'events'; +import http from 'http'; +import {AddressInfo, Socket} from 'net'; +import net from 'net'; +import {setTimeout} from 'timers/promises'; describe(`httpUtils`, () => { describe(`request`, () => { @@ -17,9 +22,10 @@ describe(`httpUtils`, () => { const jsonRequest = true; const jsonResponse = true; const method = httpUtils.Method.PUT; + const signal = new AbortController().signal; // Act - await httpUtils.request(target, body, {configuration, headers, jsonRequest, jsonResponse, method}); + await httpUtils.request(target, body, {configuration, headers, jsonRequest, jsonResponse, method, signal}); // Assert expect(mockWrapNetworkRequest.mock.calls.length).toBe(1); @@ -32,9 +38,109 @@ describe(`httpUtils`, () => { expect(hookArgumentResult.jsonRequest).toBe(jsonRequest); expect(hookArgumentResult.jsonResponse).toBe(jsonResponse); expect(hookArgumentResult.method).toBe(method); + expect(hookArgumentResult.signal).toBe(signal); + }); + + it(`cancels active requests and releases their network concurrency slot`, async () => { + // Requests are never answered, so they stay active until they get cancelled + const server = http.createServer(() => {}); + + server.listen(0, `127.0.0.1`); + await events.once(server, `listening`); + + try { + const configuration = Configuration.create(npath.toPortablePath(`.`)); + configuration.values.set(`httpRetry`, 0); + configuration.values.set(`networkConcurrency`, 1); + configuration.values.set(`unsafeHttpWhitelist`, [`127.0.0.1`]); + + const abortController = new AbortController(); + const {port} = server.address() as AddressInfo; + const request = httpUtils.request(`http://127.0.0.1:${port}`, null, { + configuration, + signal: abortController.signal, + }); + + await events.once(server, `request`); + + let queuedRequestStarted = false; + const queuedRequest = configuration.getLimit(`networkConcurrency`)(async () => { + queuedRequestStarted = true; + }); + + expect(queuedRequestStarted).toBe(false); + + const requestExpectation = expect(request).rejects.toMatchObject({ + name: `CancelError`, + }); + + abortController.abort(); + + await requestExpectation; + await expectToSettle(queuedRequest, `Expected the cancelled request to release its network concurrency slot`); + + expect(queuedRequestStarted).toBe(true); + } finally { + server.closeAllConnections(); + server.close(); + await events.once(server, `close`); + } + }); + + it(`cancels proxy connections while their tunnel is being established`, async () => { + // The tunnel is never established, so the proxy connection stays open until + // it gets cancelled + const server = net.createServer(); + + server.listen(0, `127.0.0.1`); + await events.once(server, `listening`); + + let proxySocket: Socket | undefined; + + try { + const configuration = Configuration.create(npath.toPortablePath(`.`)); + const {port} = server.address() as AddressInfo; + configuration.values.set(`httpsProxy`, `http://127.0.0.1:${port}`); + configuration.values.set(`httpRetry`, 0); + + const abortController = new AbortController(); + const proxyConnection = events.once(server, `connection`); + const request = httpUtils.request(`https://example.com`, null, { + configuration, + signal: abortController.signal, + }); + + const [socket] = await proxyConnection as [Socket]; + proxySocket = socket; + + await events.once(socket, `data`); + + const proxySocketClosed = events.once(socket, `close`); + + const requestExpectation = expect(request).rejects.toMatchObject({ + name: `CancelError`, + }); + + abortController.abort(); + + await requestExpectation; + await expectToSettle(proxySocketClosed, `Expected the cancelled proxy connection to close`); + } finally { + proxySocket?.destroy(); + server.close(); + await events.once(server, `close`); + } }); }); + // Without this the tests would hang until Jest's own timeout kicks in, which + // wouldn't tell us which of the expectations actually failed + async function expectToSettle(promise: Promise, message: string) { + await Promise.race([promise, setTimeout(2_000, undefined, {ref: false}).then(() => { + throw new Error(message); + })]); + } + function getPluginsWithMockWrapNetworkRequestPlugin() { const mockWrapNetworkRequest = jest.fn(); const plugins = new Map>();