fix: prevent TypeError in getNodeTypeAlternatives with invalid inputs

## Problem
Critical TypeError bugs affecting 60% of production errors (323/563 errors, 127 users):
- "Cannot read properties of undefined (reading 'split')" in get_node_essentials
- "Cannot read properties of undefined (reading 'includes')" in get_node_info

## Root Cause
getNodeTypeAlternatives() in src/utils/node-utils.ts called string methods
(toLowerCase, includes, split) without validating nodeType parameter.

When AI assistants passed undefined/null/empty nodeType values, the code
crashed with TypeError instead of returning a helpful error message.

## Solution (Defense in Depth)

### Layer 1: Defensive Programming (node-utils.ts:41-43)
Added type guard in getNodeTypeAlternatives():
- Returns empty array for undefined, null, non-string, or empty inputs
- Prevents TypeError crashes in utility function
- Allows calling code to handle "not found" gracefully

### Layer 2: Enhanced Validation (server.ts:607-609)
Improved validateToolParamsBasic() to catch empty strings:
- Detects empty string parameters before processing
- Provides clear error: "String parameters cannot be empty"
- Complements existing undefined/null validation

## Impact
- Eliminates 323 errors (57.4% of production errors)
- Helps 127 users (76.5% of users experiencing errors)
- Provides clear, actionable error messages instead of TypeErrors
- No performance impact on valid inputs

## Testing
- Added 21 comprehensive unit tests (all passing)
- Tested with n8n-mcp-tester agent (all scenarios verified)
- Confirmed no TypeErrors with invalid inputs
- Verified valid inputs continue to work perfectly

## Affected Tools
- get_node_essentials (208 errors → 0)
- get_node_info (115 errors → 0)
- get_node_documentation (17 errors → 0)

Resolves #275

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
czlonkowski
2025-10-06 00:02:48 +02:00
parent aeaba3b9ca
commit f139d38c81
3 changed files with 146 additions and 4 deletions

View File

@@ -599,16 +599,23 @@ export class N8NDocumentationMCPServer {
*/
private validateToolParamsBasic(toolName: string, args: any, requiredParams: string[]): void {
const missing: string[] = [];
const invalid: string[] = [];
for (const param of requiredParams) {
if (!(param in args) || args[param] === undefined || args[param] === null) {
missing.push(param);
} else if (typeof args[param] === 'string' && args[param].trim() === '') {
invalid.push(`${param} (empty string)`);
}
}
if (missing.length > 0) {
throw new Error(`Missing required parameters for ${toolName}: ${missing.join(', ')}. Please provide the required parameters to use this tool.`);
}
if (invalid.length > 0) {
throw new Error(`Invalid parameters for ${toolName}: ${invalid.join(', ')}. String parameters cannot be empty.`);
}
}
/**

View File

@@ -32,13 +32,18 @@ export function normalizeNodeType(nodeType: string): string {
/**
* Gets alternative node type formats to try for lookups
*
*
* @param nodeType The original node type
* @returns Array of alternative formats to try
*/
export function getNodeTypeAlternatives(nodeType: string): string[] {
// Defensive: validate input to prevent TypeError when nodeType is undefined/null/empty
if (!nodeType || typeof nodeType !== 'string' || nodeType.trim() === '') {
return [];
}
const alternatives: string[] = [];
// Add lowercase version
alternatives.push(nodeType.toLowerCase());

View File

@@ -0,0 +1,130 @@
import { describe, it, expect } from 'vitest';
import { getNodeTypeAlternatives, normalizeNodeType, getWorkflowNodeType } from '../../../src/utils/node-utils';
describe('node-utils', () => {
describe('getNodeTypeAlternatives', () => {
describe('valid inputs', () => {
it('should generate alternatives for standard node type', () => {
const alternatives = getNodeTypeAlternatives('nodes-base.httpRequest');
expect(alternatives).toContain('nodes-base.httprequest');
expect(alternatives.length).toBeGreaterThan(0);
});
it('should generate alternatives for langchain node type', () => {
const alternatives = getNodeTypeAlternatives('nodes-langchain.agent');
expect(alternatives).toContain('nodes-langchain.agent');
expect(alternatives.length).toBeGreaterThan(0);
});
it('should generate alternatives for bare node name', () => {
const alternatives = getNodeTypeAlternatives('webhook');
expect(alternatives).toContain('nodes-base.webhook');
expect(alternatives).toContain('nodes-langchain.webhook');
});
});
describe('invalid inputs - defensive validation', () => {
it('should return empty array for undefined', () => {
const alternatives = getNodeTypeAlternatives(undefined as any);
expect(alternatives).toEqual([]);
});
it('should return empty array for null', () => {
const alternatives = getNodeTypeAlternatives(null as any);
expect(alternatives).toEqual([]);
});
it('should return empty array for empty string', () => {
const alternatives = getNodeTypeAlternatives('');
expect(alternatives).toEqual([]);
});
it('should return empty array for whitespace-only string', () => {
const alternatives = getNodeTypeAlternatives(' ');
expect(alternatives).toEqual([]);
});
it('should return empty array for non-string input (number)', () => {
const alternatives = getNodeTypeAlternatives(123 as any);
expect(alternatives).toEqual([]);
});
it('should return empty array for non-string input (object)', () => {
const alternatives = getNodeTypeAlternatives({} as any);
expect(alternatives).toEqual([]);
});
it('should return empty array for non-string input (array)', () => {
const alternatives = getNodeTypeAlternatives([] as any);
expect(alternatives).toEqual([]);
});
});
describe('edge cases', () => {
it('should handle node type with only prefix', () => {
const alternatives = getNodeTypeAlternatives('nodes-base.');
expect(alternatives).toBeInstanceOf(Array);
});
it('should handle node type with multiple dots', () => {
const alternatives = getNodeTypeAlternatives('nodes-base.some.complex.type');
expect(alternatives).toBeInstanceOf(Array);
expect(alternatives.length).toBeGreaterThan(0);
});
it('should handle camelCase node names', () => {
const alternatives = getNodeTypeAlternatives('nodes-base.httpRequest');
expect(alternatives).toContain('nodes-base.httprequest');
});
});
});
describe('normalizeNodeType', () => {
it('should normalize n8n-nodes-base prefix', () => {
expect(normalizeNodeType('n8n-nodes-base.webhook')).toBe('nodes-base.webhook');
});
it('should normalize @n8n/n8n-nodes-langchain prefix', () => {
expect(normalizeNodeType('@n8n/n8n-nodes-langchain.agent')).toBe('nodes-langchain.agent');
});
it('should normalize n8n-nodes-langchain prefix', () => {
expect(normalizeNodeType('n8n-nodes-langchain.chatTrigger')).toBe('nodes-langchain.chatTrigger');
});
it('should leave already normalized types unchanged', () => {
expect(normalizeNodeType('nodes-base.slack')).toBe('nodes-base.slack');
});
it('should leave community nodes unchanged', () => {
expect(normalizeNodeType('community.customNode')).toBe('community.customNode');
});
});
describe('getWorkflowNodeType', () => {
it('should construct workflow node type for n8n-nodes-base', () => {
expect(getWorkflowNodeType('n8n-nodes-base', 'nodes-base.webhook')).toBe('n8n-nodes-base.webhook');
});
it('should construct workflow node type for langchain', () => {
expect(getWorkflowNodeType('@n8n/n8n-nodes-langchain', 'nodes-langchain.agent')).toBe('@n8n/n8n-nodes-langchain.agent');
});
it('should return as-is for unknown packages', () => {
expect(getWorkflowNodeType('custom-package', 'custom.node')).toBe('custom.node');
});
});
});