From 99d459a3855eecc8b2ca730f0adf9e21a82c96cd Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 24 Jun 2026 18:43:45 +0000 Subject: [PATCH 1/2] fix(plugin-typescript): add a timeout to the Algolia auto-types lookup Bound the optional Algolia auto-types lookup to 10 seconds and cancel the underlying Yarn HTTP request when the deadline expires, including active proxy tunnels. This releases the network concurrency slot and prevents Algolia from retrying fallback hosts after the command has already continued. Warn clearly on timeout or network failure, continue without the matching @types package, and harden transport errors that have no response. Add deterministic cancellation and warning coverage. Closes #7111 Co-authored-by: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01NS1JKnVAz9uPwdyUhfub2g Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .yarn/versions/7111fix0.yml | 37 +++++ .../sources/typescriptUtils.ts | 72 +++++++-- .../tests/typescriptUtils.test.ts | 91 +++++++++++ packages/yarnpkg-core/sources/httpUtils.ts | 32 ++-- packages/yarnpkg-core/tests/httpUtils.test.ts | 141 +++++++++++++++++- 5 files changed, 352 insertions(+), 21 deletions(-) create mode 100644 .yarn/versions/7111fix0.yml create mode 100644 packages/plugin-typescript/tests/typescriptUtils.test.ts diff --git a/.yarn/versions/7111fix0.yml b/.yarn/versions/7111fix0.yml new file mode 100644 index 000000000000..bd8bfe9f2432 --- /dev/null +++ b/.yarn/versions/7111fix0.yml @@ -0,0 +1,37 @@ +releases: + "@yarnpkg/core": patch + "@yarnpkg/plugin-typescript": patch + +declined: + - "@yarnpkg/cli" + - "@yarnpkg/extensions" + - "@yarnpkg/plugin-catalog" + - "@yarnpkg/plugin-compat" + - "@yarnpkg/plugin-constraints" + - "@yarnpkg/plugin-dlx" + - "@yarnpkg/plugin-essentials" + - "@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/plugin-typescript/sources/typescriptUtils.ts b/packages/plugin-typescript/sources/typescriptUtils.ts index bb4f55784b25..c079c60d0ad9 100644 --- a/packages/plugin-typescript/sources/typescriptUtils.ts +++ b/packages/plugin-typescript/sources/typescriptUtils.ts @@ -1,43 +1,87 @@ -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; }; } +class AlgoliaTimeoutError extends Error { + constructor() { + super(`Timed out after ${ALGOLIA_TIMEOUT}ms`); + } +} + export const hasDefinitelyTyped = async ( descriptor: Descriptor, 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`); + let timeout: ReturnType | undefined; + try { - const packageInfo = await index.getObject(stringifiedIdent, {attributesToRetrieve: [`types`]}); + const packageInfo = await Promise.race([ + index.getObject(stringifiedIdent, {attributesToRetrieve: [`types`]}), + new Promise((resolve, reject) => { + timeout = setTimeout(() => { + const error = new AlgoliaTimeoutError(); + + reject(error); + abortController.abort(error); + }, ALGOLIA_TIMEOUT); + }), + ]); 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 (error instanceof AlgoliaTimeoutError || 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); + + 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.\n` + + `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 +90,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..0e0a9cf10b73 100644 --- a/packages/yarnpkg-core/tests/httpUtils.test.ts +++ b/packages/yarnpkg-core/tests/httpUtils.test.ts @@ -1,5 +1,8 @@ import {Configuration, Plugin, httpUtils} from '@yarnpkg/core'; import {npath} from '@yarnpkg/fslib'; +import http from 'http'; +import {AddressInfo, Socket} from 'net'; +import net from 'net'; describe(`httpUtils`, () => { describe(`request`, () => { @@ -17,9 +20,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,6 +36,141 @@ 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 () => { + const sockets = new Set(); + let resolveRequest!: () => void; + const requestReceived = new Promise(resolve => { + resolveRequest = resolve; + }); + + const server = http.createServer(() => { + resolveRequest(); + }); + server.on(`connection`, socket => { + sockets.add(socket); + socket.on(`close`, () => { + sockets.delete(socket); + }); + }); + + await new Promise(resolve => { + server.listen(0, `127.0.0.1`, resolve); + }); + + 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 requestReceived; + + 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 queuedRequest; + + expect(queuedRequestStarted).toBe(true); + } finally { + for (const socket of sockets) + socket.destroy(); + + await new Promise(resolve => { + server.close(() => resolve()); + }); + } + }); + + it(`cancels proxy connections while their tunnel is being established`, async () => { + const sockets = new Set(); + let resolveProxyRequest!: () => void; + const proxyRequestReceived = new Promise(resolve => { + resolveProxyRequest = resolve; + }); + let resolveProxySocketClosed!: () => void; + const proxySocketClosed = new Promise(resolve => { + resolveProxySocketClosed = resolve; + }); + + const server = net.createServer(socket => { + sockets.add(socket); + socket.once(`data`, () => { + resolveProxyRequest(); + }); + socket.once(`close`, () => { + sockets.delete(socket); + resolveProxySocketClosed(); + }); + }); + + await new Promise(resolve => { + server.listen(0, `127.0.0.1`, resolve); + }); + + 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 request = httpUtils.request(`https://example.com`, null, { + configuration, + signal: abortController.signal, + }); + + await proxyRequestReceived; + + const requestExpectation = expect(request).rejects.toMatchObject({ + name: `CancelError`, + }); + + abortController.abort(); + + await requestExpectation; + + let timeout: ReturnType | undefined; + try { + await Promise.race([ + proxySocketClosed, + new Promise((_resolve, reject) => { + timeout = setTimeout(() => { + reject(new Error(`Expected the aborted proxy connection to close`)); + }, 2_000); + }), + ]); + } finally { + clearTimeout(timeout); + } + } finally { + for (const socket of sockets) + socket.destroy(); + + await new Promise(resolve => { + server.close(() => resolve()); + }); + } }); }); From fde430db9b87e54cb945b20cc7bbd8133fb5be3a Mon Sep 17 00:00:00 2001 From: Sebastian Danielsson Date: Wed, 5 Aug 2026 16:56:54 +0200 Subject: [PATCH 2/2] fix(plugin-essentials): report dependency hook warnings through a report The `afterWorkspaceDependencyAddition` and `afterWorkspaceDependencyReplacement` hooks were triggered outside of any report, so warnings they emitted (such as the new plugin-typescript one) were printed as raw Node.js process warnings instead of regular Yarn messages. Also addresses the review feedback on #7206: - drops the `Promise.race` in `hasDefinitelyTyped` in favour of checking `signal.aborted` in the `catch` clause - rewrites the httpUtils cancellation tests around `events.once`, `server.closeAllConnections()`, and `setTimeout` from `timers/promises` - adds an acceptance test covering an unreachable Algolia index - releases `@yarnpkg/cli` and `@yarnpkg/plugin-essentials` Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0158RapNp7NHWhFqHp3gKNxm --- .yarn/versions/7111fix0.yml | 4 +- .../sources/plugins/plugin-typescript.test.ts | 49 ++++++++ .../plugin-essentials/sources/commands/add.ts | 31 +++-- .../sources/typescriptUtils.ts | 33 +++--- packages/yarnpkg-core/tests/httpUtils.test.ts | 107 ++++++------------ 5 files changed, 123 insertions(+), 101 deletions(-) diff --git a/.yarn/versions/7111fix0.yml b/.yarn/versions/7111fix0.yml index bd8bfe9f2432..d67cc3d44af3 100644 --- a/.yarn/versions/7111fix0.yml +++ b/.yarn/versions/7111fix0.yml @@ -1,15 +1,15 @@ releases: + "@yarnpkg/cli": patch "@yarnpkg/core": patch + "@yarnpkg/plugin-essentials": patch "@yarnpkg/plugin-typescript": patch declined: - - "@yarnpkg/cli" - "@yarnpkg/extensions" - "@yarnpkg/plugin-catalog" - "@yarnpkg/plugin-compat" - "@yarnpkg/plugin-constraints" - "@yarnpkg/plugin-dlx" - - "@yarnpkg/plugin-essentials" - "@yarnpkg/plugin-exec" - "@yarnpkg/plugin-file" - "@yarnpkg/plugin-git" 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 c079c60d0ad9..e13a3ed5dcb8 100644 --- a/packages/plugin-typescript/sources/typescriptUtils.ts +++ b/packages/plugin-typescript/sources/typescriptUtils.ts @@ -21,12 +21,6 @@ interface AlgoliaObj { }; } -class AlgoliaTimeoutError extends Error { - constructor() { - super(`Timed out after ${ALGOLIA_TIMEOUT}ms`); - } -} - export const hasDefinitelyTyped = async ( descriptor: Descriptor, configuration: Configuration, @@ -36,27 +30,21 @@ export const hasDefinitelyTyped = async ( const algoliaClient = createAlgoliaClient(configuration, abortController.signal); const index = algoliaClient.initIndex(`npm-search`); - let timeout: ReturnType | undefined; + // 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 Promise.race([ - index.getObject(stringifiedIdent, {attributesToRetrieve: [`types`]}), - new Promise((resolve, reject) => { - timeout = setTimeout(() => { - const error = new AlgoliaTimeoutError(); - - reject(error); - abortController.abort(error); - }, ALGOLIA_TIMEOUT); - }), - ]); + const packageInfo = await index.getObject(stringifiedIdent, {attributesToRetrieve: [`types`]}); return packageInfo.types?.ts === `definitely-typed`; } 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 (error instanceof AlgoliaTimeoutError || error?.name === `RetryError`) + if (abortController.signal.aborted || error?.name === `RetryError`) reportAutoTypesError(configuration, descriptor, error); return false; @@ -68,8 +56,13 @@ export const hasDefinitelyTyped = async ( 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( - `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.\n` + `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).`, ); }; diff --git a/packages/yarnpkg-core/tests/httpUtils.test.ts b/packages/yarnpkg-core/tests/httpUtils.test.ts index 0e0a9cf10b73..b6ca2f92089d 100644 --- a/packages/yarnpkg-core/tests/httpUtils.test.ts +++ b/packages/yarnpkg-core/tests/httpUtils.test.ts @@ -1,8 +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`, () => { @@ -40,25 +42,11 @@ describe(`httpUtils`, () => { }); it(`cancels active requests and releases their network concurrency slot`, async () => { - const sockets = new Set(); - let resolveRequest!: () => void; - const requestReceived = new Promise(resolve => { - resolveRequest = resolve; - }); - - const server = http.createServer(() => { - resolveRequest(); - }); - server.on(`connection`, socket => { - sockets.add(socket); - socket.on(`close`, () => { - sockets.delete(socket); - }); - }); + // Requests are never answered, so they stay active until they get cancelled + const server = http.createServer(() => {}); - await new Promise(resolve => { - server.listen(0, `127.0.0.1`, resolve); - }); + server.listen(0, `127.0.0.1`); + await events.once(server, `listening`); try { const configuration = Configuration.create(npath.toPortablePath(`.`)); @@ -73,7 +61,7 @@ describe(`httpUtils`, () => { signal: abortController.signal, }); - await requestReceived; + await events.once(server, `request`); let queuedRequestStarted = false; const queuedRequest = configuration.getLimit(`networkConcurrency`)(async () => { @@ -89,44 +77,25 @@ describe(`httpUtils`, () => { abortController.abort(); await requestExpectation; - await queuedRequest; + await expectToSettle(queuedRequest, `Expected the cancelled request to release its network concurrency slot`); expect(queuedRequestStarted).toBe(true); } finally { - for (const socket of sockets) - socket.destroy(); - - await new Promise(resolve => { - server.close(() => resolve()); - }); + server.closeAllConnections(); + server.close(); + await events.once(server, `close`); } }); it(`cancels proxy connections while their tunnel is being established`, async () => { - const sockets = new Set(); - let resolveProxyRequest!: () => void; - const proxyRequestReceived = new Promise(resolve => { - resolveProxyRequest = resolve; - }); - let resolveProxySocketClosed!: () => void; - const proxySocketClosed = new Promise(resolve => { - resolveProxySocketClosed = resolve; - }); - - const server = net.createServer(socket => { - sockets.add(socket); - socket.once(`data`, () => { - resolveProxyRequest(); - }); - socket.once(`close`, () => { - sockets.delete(socket); - resolveProxySocketClosed(); - }); - }); + // 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`); - await new Promise(resolve => { - server.listen(0, `127.0.0.1`, resolve); - }); + let proxySocket: Socket | undefined; try { const configuration = Configuration.create(npath.toPortablePath(`.`)); @@ -135,12 +104,18 @@ describe(`httpUtils`, () => { 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, }); - await proxyRequestReceived; + 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`, @@ -149,31 +124,23 @@ describe(`httpUtils`, () => { abortController.abort(); await requestExpectation; - - let timeout: ReturnType | undefined; - try { - await Promise.race([ - proxySocketClosed, - new Promise((_resolve, reject) => { - timeout = setTimeout(() => { - reject(new Error(`Expected the aborted proxy connection to close`)); - }, 2_000); - }), - ]); - } finally { - clearTimeout(timeout); - } + await expectToSettle(proxySocketClosed, `Expected the cancelled proxy connection to close`); } finally { - for (const socket of sockets) - socket.destroy(); - - await new Promise(resolve => { - server.close(() => resolve()); - }); + 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>();