refactor: Flatten update lookup tasks (#27369)

Co-authored-by: Rhys Arkins <[email protected]>

lib/util/promises.ts

@@ -10,7 +10,7 @@ function isExternalHostError(err: any): err is ExternalHostError {
   return err instanceof ExternalHostError;
 }
 
-function handleMultipleErrors(errors: Error[]): never {
+export function handleMultipleErrors(errors: Error[]): never {
   const hostError = errors.find(isExternalHostError);
   if (hostError) {
     throw hostError;

lib/workers/repository/process/__snapshots__/fetch.spec.ts.snap

@@ -44,7 +44,6 @@ exports[`workers/repository/process/fetch fetchUpdates() handles ignored, skippe
         },
         {
           "depName": "skipped",
-          "packageName": "skipped",
           "skipReason": "some-reason",
           "updates": [],
         },

lib/workers/repository/process/fetch.spec.ts

@@ -4,6 +4,7 @@ import { getConfig } from '../../../config/defaults';
 import { MavenDatasource } from '../../../modules/datasource/maven';
 import type { PackageFile } from '../../../modules/manager/types';
 import { ExternalHostError } from '../../../types/errors/external-host-error';
+import { Result } from '../../../util/result';
 import { fetchUpdates } from './fetch';
 import * as lookup from './lookup';
 
@@ -156,7 +157,7 @@ describe('workers/repository/process/fetch', () => {
           },
         ],
       };
-      lookupUpdates.mockRejectedValueOnce(new Error('some error'));
+      lookupUpdates.mockResolvedValueOnce(Result.err(new Error('some error')));
 
       await expect(
         fetchUpdates({ ...config, repoIsOnboarded: true }, packageFiles),
@@ -173,7 +174,7 @@ describe('workers/repository/process/fetch', () => {
           },
         ],
       };
-      lookupUpdates.mockRejectedValueOnce(new Error('some error'));
+      lookupUpdates.mockResolvedValueOnce(Result.err(new Error('some error')));
 
       await expect(
         fetchUpdates({ ...config, repoIsOnboarded: true }, packageFiles),

lib/workers/repository/process/fetch.ts

@@ -1,7 +1,7 @@
 // TODO #22198
 import is from '@sindresorhus/is';
 import { getManagerConfig, mergeChildConfig } from '../../../config';
-import type { RenovateConfig } from '../../../config/types';
+import type { ManagerConfig, RenovateConfig } from '../../../config/types';
 import { logger } from '../../../logger';
 import { getDefaultConfig } from '../../../modules/datasource';
 import { getDefaultVersioning } from '../../../modules/datasource/common';
@@ -17,27 +17,44 @@ import { Result } from '../../../util/result';
 import { LookupStats } from '../../../util/stats';
 import { PackageFiles } from '../package-files';
 import { lookupUpdates } from './lookup';
-import type { LookupUpdateConfig } from './lookup/types';
+import type { LookupUpdateConfig, UpdateResult } from './lookup/types';
 
-async function fetchDepUpdates(
-  packageFileConfig: RenovateConfig & PackageFile,
+type LookupResult = Result<PackageDependency, Error>;
+
+interface LookupTaskResult {
+  packageFileName: string;
+  manager: string;
+  result: LookupResult;
+}
+
+type LookupTask = Promise<LookupTaskResult>;
+
+async function lookup(
+  packageFileConfig: ManagerConfig & PackageFile,
   indep: PackageDependency,
-): Promise<Result<PackageDependency, Error>> {
+): Promise<LookupResult> {
   const dep = clone(indep);
   dep.updates = [];
+
+  if (dep.skipReason) {
+    return Result.ok(dep);
+  }
+
   if (is.string(dep.depName)) {
     dep.depName = dep.depName.trim();
   }
+
   dep.packageName ??= dep.depName;
   if (!is.nonEmptyString(dep.packageName)) {
     dep.skipReason = 'invalid-name';
+    return Result.ok(dep);
   }
+
   if (dep.isInternal && !packageFileConfig.updateInternalDeps) {
     dep.skipReason = 'internal-package';
-  }
-  if (dep.skipReason) {
     return Result.ok(dep);
   }
+
   const { depName } = dep;
   // TODO: fix types
   let depConfig = mergeChildConfig(packageFileConfig, dep);
@@ -46,24 +63,33 @@ async function fetchDepUpdates(
   depConfig.versioning ??= getDefaultVersioning(depConfig.datasource);
   depConfig = applyPackageRules(depConfig, 'pre-lookup');
   depConfig.packageName ??= depConfig.depName;
+
   if (depConfig.ignoreDeps!.includes(depName!)) {
     // TODO: fix types (#22198)
     logger.debug(`Dependency: ${depName!}, is ignored`);
     dep.skipReason = 'ignored';
-  } else if (depConfig.enabled === false) {
+    return Result.ok(dep);
+  }
+
+  if (depConfig.enabled === false) {
     logger.debug(`Dependency: ${depName!}, is disabled`);
     dep.skipReason = 'disabled';
-  } else {
-    if (depConfig.datasource) {
-      const { val: updateResult, err } = await LookupStats.wrap(
-        depConfig.datasource,
-        () =>
-          Result.wrap(lookupUpdates(depConfig as LookupUpdateConfig)).unwrap(),
-      );
-
-      if (updateResult) {
-        Object.assign(dep, updateResult);
-      } else {
+    return Result.ok(dep);
+  }
+
+  if (!depConfig.datasource) {
+    return Result.ok(dep);
+  }
+
+  return LookupStats.wrap(depConfig.datasource, async () => {
+    return await Result.wrap(lookupUpdates(depConfig as LookupUpdateConfig))
+      .onValue((dep) => {
+        logger.trace({ dep }, 'Dependency lookup success');
+      })
+      .onError((err) => {
+        logger.trace({ err, depName }, 'Dependency lookup error');
+      })
+      .catch((err): Result<UpdateResult, Error> => {
         if (
           packageFileConfig.repoIsOnboarded === true ||
           !(err instanceof ExternalHostError)
@@ -72,76 +98,101 @@ async function fetchDepUpdates(
         }
 
         const cause = err.err;
-        dep.warnings ??= [];
-        dep.warnings.push({
-          topic: 'Lookup Error',
-          // TODO: types (#22198)
-          message: `${depName!}: ${cause.message}`,
+        return Result.ok({
+          updates: [],
+          warnings: [
+            {
+              topic: 'Lookup Error',
+              message: `${depName}: ${cause.message}`,
+            },
+          ],
         });
-      }
-    }
-    dep.updates ??= [];
-  }
-  return Result.ok(dep);
+      })
+      .transform((upd): PackageDependency => Object.assign(dep, upd));
+  });
 }
 
-async function fetchManagerPackagerFileUpdates(
+function createLookupTasks(
   config: RenovateConfig,
-  managerConfig: RenovateConfig,
-  pFile: PackageFile,
-): Promise<void> {
-  const { packageFile } = pFile;
-  const packageFileConfig = mergeChildConfig(managerConfig, pFile);
-  if (pFile.extractedConstraints) {
-    packageFileConfig.constraints = {
-      ...pFile.extractedConstraints,
-      ...config.constraints,
-    };
-  }
-  const { manager } = packageFileConfig;
-  const queue = pFile.deps.map(
-    (dep) => async (): Promise<PackageDependency> => {
-      const updates = await fetchDepUpdates(packageFileConfig, dep);
-      return updates.unwrapOrThrow();
-    },
-  );
-  logger.trace(
-    { manager, packageFile, queueLength: queue.length },
-    'fetchManagerPackagerFileUpdates starting with concurrency',
-  );
+  managerPackageFiles: Record<string, PackageFile[]>,
+): LookupTask[] {
+  const lookupTasks: LookupTask[] = [];
 
-  pFile.deps = await p.all(queue);
-  logger.trace({ packageFile }, 'fetchManagerPackagerFileUpdates finished');
-}
+  for (const [manager, packageFiles] of Object.entries(managerPackageFiles)) {
+    const managerConfig = getManagerConfig(config, manager);
 
-async function fetchManagerUpdates(
-  config: RenovateConfig,
-  packageFiles: Record<string, PackageFile[]>,
-  manager: string,
-): Promise<void> {
-  const managerConfig = getManagerConfig(config, manager);
-  const queue = packageFiles[manager].map(
-    (pFile) => (): Promise<void> =>
-      fetchManagerPackagerFileUpdates(config, managerConfig, pFile),
-  );
-  logger.trace(
-    { manager, queueLength: queue.length },
-    'fetchManagerUpdates starting',
-  );
-  await p.all(queue);
-  logger.trace({ manager }, 'fetchManagerUpdates finished');
+    for (const packageFile of packageFiles) {
+      const packageFileConfig = mergeChildConfig(managerConfig, packageFile);
+      if (packageFile.extractedConstraints) {
+        packageFileConfig.constraints = {
+          ...packageFile.extractedConstraints,
+          ...config.constraints,
+        };
+      }
+
+      for (const dep of packageFile.deps) {
+        lookupTasks.push(
+          lookup(packageFileConfig, dep).then((result) => ({
+            packageFileName: packageFile.packageFile,
+            manager: managerConfig.manager,
+            result,
+          })),
+        );
+      }
+    }
+  }
+
+  return lookupTasks;
 }
 
 export async function fetchUpdates(
   config: RenovateConfig,
-  packageFiles: Record<string, PackageFile[]>,
+  managerPackageFiles: Record<string, PackageFile[]>,
 ): Promise<void> {
-  const managers = Object.keys(packageFiles);
-  const allManagerJobs = managers.map((manager) =>
-    fetchManagerUpdates(config, packageFiles, manager),
+  logger.debug(
+    { baseBranch: config.baseBranch },
+    'Starting package releases lookups',
   );
-  await Promise.all(allManagerJobs);
-  PackageFiles.add(config.baseBranch!, { ...packageFiles });
+
+  const allTasks = createLookupTasks(config, managerPackageFiles);
+
+  const fetchResults = await Promise.all(allTasks);
+
+  const errors: Error[] = [];
+
+  type Manager = string;
+  type PackageFileName = string;
+  type PackageFileDeps = Record<PackageFileName, PackageDependency[]>;
+  type ManagerPackageFileDeps = Record<Manager, PackageFileDeps>;
+  const deps: ManagerPackageFileDeps = {};
+
+  // Separate good results from errors
+  for (const { packageFileName, manager, result } of fetchResults) {
+    const { val: dep, err } = result.unwrap();
+    if (dep) {
+      deps[manager] ??= {};
+      deps[manager][packageFileName] ??= [];
+      deps[manager][packageFileName].push(dep);
+    } else {
+      errors.push(err);
+    }
+  }
+
+  if (errors.length) {
+    p.handleMultipleErrors(errors);
+  }
+
+  // Assign fetched deps back to packageFiles
+  for (const [manager, packageFiles] of Object.entries(managerPackageFiles)) {
+    for (const packageFile of packageFiles) {
+      const packageFileDeps = deps[manager]?.[packageFile.packageFile];
+      if (packageFileDeps) {
+        packageFile.deps = packageFileDeps;
+      }
+    }
+  }
+
+  PackageFiles.add(config.baseBranch!, { ...managerPackageFiles });
   logger.debug(
     { baseBranch: config.baseBranch },
     'Package releases lookups complete',