From 161de110cedf2d5d8bc345349d4364ea202e2abd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Fri, 24 Mar 2023 17:04:26 +0100 Subject: [PATCH] fix(core): Improve community nodes loading (#5608) --- packages/cli/src/CredentialsHelper.ts | 1 - packages/cli/src/LoadNodesAndCredentials.ts | 191 ++++++------------ packages/cli/src/commands/start.ts | 5 +- packages/cli/src/constants.ts | 3 +- .../cli/src/controllers/nodes.controller.ts | 7 +- .../cli/test/integration/nodes.api.test.ts | 2 +- packages/nodes-base/.eslintrc.js | 2 + packages/nodes-base/index.js | 0 packages/nodes-base/package.json | 1 + 9 files changed, 74 insertions(+), 138 deletions(-) create mode 100644 packages/nodes-base/index.js diff --git a/packages/cli/src/CredentialsHelper.ts b/packages/cli/src/CredentialsHelper.ts index 9a5a230268305..4fd8295ce1fa4 100644 --- a/packages/cli/src/CredentialsHelper.ts +++ b/packages/cli/src/CredentialsHelper.ts @@ -451,7 +451,6 @@ export class CredentialsHelper extends ICredentialsHelper { type: string, data: ICredentialDataDecryptedObject, ): Promise { - // eslint-disable-next-line @typescript-eslint/await-thenable const credentials = await this.getCredentials(nodeCredentials, type); if (!Db.isInitialized) { diff --git a/packages/cli/src/LoadNodesAndCredentials.ts b/packages/cli/src/LoadNodesAndCredentials.ts index f25cef5f3b915..8048673425728 100644 --- a/packages/cli/src/LoadNodesAndCredentials.ts +++ b/packages/cli/src/LoadNodesAndCredentials.ts @@ -1,4 +1,5 @@ import uniq from 'lodash.uniq'; +import glob from 'fast-glob'; import type { DirectoryLoader, Types } from 'n8n-core'; import { CUSTOM_EXTENSION_ENV, @@ -18,18 +19,18 @@ import type { import { LoggerProxy, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow'; import { createWriteStream } from 'fs'; -import { access as fsAccess, mkdir, readdir as fsReaddir, stat as fsStat } from 'fs/promises'; +import { mkdir } from 'fs/promises'; import path from 'path'; import config from '@/config'; import type { InstalledPackages } from '@db/entities/InstalledPackages'; import { executeCommand } from '@/CommunityNodes/helpers'; import { - CLI_DIR, GENERATED_STATIC_DIR, RESPONSE_ERROR_MESSAGES, CUSTOM_API_CALL_KEY, CUSTOM_API_CALL_NAME, inTest, + CLI_DIR, } from '@/constants'; import { CredentialsOverwrites } from '@/CredentialsOverwrites'; import { Service } from 'typedi'; @@ -52,6 +53,8 @@ export class LoadNodesAndCredentials implements INodesAndCredentials { logger: ILogger; + private downloadFolder: string; + async init() { // Make sure the imported modules can resolve dependencies fine. const delimiter = process.platform === 'win32' ? ';' : ':'; @@ -61,8 +64,13 @@ export class LoadNodesAndCredentials implements INodesAndCredentials { // eslint-disable-next-line @typescript-eslint/no-unsafe-call if (!inTest) module.constructor._initPaths(); - await this.loadNodesFromBasePackages(); - await this.loadNodesFromDownloadedPackages(); + this.downloadFolder = UserSettings.getUserN8nFolderDownloadedNodesPath(); + + // Load nodes from `n8n-nodes-base` and any other `n8n-nodes-*` package in the main `node_modules` + await this.loadNodesFromNodeModules(CLI_DIR); + // Load nodes from installed community packages + await this.loadNodesFromNodeModules(this.downloadFolder); + await this.loadNodesFromCustomDirectories(); await this.postProcessLoaders(); this.injectCustomApiCallOptions(); @@ -109,32 +117,20 @@ export class LoadNodesAndCredentials implements INodesAndCredentials { await writeStaticJSON('credentials', this.types.credentials); } - private async loadNodesFromBasePackages() { - const nodeModulesPath = await this.getNodeModulesPath(); - const nodePackagePaths = await this.getN8nNodePackages(nodeModulesPath); - - for (const packagePath of nodePackagePaths) { - await this.runDirectoryLoader(LazyPackageDirectoryLoader, packagePath); - } - } - - private async loadNodesFromDownloadedPackages(): Promise { - const nodePackages = []; - try { - // Read downloaded nodes and credentials - const downloadedNodesDir = UserSettings.getUserN8nFolderDownloadedNodesPath(); - const downloadedNodesDirModules = path.join(downloadedNodesDir, 'node_modules'); - await fsAccess(downloadedNodesDirModules); - const downloadedPackages = await this.getN8nNodePackages(downloadedNodesDirModules); - nodePackages.push(...downloadedPackages); - } catch (error) { - // Folder does not exist so ignore and return - return; - } + private async loadNodesFromNodeModules(scanDir: string): Promise { + const nodeModulesDir = path.join(scanDir, 'node_modules'); + const globOptions = { cwd: nodeModulesDir, onlyDirectories: true }; + const installedPackagePaths = [ + ...(await glob('n8n-nodes-*', { ...globOptions, deep: 1 })), + ...(await glob('@*/n8n-nodes-*', { ...globOptions, deep: 2 })), + ]; - for (const packagePath of nodePackages) { + for (const packagePath of installedPackagePaths) { try { - await this.runDirectoryLoader(PackageDirectoryLoader, packagePath); + await this.runDirectoryLoader( + LazyPackageDirectoryLoader, + path.join(nodeModulesDir, packagePath), + ); } catch (error) { ErrorReporter.error(error); } @@ -158,49 +154,45 @@ export class LoadNodesAndCredentials implements INodesAndCredentials { } } - /** - * Returns all the names of the packages which could contain n8n nodes - */ - private async getN8nNodePackages(baseModulesPath: string): Promise { - const getN8nNodePackagesRecursive = async (relativePath: string): Promise => { - const results: string[] = []; - const nodeModulesPath = `${baseModulesPath}/${relativePath}`; - const nodeModules = await fsReaddir(nodeModulesPath); - for (const nodeModule of nodeModules) { - const isN8nNodesPackage = nodeModule.indexOf('n8n-nodes-') === 0; - const isNpmScopedPackage = nodeModule.indexOf('@') === 0; - if (!isN8nNodesPackage && !isNpmScopedPackage) { - continue; - } - if (!(await fsStat(nodeModulesPath)).isDirectory()) { - continue; - } - if (isN8nNodesPackage) { - results.push(`${baseModulesPath}/${relativePath}${nodeModule}`); - } - if (isNpmScopedPackage) { - results.push(...(await getN8nNodePackagesRecursive(`${relativePath}${nodeModule}/`))); - } - } - return results; - }; - return getN8nNodePackagesRecursive(''); - } - - async loadNpmModule(packageName: string, version?: string): Promise { - const downloadFolder = UserSettings.getUserN8nFolderDownloadedNodesPath(); - const command = `npm install ${packageName}${version ? `@${version}` : ''}`; + private async installOrUpdateNpmModule( + packageName: string, + options: { version?: string } | { installedPackage: InstalledPackages }, + ) { + const isUpdate = 'installedPackage' in options; + const command = isUpdate + ? `npm update ${packageName}` + : `npm install ${packageName}${options.version ? `@${options.version}` : ''}`; - await executeCommand(command); + try { + await executeCommand(command); + } catch (error) { + if (error instanceof Error && error.message === RESPONSE_ERROR_MESSAGES.PACKAGE_NOT_FOUND) { + throw new Error(`The npm package "${packageName}" could not be found.`); + } + throw error; + } - const finalNodeUnpackedPath = path.join(downloadFolder, 'node_modules', packageName); + const finalNodeUnpackedPath = path.join(this.downloadFolder, 'node_modules', packageName); - const loader = await this.runDirectoryLoader(PackageDirectoryLoader, finalNodeUnpackedPath); + let loader: PackageDirectoryLoader; + try { + loader = await this.runDirectoryLoader(PackageDirectoryLoader, finalNodeUnpackedPath); + } catch (error) { + // Remove this package since loading it failed + const removeCommand = `npm remove ${packageName}`; + try { + await executeCommand(removeCommand); + } catch {} + throw new Error(RESPONSE_ERROR_MESSAGES.PACKAGE_LOADING_FAILED, { cause: error }); + } if (loader.loadedNodes.length > 0) { // Save info to DB try { - const { persistInstalledPackageData } = await import('@/CommunityNodes/packageModel'); + const { persistInstalledPackageData, removePackageFromDatabase } = await import( + '@/CommunityNodes/packageModel' + ); + if (isUpdate) await removePackageFromDatabase(options.installedPackage); const installedPackage = await persistInstalledPackageData(loader); await this.postProcessLoaders(); await this.generateTypesForFrontend(); @@ -223,6 +215,10 @@ export class LoadNodesAndCredentials implements INodesAndCredentials { } } + async installNpmModule(packageName: string, version?: string): Promise { + return this.installOrUpdateNpmModule(packageName, { version }); + } + async removeNpmModule(packageName: string, installedPackage: InstalledPackages): Promise { const command = `npm remove ${packageName}`; @@ -244,49 +240,7 @@ export class LoadNodesAndCredentials implements INodesAndCredentials { packageName: string, installedPackage: InstalledPackages, ): Promise { - const downloadFolder = UserSettings.getUserN8nFolderDownloadedNodesPath(); - - const command = `npm i ${packageName}@latest`; - - try { - await executeCommand(command); - } catch (error) { - if (error instanceof Error && error.message === RESPONSE_ERROR_MESSAGES.PACKAGE_NOT_FOUND) { - throw new Error(`The npm package "${packageName}" could not be found.`); - } - throw error; - } - - const finalNodeUnpackedPath = path.join(downloadFolder, 'node_modules', packageName); - - const loader = await this.runDirectoryLoader(PackageDirectoryLoader, finalNodeUnpackedPath); - - if (loader.loadedNodes.length > 0) { - // Save info to DB - try { - const { persistInstalledPackageData, removePackageFromDatabase } = await import( - '@/CommunityNodes/packageModel' - ); - await removePackageFromDatabase(installedPackage); - const newlyInstalledPackage = await persistInstalledPackageData(loader); - await this.postProcessLoaders(); - await this.generateTypesForFrontend(); - return newlyInstalledPackage; - } catch (error) { - LoggerProxy.error('Failed to save installed packages and nodes', { - error: error as Error, - packageName, - }); - throw error; - } - } else { - // Remove this package since it contains no loadable nodes - const removeCommand = `npm remove ${packageName}`; - try { - await executeCommand(removeCommand); - } catch {} - throw new Error(RESPONSE_ERROR_MESSAGES.PACKAGE_DOES_NOT_CONTAIN_NODES); - } + return this.installOrUpdateNpmModule(packageName, { installedPackage }); } /** @@ -399,27 +353,4 @@ export class LoadNodesAndCredentials implements INodesAndCredentials { } } } - - private async getNodeModulesPath(): Promise { - // Get the path to the node-modules folder to be later able - // to load the credentials and nodes - const checkPaths = [ - // In case "n8n" package is in same node_modules folder. - path.join(CLI_DIR, '..', 'n8n-workflow'), - // In case "n8n" package is the root and the packages are - // in the "node_modules" folder underneath it. - path.join(CLI_DIR, 'node_modules', 'n8n-workflow'), - // In case "n8n" package is installed using npm/yarn workspaces - // the node_modules folder is in the root of the workspace. - path.join(CLI_DIR, '..', '..', 'node_modules', 'n8n-workflow'), - ]; - for (const checkPath of checkPaths) { - try { - await fsAccess(checkPath); - // Folder exists, so use it. - return path.dirname(checkPath); - } catch {} // Folder does not exist so get next one - } - throw new Error('Could not find "node_modules" folder!'); - } } diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index 6abd162ad8ab8..991d19e9d4401 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -267,11 +267,10 @@ export class Start extends BaseCommand { // Optimistic approach - stop if any installation fails // eslint-disable-next-line no-restricted-syntax for (const missingPackage of missingPackages) { - // eslint-disable-next-line no-await-in-loop - void (await this.loadNodesAndCredentials.loadNpmModule( + await this.loadNodesAndCredentials.installNpmModule( missingPackage.packageName, missingPackage.version, - )); + ); missingPackages.delete(missingPackage); } LoggerProxy.info('Packages reinstalled successfully. Resuming regular initialization.'); diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts index 2a984afb54cba..82f87dc13c048 100644 --- a/packages/cli/src/constants.ts +++ b/packages/cli/src/constants.ts @@ -18,7 +18,7 @@ export const CUSTOM_API_CALL_KEY = '__CUSTOM_API_CALL__'; export const CLI_DIR = resolve(__dirname, '..'); export const TEMPLATES_DIR = join(CLI_DIR, 'templates'); -export const NODES_BASE_DIR = join(CLI_DIR, '..', 'nodes-base'); +export const NODES_BASE_DIR = dirname(require.resolve('n8n-nodes-base')); export const GENERATED_STATIC_DIR = join(UserSettings.getUserHome(), '.cache/n8n/public'); export const EDITOR_UI_DIST_DIR = join(dirname(require.resolve('n8n-editor-ui')), 'dist'); @@ -45,6 +45,7 @@ export const RESPONSE_ERROR_MESSAGES = { PACKAGE_NOT_FOUND: 'Package not found in npm', PACKAGE_VERSION_NOT_FOUND: 'The specified package version was not found', PACKAGE_DOES_NOT_CONTAIN_NODES: 'The specified package does not contain any nodes', + PACKAGE_LOADING_FAILED: 'The specified package could not be loaded', DISK_IS_FULL: 'There appears to be insufficient disk space', }; diff --git a/packages/cli/src/controllers/nodes.controller.ts b/packages/cli/src/controllers/nodes.controller.ts index 63116bb761f2b..e82bd3e5895b8 100644 --- a/packages/cli/src/controllers/nodes.controller.ts +++ b/packages/cli/src/controllers/nodes.controller.ts @@ -109,7 +109,7 @@ export class NodesController { let installedPackage: InstalledPackages; try { - installedPackage = await this.loadNodesAndCredentials.loadNpmModule( + installedPackage = await this.loadNodesAndCredentials.installNpmModule( parsed.packageName, parsed.version, ); @@ -125,7 +125,10 @@ export class NodesController { failure_reason: errorMessage, }); - const message = [`Error loading package "${name}"`, errorMessage].join(':'); + let message = [`Error loading package "${name}" `, errorMessage].join(':'); + if (error instanceof Error && error.cause instanceof Error) { + message += `\nCause: ${error.cause.message}`; + } const clientError = error instanceof Error ? isClientError(error) : false; throw new (clientError ? BadRequestError : InternalServerError)(message); diff --git a/packages/cli/test/integration/nodes.api.test.ts b/packages/cli/test/integration/nodes.api.test.ts index 37c619d13e9d6..fb6166bd9fb1b 100644 --- a/packages/cli/test/integration/nodes.api.test.ts +++ b/packages/cli/test/integration/nodes.api.test.ts @@ -191,7 +191,7 @@ describe('POST /nodes', () => { mocked(hasPackageLoaded).mockReturnValueOnce(false); mocked(checkNpmPackageStatus).mockResolvedValueOnce({ status: 'OK' }); - mockLoadNodesAndCredentials.loadNpmModule.mockImplementationOnce(mockedEmptyPackage); + mockLoadNodesAndCredentials.installNpmModule.mockImplementationOnce(mockedEmptyPackage); const { statusCode } = await authOwnerShellAgent.post('/nodes').send({ name: utils.installedPackagePayload().packageName, diff --git a/packages/nodes-base/.eslintrc.js b/packages/nodes-base/.eslintrc.js index f517dd8c4823f..a5d9b31cce7a0 100644 --- a/packages/nodes-base/.eslintrc.js +++ b/packages/nodes-base/.eslintrc.js @@ -8,6 +8,8 @@ module.exports = { ...sharedOptions(__dirname), + ignorePatterns: ['index.js'], + rules: { '@typescript-eslint/consistent-type-imports': 'error', diff --git a/packages/nodes-base/index.js b/packages/nodes-base/index.js new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/packages/nodes-base/package.json b/packages/nodes-base/package.json index 2ca45b4b12765..90d3af4d17f5c 100644 --- a/packages/nodes-base/package.json +++ b/packages/nodes-base/package.json @@ -8,6 +8,7 @@ "name": "Jan Oberhauser", "email": "jan@n8n.io" }, + "main": "index.js", "repository": { "type": "git", "url": "git+https://github.com/n8n-io/n8n.git"