fix: revert lookup refactor from #32930 (#33184)

renovatebot · Dec 18, 2024 · 4daa853 · 4daa853
1 parent 3ee4857
commit 4daa853
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 88 deletions.
diff --git a/lib/util/promises.ts b/lib/util/promises.ts
@@ -10,7 +10,7 @@ function isExternalHostError(err: any): err is ExternalHostError {
   return err instanceof ExternalHostError;
 }
 
-export function handleMultipleErrors(errors: Error[]): never {
+function handleMultipleErrors(errors: Error[]): never {
   const hostError = errors.find(isExternalHostError);
   if (hostError) {
     throw hostError;

diff --git a/lib/workers/repository/process/fetch.spec.ts b/lib/workers/repository/process/fetch.spec.ts
@@ -4,7 +4,6 @@ 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';
 
@@ -59,21 +58,18 @@ describe('workers/repository/process/fetch', () => {
                 depName: 'abcd',
                 packageName: 'abcd',
                 skipReason: 'ignored',
-                skipStage: 'lookup',
                 updates: [],
               },
               {
                 depName: 'foo',
                 packageName: 'foo',
                 skipReason: 'disabled',
-                skipStage: 'lookup',
                 updates: [],
               },
               {
                 depName: 'skipped',
                 packageName: 'skipped',
                 skipReason: 'some-reason',
-                skipStage: 'lookup',
                 updates: [],
               },
             ],
@@ -201,7 +197,7 @@ describe('workers/repository/process/fetch', () => {
           },
         ],
       };
-      lookupUpdates.mockResolvedValueOnce(Result.err(new Error('some error')));
+      lookupUpdates.mockRejectedValueOnce(new Error('some error'));
 
       await expect(
         fetchUpdates({ ...config, repoIsOnboarded: true }, packageFiles),
@@ -218,7 +214,7 @@ describe('workers/repository/process/fetch', () => {
           },
         ],
       };
-      lookupUpdates.mockResolvedValueOnce(Result.err(new Error('some error')));
+      lookupUpdates.mockRejectedValueOnce(new Error('some error'));
 
       await expect(
         fetchUpdates({ ...config, repoIsOnboarded: true }, packageFiles),

diff --git a/lib/workers/repository/process/fetch.ts b/lib/workers/repository/process/fetch.ts
@@ -21,13 +21,6 @@ import type { LookupUpdateConfig, UpdateResult } from './lookup/types';
 
 type LookupResult = Result<PackageDependency, Error>;
 
-interface LookupTaskResult {
-  packageFile: PackageFile;
-  result: LookupResult;
-}
-
-type LookupTask = Promise<LookupTaskResult>;
-
 async function lookup(
   packageFileConfig: RenovateConfig & PackageFile,
   indep: PackageDependency,
@@ -113,89 +106,63 @@ async function lookup(
   });
 }
 
-function createLookupTasks(
+async function fetchManagerPackagerFileUpdates(
   config: RenovateConfig,
-  managerPackageFiles: Record<string, PackageFile[]>,
-): LookupTask[] {
-  const lookupTasks: LookupTask[] = [];
-
-  for (const [manager, packageFiles] of Object.entries(managerPackageFiles)) {
-    const managerConfig = getManagerConfig(config, manager);
-
-    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) => ({
-            packageFile,
-            result,
-          })),
-        );
-      }
-    }
+  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 lookup(packageFileConfig, dep);
+      return updates.unwrapOrThrow();
+    },
+  );
+  logger.trace(
+    { manager, packageFile, queueLength: queue.length },
+    'fetchManagerPackagerFileUpdates starting with concurrency',
+  );
 
-  return lookupTasks;
+  pFile.deps = await p.all(queue);
+  logger.trace({ packageFile }, 'fetchManagerPackagerFileUpdates finished');
 }
 
-export async function fetchUpdates(
+async function fetchManagerUpdates(
   config: RenovateConfig,
-  managerPackageFiles: Record<string, PackageFile[]>,
+  packageFiles: Record<string, PackageFile[]>,
+  manager: string,
 ): Promise<void> {
-  logger.debug(
-    { baseBranch: config.baseBranch },
-    'Starting package releases lookups',
+  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');
+}
 
-  const allTasks = createLookupTasks(config, managerPackageFiles);
-
-  const fetchResults = await Promise.all(allTasks);
-
-  const errors: Error[] = [];
-
-  type PackageDeps = WeakMap<PackageFile, PackageDependency[]>;
-  const packageDeps: PackageDeps = new WeakMap();
-
-  // Separate good results from errors
-  for (const { packageFile, result } of fetchResults) {
-    const { val: dep, err } = result.unwrap();
-    if (dep) {
-      let deps = packageDeps.get(packageFile);
-      if (!deps) {
-        deps = [];
-        packageDeps.set(packageFile, deps);
-      }
-      if (dep.skipReason && !dep.skipStage) {
-        dep.skipStage = 'lookup';
-      }
-      deps.push(dep);
-    } else {
-      errors.push(err);
-    }
-  }
-
-  if (errors.length) {
-    p.handleMultipleErrors(errors);
-  }
-
-  // Assign fetched deps back to packageFiles
-  for (const packageFiles of Object.values(managerPackageFiles)) {
-    for (const packageFile of packageFiles) {
-      const packageFileDeps = packageDeps.get(packageFile);
-      if (packageFileDeps) {
-        packageFile.deps = packageFileDeps;
-      }
-    }
-  }
-
-  PackageFiles.add(config.baseBranch!, { ...managerPackageFiles });
+export async function fetchUpdates(
+  config: RenovateConfig,
+  packageFiles: Record<string, PackageFile[]>,
+): Promise<void> {
+  const managers = Object.keys(packageFiles);
+  const allManagerJobs = managers.map((manager) =>
+    fetchManagerUpdates(config, packageFiles, manager),
+  );
+  await Promise.all(allManagerJobs);
+  PackageFiles.add(config.baseBranch!, { ...packageFiles });
   logger.debug(
     { baseBranch: config.baseBranch },
     'Package releases lookups complete',