fix(core): Fix issues with community node installation (no-changelog) (#5481)

This fixes the following issues:
* After a community node is installed, we were not calling `postProcessLoaders`, which was causing a bunch of unexpected behaviors, and sometimes even crashes. This was only happening in the session where the package was installed. After a crash, the restarted service was working without these issues.
* After a community node is installed, the icon for the nodes and credentials were missing in the UI, as we were creating one icons route per installed package at startup, and this did not handle newly installed packages. restarting the service fixes this issue as well.

Fixes https://community.n8n.io/t/showing-weird-count-on-community-nodes/23035

packages/cli/src/CommunityNodes/packageModel.ts

@@ -1,5 +1,5 @@
-import type { INodeTypeData, INodeTypeNameVersion } from 'n8n-workflow';
 import { LoggerProxy } from 'n8n-workflow';
+import type { PackageDirectoryLoader } from 'n8n-core';
 import * as Db from '@/Db';
 import { InstalledNodes } from '@db/entities/InstalledNodes';
 import { InstalledPackages } from '@db/entities/InstalledPackages';
@@ -12,8 +12,9 @@ export async function findInstalledPackage(packageName: string): Promise<Install
 }
 
 export async function isPackageInstalled(packageName: string): Promise<boolean> {
-	const installedPackage = await findInstalledPackage(packageName);
-	return installedPackage !== null;
+	return Db.collections.InstalledPackages.exist({
+		where: { packageName },
+	});
 }
 
 export async function getAllInstalledPackages(): Promise<InstalledPackages[]> {
@@ -27,13 +28,11 @@ export async function removePackageFromDatabase(
 }
 
 export async function persistInstalledPackageData(
-	installedPackageName: string,
-	installedPackageVersion: string,
-	installedNodes: INodeTypeNameVersion[],
-	loadedNodeTypes: INodeTypeData,
-	authorName?: string,
-	authorEmail?: string,
+	packageLoader: PackageDirectoryLoader,
 ): Promise<InstalledPackages> {
+	const { packageJson, nodeTypes, loadedNodes } = packageLoader;
+	const { name: packageName, version: installedVersion, author } = packageJson;
+
 	let installedPackage: InstalledPackages;
 
 	try {
@@ -41,21 +40,21 @@ export async function persistInstalledPackageData(
 			const promises = [];
 
 			const installedPackagePayload = Object.assign(new InstalledPackages(), {
-				packageName: installedPackageName,
-				installedVersion: installedPackageVersion,
-				authorName,
-				authorEmail,
+				packageName,
+				installedVersion,
+				authorName: author?.name,
+				authorEmail: author?.email,
 			});
 			installedPackage = await transactionManager.save<InstalledPackages>(installedPackagePayload);
 			installedPackage.installedNodes = [];
 
 			promises.push(
-				...installedNodes.map(async (loadedNode) => {
+				...loadedNodes.map(async (loadedNode) => {
 					const installedNodePayload = Object.assign(new InstalledNodes(), {
-						name: loadedNodeTypes[loadedNode.name].type.description.displayName,
+						name: nodeTypes[loadedNode.name].type.description.displayName,
 						type: loadedNode.name,
 						latestVersion: loadedNode.version,
-						package: installedPackageName,
+						package: packageName,
 					});
 					installedPackage.installedNodes.push(installedNodePayload);
 					return transactionManager.save<InstalledNodes>(installedNodePayload);
@@ -70,7 +69,7 @@ export async function persistInstalledPackageData(
 		LoggerProxy.error('Failed to save installed packages and nodes', {
 			// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
 			error,
-			packageName: installedPackageName,
+			packageName,
 		});
 		throw error;
 	}

packages/cli/src/LoadNodesAndCredentials.ts

@@ -22,7 +22,6 @@ import { access as fsAccess, mkdir, readdir as fsReaddir, stat as fsStat } from
 import path from 'path';
 import config from '@/config';
 import type { InstalledPackages } from '@db/entities/InstalledPackages';
-import type { InstalledNodes } from '@db/entities/InstalledNodes';
 import { executeCommand } from '@/CommunityNodes/helpers';
 import {
 	CLI_DIR,
@@ -198,23 +197,13 @@ export class LoadNodesAndCredentialsClass implements INodesAndCredentials {
 
 		const finalNodeUnpackedPath = path.join(downloadFolder, 'node_modules', packageName);
 
-		const { loadedNodes, packageJson } = await this.runDirectoryLoader(
-			PackageDirectoryLoader,
-			finalNodeUnpackedPath,
-		);
+		const loader = await this.runDirectoryLoader(PackageDirectoryLoader, finalNodeUnpackedPath);
 
-		if (loadedNodes.length > 0) {
+		if (loader.loadedNodes.length > 0) {
 			// Save info to DB
 			try {
-				const installedPackage = await persistInstalledPackageData(
-					packageJson.name,
-					packageJson.version,
-					loadedNodes,
-					this.loaded.nodes,
-					packageJson.author?.name,
-					packageJson.author?.email,
-				);
-				this.attachNodesToNodeTypes(installedPackage.installedNodes);
+				const installedPackage = await persistInstalledPackageData(loader);
+				await this.postProcessLoaders();
 				await this.generateTypesForFrontend();
 				return installedPackage;
 			} catch (error) {
@@ -242,9 +231,13 @@ export class LoadNodesAndCredentialsClass implements INodesAndCredentials {
 
 		await removePackageFromDatabase(installedPackage);
 
-		await this.generateTypesForFrontend();
+		if (packageName in this.loaders) {
+			this.loaders[packageName].reset();
+			delete this.loaders[packageName];
+		}
 
-		this.unloadNodes(installedPackage.installedNodes);
+		await this.postProcessLoaders();
+		await this.generateTypesForFrontend();
 	}
 
 	async updateNpmModule(
@@ -264,33 +257,17 @@ export class LoadNodesAndCredentialsClass implements INodesAndCredentials {
 			throw error;
 		}
 
-		this.unloadNodes(installedPackage.installedNodes);
-
 		const finalNodeUnpackedPath = path.join(downloadFolder, 'node_modules', packageName);
 
-		const { loadedNodes, packageJson } = await this.runDirectoryLoader(
-			PackageDirectoryLoader,
-			finalNodeUnpackedPath,
-		);
+		const loader = await this.runDirectoryLoader(PackageDirectoryLoader, finalNodeUnpackedPath);
 
-		if (loadedNodes.length > 0) {
+		if (loader.loadedNodes.length > 0) {
 			// Save info to DB
 			try {
 				await removePackageFromDatabase(installedPackage);
-
-				const newlyInstalledPackage = await persistInstalledPackageData(
-					packageJson.name,
-					packageJson.version,
-					loadedNodes,
-					this.loaded.nodes,
-					packageJson.author?.name,
-					packageJson.author?.email,
-				);
-
-				this.attachNodesToNodeTypes(newlyInstalledPackage.installedNodes);
-
+				const newlyInstalledPackage = await persistInstalledPackageData(loader);
+				await this.postProcessLoaders();
 				await this.generateTypesForFrontend();
-
 				return newlyInstalledPackage;
 			} catch (error) {
 				LoggerProxy.error('Failed to save installed packages and nodes', {
@@ -363,20 +340,6 @@ export class LoadNodesAndCredentialsClass implements INodesAndCredentials {
 		});
 	}
 
-	private unloadNodes(installedNodes: InstalledNodes[]): void {
-		installedNodes.forEach((installedNode) => {
-			delete this.loaded.nodes[installedNode.type];
-		});
-	}
-
-	private attachNodesToNodeTypes(installedNodes: InstalledNodes[]): void {
-		const loadedNodes = this.loaded.nodes;
-		installedNodes.forEach((installedNode) => {
-			const { type, sourcePath } = loadedNodes[installedNode.type];
-			loadedNodes[installedNode.type] = { type, sourcePath };
-		});
-	}
-
 	/**
 	 * Run a loader of source files of nodes and credentials in a directory.
 	 */
@@ -386,16 +349,18 @@ export class LoadNodesAndCredentialsClass implements INodesAndCredentials {
 	) {
 		const loader = new constructor(dir, this.excludeNodes, this.includeNodes);
 		await loader.loadAll();
-		this.loaders[dir] = loader;
+		this.loaders[loader.packageName] = loader;
 		return loader;
 	}
 
 	async postProcessLoaders() {
-		this.types.nodes = [];
-		this.types.credentials = [];
-		for (const [dir, loader] of Object.entries(this.loaders)) {
+		this.known = { nodes: {}, credentials: {} };
+		this.loaded = { nodes: {}, credentials: {} };
+		this.types = { nodes: [], credentials: [] };
+
+		for (const loader of Object.values(this.loaders)) {
 			// list of node & credential types that will be sent to the frontend
-			const { types } = loader;
+			const { types, directory } = loader;
 			this.types.nodes = this.types.nodes.concat(types.nodes);
 			this.types.credentials = this.types.credentials.concat(types.credentials);
 
@@ -416,7 +381,7 @@ export class LoadNodesAndCredentialsClass implements INodesAndCredentials {
 					const { className, sourcePath } = known.nodes[type];
 					this.known.nodes[type] = {
 						className,
-						sourcePath: path.join(dir, sourcePath),
+						sourcePath: path.join(directory, sourcePath),
 					};
 				}
 
@@ -424,7 +389,7 @@ export class LoadNodesAndCredentialsClass implements INodesAndCredentials {
 					const { className, sourcePath, nodesToTestWith } = known.credentials[type];
 					this.known.credentials[type] = {
 						className,
-						sourcePath: path.join(dir, sourcePath),
+						sourcePath: path.join(directory, sourcePath),
 						nodesToTestWith: nodesToTestWith?.map((nodeName) => `${packageName}.${nodeName}`),
 					};
 				}

packages/cli/src/ReloadNodesAndCredentials.ts

@@ -1,5 +1,5 @@
 import path from 'path';
-import { realpath } from 'fs/promises';
+import { realpath, access } from 'fs/promises';
 
 import type { LoadNodesAndCredentialsClass } from '@/LoadNodesAndCredentials';
 import type { NodeTypesClass } from '@/NodeTypes';
@@ -15,8 +15,15 @@ export const reloadNodesAndCredentials = async (
 	// eslint-disable-next-line import/no-extraneous-dependencies
 	const { watch } = await import('chokidar');
 
-	Object.entries(loadNodesAndCredentials.loaders).forEach(async ([dir, loader]) => {
-		const realModulePath = path.join(await realpath(dir), path.sep);
+	Object.values(loadNodesAndCredentials.loaders).forEach(async (loader) => {
+		try {
+			await access(loader.directory);
+		} catch {
+			// If directory doesn't exist, there is nothing to watch
+			return;
+		}
+
+		const realModulePath = path.join(await realpath(loader.directory), path.sep);
 		const reloader = debounce(async () => {
 			const modulesToUnload = Object.keys(require.cache).filter((filePath) =>
 				filePath.startsWith(realModulePath),

packages/cli/src/Server.ts

@@ -1261,18 +1261,23 @@ class Server extends AbstractServer {
 				},
 			};
 
-			for (const [dir, loader] of Object.entries(this.loadNodesAndCredentials.loaders)) {
-				const pathPrefix = `/icons/${loader.packageName}`;
-				this.app.use(`${pathPrefix}/*/*.(svg|png)`, async (req, res) => {
-					const filePath = pathResolve(dir, req.originalUrl.substring(pathPrefix.length + 1));
+			this.app.use('/icons/:packageName/*/*.(svg|png)', async (req, res) => {
+				const { packageName } = req.params;
+				const loader = this.loadNodesAndCredentials.loaders[packageName];
+				if (loader) {
+					const pathPrefix = `/icons/${packageName}/`;
+					const filePath = pathResolve(
+						loader.directory,
+						req.originalUrl.substring(pathPrefix.length),
+					);
 					try {
 						await fsAccess(filePath);
-						res.sendFile(filePath);
-					} catch {
-						res.sendStatus(404);
-					}
-				});
-			}
+						return res.sendFile(filePath);
+					} catch {}
+				}
+
+				res.sendStatus(404);
+			});
 
 			this.app.use(
 				'/',

packages/core/src/DirectoryLoader.ts

@@ -45,7 +45,7 @@ export abstract class DirectoryLoader {
 	types: Types = { nodes: [], credentials: [] };
 
 	constructor(
-		protected readonly directory: string,
+		readonly directory: string,
 		protected readonly excludeNodes: string[] = [],
 		protected readonly includeNodes: string[] = [],
 	) {}