From 5a3fc3997284b3fdd7a9285b52f4a16230bd3b44 Mon Sep 17 00:00:00 2001 From: "n8n-cat-bot[bot]" <283985454+n8n-cat-bot[bot]@users.noreply.github.com> Date: Fri, 16 Oct 2026 11:53:13 +0000 Subject: [PATCH] fix: Skip dependency files and continue after custom node load (#41117) Co-authored-by: n8n-cat-bot[bot] --- ...stom-directory-loader.dependencies.test.ts | 368 ++++++++++++++++++ ...custom-directory-loader.hot-reload.test.ts | 9 + .../__tests__/directory-loader.test.ts | 3 +- .../nodes-loader/custom-directory-loader.ts | 61 ++- .../core/src/nodes-loader/directory-loader.ts | 10 +- 5 files changed, 441 insertions(+), 10 deletions(-) create mode 100644 packages/core/src/nodes-loader/__tests__/custom-directory-loader.dependencies.test.ts diff --git a/packages/core/src/nodes-loader/__tests__/custom-directory-loader.dependencies.test.ts b/packages/core/src/nodes-loader/__tests__/custom-directory-loader.dependencies.test.ts new file mode 100644 index 00000000000..70886a1ce1b --- /dev/null +++ b/packages/core/src/nodes-loader/__tests__/custom-directory-loader.dependencies.test.ts @@ -0,0 +1,368 @@ +import { Logger } from '@n8n/backend-common'; +import { + mkdirSync, + mkdtempSync, + realpathSync, + rmSync, + symlinkSync, + unlinkSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; + +import { mockInstance } from '@test/utils'; + +import { CustomDirectoryLoader } from '../custom-directory-loader'; + +// NODE-6005: Dependency build files can use n8n node and credential file suffixes. +describe('CustomDirectoryLoader dependencies', () => { + const logger = mockInstance(Logger); + + let tmpRoot: string; + let customDir: string; + let loader: CustomDirectoryLoader; + + const writeFile = (root: string, relativePath: string, source: string) => { + const filePath = path.join(root, relativePath); + mkdirSync(path.dirname(filePath), { recursive: true }); + writeFileSync(filePath, source); + return filePath; + }; + + const writeCustomFiles = (root: string) => { + const nodePath = writeFile( + root, + 'nodes/Foo.node.js', + `module.exports.Foo = class Foo { + constructor() { + this.description = { + displayName: 'Foo', + name: 'foo', + group: ['transform'], + version: 1, + description: 'Test node', + defaults: { name: 'Foo' }, + inputs: ['main'], + outputs: ['main'], + credentials: [{ name: 'fooApi', required: true }], + properties: [], + }; + } + };`, + ); + const credentialPath = writeFile( + root, + 'credentials/FooApi.credentials.js', + `module.exports.FooApi = class FooApi { + constructor() { + this.name = 'fooApi'; + this.displayName = 'Foo API'; + this.properties = []; + } + };`, + ); + return { nodePath, credentialPath }; + }; + + const writeDependency = (root: string, suffix: string) => { + writeFile(root, 'node_modules/wmf/package.json', JSON.stringify({ name: 'wmf' })); + return writeFile(root, `node_modules/wmf/dist/wmf.${suffix}.js`, 'module.exports = {};'); + }; + + const expectCustomFilesLoaded = (files: { nodePath: string; credentialPath: string }) => { + expect(loader.packageName).toBe('CUSTOM'); + expect(Object.keys(loader.nodeTypes)).toEqual(['foo']); + expect(Object.keys(loader.credentialTypes)).toEqual(['fooApi']); + expect(loader.getNode('foo').sourcePath).toBe(files.nodePath); + expect(loader.getCredential('fooApi').sourcePath).toBe(files.credentialPath); + }; + + beforeEach(() => { + vi.clearAllMocks(); + tmpRoot = realpathSync(mkdtempSync(path.join(tmpdir(), 'n8n-custom-dependencies-'))); + customDir = path.join(tmpRoot, 'custom'); + mkdirSync(customDir); + loader = new CustomDirectoryLoader(customDir); + }); + + afterEach(() => { + loader.reset(); + vi.restoreAllMocks(); + rmSync(tmpRoot, { recursive: true, force: true }); + }); + + it('loads loose custom nodes and credentials without dependencies', async () => { + const files = writeCustomFiles(customDir); + + await loader.loadAll(); + + expectCustomFilesLoaded(files); + }); + + describe.each(['loose', 'package'])('%s custom files', (layout) => { + it.each(['single-version', 'versioned'])( + 'skips a %s node with an invalid description without registering it', + async (kind) => { + const root = + layout === 'loose' ? customDir : path.join(customDir, 'node_modules/n8n-nodes-foo'); + const files = writeCustomFiles(root); + const brokenPath = writeFile( + root, + 'AaaBroken.node.js', + `module.exports.AaaBroken = class AaaBroken { + constructor() { + this.description = { + displayName: 'Broken', + name: 'broken', + group: ['transform'], + version: 1, + description: 'Test node', + defaults: { name: 'Broken' }, + inputs: ['main'], + outputs: ['main'], + credentials: [{ name: 'brokenApi', required: true }], + properties: [{ + displayName: 'Operation', + name: 'operation', + type: 'options', + default: 'test', + options: [{ name: 'Test', value: 'test' }], + builderHint: { searchHint: 'Test hint' }, + }], + }; + if (${kind === 'versioned'}) { + this.currentVersion = 2; + this.nodeVersions = { + 1: { description: this.description }, + 2: { description: { ...this.description, version: 2, properties: [] } }, + }; + } + } + };`, + ); + if (layout === 'package') { + writeFile( + root, + 'package.json', + JSON.stringify({ + name: 'n8n-nodes-foo', + n8n: { + nodes: ['AaaBroken.node.js', 'nodes/Foo.node.js'], + credentials: ['credentials/FooApi.credentials.js'], + }, + }), + ); + } + + await expect(loader.loadAll()).resolves.toBeUndefined(); + + expectCustomFilesLoaded(files); + expect(Object.keys(loader.known.nodes)).toEqual(['foo']); + expect(loader.loadedNodes).toEqual([{ name: 'foo', version: 1 }]); + expect(loader.types.nodes.map(({ name }) => name)).toEqual(['foo']); + expect(loader.nodesByCredential).toEqual({ fooApi: ['foo'] }); + expect(() => loader.getNode('broken')).toThrow(); + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining(brokenPath), { + error: expect.objectContaining({ + message: expect.stringContaining('has a builderHint on the "operation" discriminator'), + }), + }); + }, + ); + + describe.each(['node', 'credentials'])('*.%s.js', (suffix) => { + it.each([ + { + failure: 'a missing dependency', + source: "require('n8n-test-missing-dependency');", + error: expect.objectContaining({ code: 'MODULE_NOT_FOUND' }), + }, + { + failure: 'an incorrect class name', + source: 'module.exports = {};', + error: expect.objectContaining({ + message: 'Class could not be found. Please check if the class is named correctly.', + }), + }, + ])('skips a file with $failure and loads healthy files', async ({ source, error }) => { + const root = + layout === 'loose' ? customDir : path.join(customDir, 'node_modules/n8n-nodes-foo'); + const files = writeCustomFiles(root); + const brokenPath = writeFile(root, `AaaBroken.${suffix}.js`, source); + if (layout === 'package') { + writeFile( + root, + 'package.json', + JSON.stringify({ + name: 'n8n-nodes-foo', + n8n: { + nodes: [ + ...(suffix === 'node' ? [path.basename(brokenPath)] : []), + 'nodes/Foo.node.js', + ], + credentials: [ + ...(suffix === 'credentials' ? [path.basename(brokenPath)] : []), + 'credentials/FooApi.credentials.js', + ], + }, + }), + ); + } + + await expect(loader.loadAll()).resolves.toBeUndefined(); + + expectCustomFilesLoaded(files); + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining(brokenPath), { error }); + }); + }); + }); + + it.each(['node', 'credentials'])( + 'ignores dependency *.%s.js files beside loose custom files', + async (suffix) => { + const files = writeCustomFiles(customDir); + const dependencyPath = writeDependency(customDir, suffix); + const loadNode = vi.spyOn(loader, 'loadNodeFromFile'); + const loadCredential = vi.spyOn(loader, 'loadCredentialFromFile'); + + await expect(loader.loadAll()).resolves.toBeUndefined(); + + expectCustomFilesLoaded(files); + expect(loadNode).not.toHaveBeenCalledWith(dependencyPath); + expect(loadCredential).not.toHaveBeenCalledWith(dependencyPath); + }, + ); + + it('skips ordinary dependencies without a warning', async () => { + const files = writeCustomFiles(customDir); + writeDependency(customDir, 'node'); + + await loader.loadAll(); + + expectCustomFilesLoaded(files); + expect(logger.warn).not.toHaveBeenCalled(); + expect(logger.debug).toHaveBeenCalledWith( + `Skipping custom node package "${path.join(customDir, 'node_modules/wmf/package.json')}": no n8n field found.`, + ); + }); + + it('skips a broken manifest and loads another custom package', async () => { + const packageFile = writeFile(customDir, 'node_modules/a-broken/package.json', '{ "n8n":'); + const packageDir = path.join(customDir, 'node_modules/z-working'); + const files = writeCustomFiles(packageDir); + writeFile( + packageDir, + 'package.json', + JSON.stringify({ + name: 'z-working', + n8n: { + nodes: ['nodes/Foo.node.js'], + credentials: ['credentials/FooApi.credentials.js'], + }, + }), + ); + + await expect(loader.loadAll()).resolves.toBeUndefined(); + + expectCustomFilesLoaded(files); + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining(packageFile), { + error: expect.any(Error), + }); + }); + + it.each(['nodePath', 'credentialPath'] as const)( + 'skips a linked package with a missing %s until it is built', + async (missingFile) => { + const looseFiles = writeCustomFiles(customDir); + const projectDir = path.join(tmpRoot, 'project'); + const projectFiles = writeCustomFiles(path.join(projectDir, 'dist')); + unlinkSync(projectFiles[missingFile]); + writeFile( + projectDir, + 'package.json', + JSON.stringify({ + name: 'n8n-nodes-foo', + n8n: { + nodes: ['dist/nodes/Foo.node.js'], + credentials: ['dist/credentials/FooApi.credentials.js'], + }, + }), + ); + const packageLink = path.join(customDir, 'node_modules/n8n-nodes-foo'); + mkdirSync(path.dirname(packageLink), { recursive: true }); + symlinkSync(projectDir, packageLink, 'dir'); + const linkedFiles = { + nodePath: path.join(packageLink, 'dist/nodes/Foo.node.js'), + credentialPath: path.join(packageLink, 'dist/credentials/FooApi.credentials.js'), + }; + const loadNode = vi.spyOn(loader, 'loadNodeFromFile'); + const loadCredential = vi.spyOn(loader, 'loadCredentialFromFile'); + + await expect(loader.loadAll()).resolves.toBeUndefined(); + + expectCustomFilesLoaded(looseFiles); + expect(loadNode).not.toHaveBeenCalledWith(linkedFiles.nodePath); + expect(loadCredential).not.toHaveBeenCalledWith(linkedFiles.credentialPath); + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining(path.join(packageLink, 'package.json')), + { error: expect.objectContaining({ code: 'ENOENT' }) }, + ); + + writeCustomFiles(path.join(projectDir, 'dist')); + loader.reset(); + vi.clearAllMocks(); + await loader.loadAll(); + + expectCustomFilesLoaded(linkedFiles); + expect(logger.warn).not.toHaveBeenCalled(); + }, + ); + + it.each(['n8n-nodes-foo', '@scope/n8n-nodes-foo'])( + 'loads the symlinked dev package %s without loading its dependencies', + async (packageName) => { + const projectDir = path.join(tmpRoot, 'project'); + writeCustomFiles(path.join(projectDir, 'dist')); + writeFile( + projectDir, + 'package.json', + JSON.stringify({ + name: packageName, + version: '1.0.0', + n8n: { + nodes: ['dist/nodes/Foo.node.js'], + credentials: ['dist/credentials/FooApi.credentials.js'], + }, + }), + ); + writeDependency(projectDir, 'node'); + writeDependency(projectDir, 'credentials'); + writeFile(projectDir, 'dist/nodes/Unlisted.node.js', 'module.exports = {};'); + writeFile(projectDir, 'dist/credentials/Unlisted.credentials.js', 'module.exports = {};'); + const packageLink = path.join(customDir, 'node_modules', packageName); + mkdirSync(path.dirname(packageLink), { recursive: true }); + symlinkSync(projectDir, packageLink, 'dir'); + const loadNode = vi.spyOn(loader, 'loadNodeFromFile'); + const loadCredential = vi.spyOn(loader, 'loadCredentialFromFile'); + + await expect(loader.loadAll()).resolves.toBeUndefined(); + + expect(loader.packageName).toBe('CUSTOM'); + expect(Object.keys(loader.nodeTypes)).toEqual(['foo']); + expect(Object.keys(loader.credentialTypes)).toEqual(['fooApi']); + expect(loader.getNode('foo').type.description.name).toBe('foo'); + expect(loader.getCredential('fooApi').type.name).toBe('fooApi'); + expect(loadNode).not.toHaveBeenCalledWith( + path.join(packageLink, 'node_modules/wmf/dist/wmf.node.js'), + ); + expect(loadCredential).not.toHaveBeenCalledWith( + path.join(packageLink, 'node_modules/wmf/dist/wmf.credentials.js'), + ); + }, + ); +}); diff --git a/packages/core/src/nodes-loader/__tests__/custom-directory-loader.hot-reload.test.ts b/packages/core/src/nodes-loader/__tests__/custom-directory-loader.hot-reload.test.ts index 125eef42954..00d5c560e0e 100644 --- a/packages/core/src/nodes-loader/__tests__/custom-directory-loader.hot-reload.test.ts +++ b/packages/core/src/nodes-loader/__tests__/custom-directory-loader.hot-reload.test.ts @@ -41,6 +41,13 @@ describe('CustomDirectoryLoader hot reload (NODE-5225)', () => { return file; }; + const writePackage = (dir: string, name: string) => { + fs.writeFileSync( + path.join(dir, 'package.json'), + JSON.stringify({ name, n8n: { nodes: ['dist/nodes/Reloadable.node.js'] } }), + ); + }; + // The node description type is a union; the fixture is a non-versioned node. const loadedVersion = (loader: CustomDirectoryLoader) => (loader.getNode('reloadable').type.description as INodeTypeDescription).version; @@ -59,6 +66,7 @@ describe('CustomDirectoryLoader hot reload (NODE-5225)', () => { // The developer's node project, compiled to `dist`. const projectDir = path.join(tmpRoot, 'my-node-project'); const nodeFile = writeNode(path.join(projectDir, 'dist', 'nodes'), 1); + writePackage(projectDir, 'n8n-nodes-reloadable'); // The project is symlinked into node_modules, exactly how `n8n-node dev` // installs the node for live preview. @@ -86,6 +94,7 @@ describe('CustomDirectoryLoader hot reload (NODE-5225)', () => { // one level deeper, under `node_modules/@scope/`. const projectDir = path.join(tmpRoot, 'my-scoped-node-project'); const nodeFile = writeNode(path.join(projectDir, 'dist', 'nodes'), 1); + writePackage(projectDir, '@scope/n8n-nodes-reloadable'); const scopeDir = path.join(customDir, 'node_modules', '@scope'); fs.mkdirSync(scopeDir, { recursive: true }); diff --git a/packages/core/src/nodes-loader/__tests__/directory-loader.test.ts b/packages/core/src/nodes-loader/__tests__/directory-loader.test.ts index 18c79482636..2ba5b637de2 100644 --- a/packages/core/src/nodes-loader/__tests__/directory-loader.test.ts +++ b/packages/core/src/nodes-loader/__tests__/directory-loader.test.ts @@ -18,7 +18,8 @@ const mockFs = mock(fs); const mockFsPromises = mock(fsPromises); vi.mock('fast-glob', () => ({ - default: async (pattern: string) => { + default: async (pattern: string | string[]) => { + if (Array.isArray(pattern)) return []; return pattern.endsWith('.node.js') ? ['dist/Node1/Node1.node.js', 'dist/Node2/Node2.node.js'] : ['dist/Credential1.js']; diff --git a/packages/core/src/nodes-loader/custom-directory-loader.ts b/packages/core/src/nodes-loader/custom-directory-loader.ts index 5df21b5717d..d5b5edc1afa 100644 --- a/packages/core/src/nodes-loader/custom-directory-loader.ts +++ b/packages/core/src/nodes-loader/custom-directory-loader.ts @@ -1,7 +1,11 @@ import glob from 'fast-glob'; +import { jsonParse } from 'n8n-workflow'; +import { access, readFile } from 'node:fs/promises'; +import path from 'node:path'; import { CUSTOM_NODES_PACKAGE_NAME } from './constants'; import { DirectoryLoader } from './directory-loader'; +import type { n8n } from './types'; /** * Loader for source files of nodes and credentials located in a custom dir, @@ -21,19 +25,66 @@ export class CustomDirectoryLoader extends DirectoryLoader { const nodes = await glob('**/*.node.js', { cwd: this.directory, absolute: true, + ignore: ['**/node_modules/**'], }); - for (const nodePath of nodes) { - this.loadNodeFromFile(nodePath); - } - const credentials = await glob('**/*.credentials.js', { cwd: this.directory, absolute: true, + ignore: ['**/node_modules/**'], }); + // Dev packages are linked here. Load their manifest entries, not their dependencies. + const packageFiles = await glob( + ['node_modules/*/package.json', 'node_modules/@*/*/package.json'], + { cwd: this.directory, absolute: true }, + ); + for (const packageFile of packageFiles) { + try { + const { n8n } = jsonParse(await readFile(packageFile, 'utf8')); + if (!n8n) { + this.logger.debug(`Skipping custom node package "${packageFile}": no n8n field found.`); + continue; + } + + const packageDirectory = path.dirname(packageFile); + const packageNodes = Array.isArray(n8n.nodes) + ? n8n.nodes.map((nodePath) => path.resolve(packageDirectory, nodePath)) + : []; + const packageCredentials = Array.isArray(n8n.credentials) + ? n8n.credentials.map((credentialPath) => path.resolve(packageDirectory, credentialPath)) + : []; + + // Skip the whole package if a build has not produced all its manifest entries. + await Promise.all( + [...packageNodes, ...packageCredentials].map(async (filePath) => await access(filePath)), + ); + nodes.push(...packageNodes); + credentials.push(...packageCredentials); + } catch (error) { + this.logger.warn( + `Skipping custom node package "${packageFile}": failed to read its manifest or find its files.`, + { error }, + ); + } + } + + for (const nodePath of nodes) { + try { + this.loadNodeFromFile(nodePath); + } catch (error) { + this.logger.warn(`Skipping custom node "${nodePath}": failed to load.`, { error }); + } + } + for (const credentialPath of credentials) { - this.loadCredentialFromFile(credentialPath); + try { + this.loadCredentialFromFile(credentialPath); + } catch (error) { + this.logger.warn(`Skipping custom credential "${credentialPath}": failed to load.`, { + error, + }); + } } } } diff --git a/packages/core/src/nodes-loader/directory-loader.ts b/packages/core/src/nodes-loader/directory-loader.ts index c8c9ae5f520..ac24340e69b 100644 --- a/packages/core/src/nodes-loader/directory-loader.ts +++ b/packages/core/src/nodes-loader/directory-loader.ts @@ -211,6 +211,11 @@ export abstract class DirectoryLoader implements NodeLoader { : tempNode.description.version; } + const nodeDescriptions = this.getVersionedNodeTypeAll(tempNode).map( + ({ description }) => description, + ); + nodeDescriptions.forEach(validateNodeDescription); + this.known.nodes[nodeType] = { className: tempNode.constructor.name, sourcePath: filePath, @@ -226,10 +231,7 @@ export abstract class DirectoryLoader implements NodeLoader { version: nodeVersion, }); - this.getVersionedNodeTypeAll(tempNode).forEach(({ description }) => { - validateNodeDescription(description); - this.types.nodes.push(description); - }); + this.types.nodes.push(...nodeDescriptions); for (const credential of this.getCredentialsForNode(tempNode)) { if (!this.nodesByCredential[credential.name]) {