fix(plugin-typescript): add a timeout to the Algolia auto-types lookup

.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"

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<typeof setTimeout> | undefined;
+
   try {
-    const packageInfo = await index.getObject<AlgoliaObj>(stringifiedIdent, {attributesToRetrieve: [`types`]});
+    const packageInfo = await Promise.race([
+      index.getObject<AlgoliaObj>(stringifiedIdent, {attributesToRetrieve: [`types`]}),
+      new Promise<never>((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<Response> {
       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,
         };
       }
     }};

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<void, [AbortSignal]>();
+
+const descriptor = structUtils.makeDescriptor(
+  structUtils.makeIdent(null, `is-number`),
+  `unknown`,
+);
+
+const makeConfiguration = (executeRequest: (signal: AbortSignal) => Promise<httpUtils.Response>) => {
+  const testPlugin: Plugin<Hooks> = {
+    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<string, Plugin>([
+    [`@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`));
+    });
+  });
+});

packages/yarnpkg-core/sources/httpUtils.ts

@@ -168,11 +168,12 @@ export type Options = {
   jsonRequest?: boolean;
   jsonResponse?: boolean;
   method?: Method;
+  signal?: AbortSignal;
   wrapNetworkRequest?: (executor: () => Promise<Response>, extra: WrapNetworkRequestInfo) => Promise<() => Promise<Response>>;
 };
 
-export async function request(target: string | URL, body: Body, {configuration, headers, jsonRequest, jsonResponse, method = Method.GET, wrapNetworkRequest}: Omit<Options, `customErrorMessage`>) {
-  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<Options, `customErrorMessage`>) {
+  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<Options, `customErrorMessage`>): Promise<Response> {
+async function requestImpl(target: string | URL, body: Body, {configuration, headers, jsonRequest, jsonResponse, method = Method.GET, signal}: Omit<Options, `customErrorMessage`>): Promise<Response> {
   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);
+    }
   });
 }