From 302718f38e54c6c7590f488d749fb4da8362f36f Mon Sep 17 00:00:00 2001
From: Michael Kriese <michael.kriese@visualon.de>
Date: Wed, 21 Aug 2024 13:36:42 +0200
Subject: [PATCH] fix: Revert "refactor: Flatten update lookup tasks" (#30933)
---
lib/util/promises.ts | 2 +-
.../process/__snapshots__/fetch.spec.ts.snap | 1 +
lib/workers/repository/process/fetch.spec.ts | 5 +-
lib/workers/repository/process/fetch.ts | 205 +++++++-----------
4 files changed, 81 insertions(+), 132 deletions(-)
diff --git a/lib/util/promises.ts b/lib/util/promises.ts
index 1b614aa036..736437bc1d 100644
--- 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/__snapshots__/fetch.spec.ts.snap b/lib/workers/repository/process/__snapshots__/fetch.spec.ts.snap
index 115dc22d89..8352aa00ba 100644
--- a/lib/workers/repository/process/__snapshots__/fetch.spec.ts.snap
+++ b/lib/workers/repository/process/__snapshots__/fetch.spec.ts.snap
@@ -44,6 +44,7 @@ exports[`workers/repository/process/fetch fetchUpdates() handles ignored, skippe
},
{
"depName": "skipped",
+ "packageName": "skipped",
"skipReason": "some-reason",
"updates": [],
},
diff --git a/lib/workers/repository/process/fetch.spec.ts b/lib/workers/repository/process/fetch.spec.ts
index c674ccd210..172a6cb456 100644
--- 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';
@@ -157,7 +156,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),
@@ -174,7 +173,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
index 0d9094fcef..025aea391b 100644
--- a/lib/workers/repository/process/fetch.ts
+++ b/lib/workers/repository/process/fetch.ts
@@ -1,7 +1,7 @@
// TODO #22198
import is from '@sindresorhus/is';
import { getManagerConfig, mergeChildConfig } from '../../../config';
-import type { ManagerConfig, RenovateConfig } from '../../../config/types';
+import type { RenovateConfig } from '../../../config/types';
import { logger } from '../../../logger';
import { getDefaultConfig } from '../../../modules/datasource';
import { getDefaultVersioning } from '../../../modules/datasource/common';
@@ -17,44 +17,27 @@ import { Result } from '../../../util/result';
import { LookupStats } from '../../../util/stats';
import { PackageFiles } from '../package-files';
import { lookupUpdates } from './lookup';
-import type { LookupUpdateConfig, UpdateResult } from './lookup/types';
+import type { LookupUpdateConfig } from './lookup/types';
-type LookupResult = Result<PackageDependency, Error>;
-
-interface LookupTaskResult {
- packageFileName: string;
- manager: string;
- result: LookupResult;
-}
-
-type LookupTask = Promise<LookupTaskResult>;
-
-async function lookup(
- packageFileConfig: ManagerConfig & PackageFile,
+async function fetchDepUpdates(
+ packageFileConfig: RenovateConfig & PackageFile,
indep: PackageDependency,
-): Promise<LookupResult> {
+): Promise<Result<PackageDependency, Error>> {
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);
@@ -63,33 +46,24 @@ async function lookup(
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';
- return Result.ok(dep);
- }
-
- if (depConfig.enabled === false) {
+ } else if (depConfig.enabled === false) {
logger.debug(`Dependency: ${depName!}, is disabled`);
dep.skipReason = 'disabled';
- 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> => {
+ } 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 {
if (
packageFileConfig.repoIsOnboarded === true ||
!(err instanceof ExternalHostError)
@@ -98,101 +72,76 @@ async function lookup(
}
const cause = err.err;
- return Result.ok({
- updates: [],
- warnings: [
- {
- topic: 'Lookup Error',
- message: `${depName}: ${cause.message}`,
- },
- ],
+ dep.warnings ??= [];
+ dep.warnings.push({
+ topic: 'Lookup Error',
+ // TODO: types (#22198)
+ message: `${depName!}: ${cause.message}`,
});
- })
- .transform((upd): PackageDependency => Object.assign(dep, upd));
- });
-}
-
-function createLookupTasks(
- 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) => ({
- packageFileName: packageFile.packageFile,
- manager: managerConfig.manager,
- result,
- })),
- );
}
}
+ dep.updates ??= [];
}
-
- return lookupTasks;
+ return Result.ok(dep);
}
-export async function fetchUpdates(
+async function fetchManagerPackagerFileUpdates(
config: RenovateConfig,
- managerPackageFiles: Record<string, PackageFile[]>,
+ managerConfig: RenovateConfig,
+ pFile: PackageFile,
): Promise<void> {
- logger.debug(
- { baseBranch: config.baseBranch },
- 'Starting package releases lookups',
- );
-
- 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);
- }
+ 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',
+ );
- if (errors.length) {
- p.handleMultipleErrors(errors);
- }
+ pFile.deps = await p.all(queue);
+ logger.trace({ packageFile }, 'fetchManagerPackagerFileUpdates finished');
+}
- // 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;
- }
- }
- }
+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');
+}
- 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',
--
GitLab