diff --git a/CHANGELOG.md b/CHANGELOG.md index bc4783a..0fe327c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,89 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### ✨ Enhancements + +**Issue #361: Enhanced HTTP Request Node Validation Suggestions** + +Added helpful suggestions for HTTP Request node best practices to prevent common production issues discovered through real-world workflow analysis. + +#### What's New + +1. **alwaysOutputData Suggestion** + - Suggests adding `alwaysOutputData: true` at node level (not in parameters) + - Prevents silent workflow failures when HTTP requests error + - Ensures downstream error handling can process failed requests + - Example suggestion: "Consider adding alwaysOutputData: true at node level for better error handling. This ensures the node produces output even when HTTP requests fail, allowing downstream error handling." + +2. **responseFormat Suggestion for API Endpoints** + - Suggests setting `options.response.response.responseFormat` for API endpoints + - Prevents JSON parsing confusion + - Triggered when URL contains `/api`, `/rest`, `supabase`, `firebase`, `googleapis`, or `.com/v` patterns + - Example suggestion: "API endpoints should explicitly set options.response.response.responseFormat to 'json' or 'text' to prevent confusion about response parsing" + +3. **Enhanced URL Protocol Validation** + - Detects missing protocol in expression-based URLs + - Warns about patterns like `=www.{{ $json.domain }}.com` (missing http://) + - Warns about expressions without protocol: `={{ $json.domain }}/api/data` + - Example warning: "URL expression appears to be missing http:// or https:// protocol" + +#### Investigation Findings + +This enhancement was developed after thorough investigation of issue #361: + +**Key Discoveries:** +- ✅ Mixed expression syntax `=literal{{ expression }}` **actually works in n8n** - the issue report's primary claim was incorrect +- ✅ Real validation gaps identified: missing `alwaysOutputData` and `responseFormat` checks +- ✅ Workflow analysis showed "?" icon in UI caused by missing required URL (already caught by validation) +- ✅ Compared broken vs fixed workflows to identify actual production issues + +**Testing Evidence:** +- Analyzed workflow SwjKJsJhe8OsYfBk with mixed syntax - executions successful +- Compared broken workflow (mBmkyj460i5rYTG4) with fixed workflow (hQI9pby3nSFtk4TV) +- Identified that fixed workflow has `alwaysOutputData: true` and explicit `responseFormat: "json"` + +#### Impact + +- **Non-Breaking**: All changes are suggestions/warnings, not errors +- **Profile-Aware**: Suggestions shown in all profiles for maximum helpfulness +- **Actionable**: Clear guidance on how to implement best practices +- **Production-Focused**: Addresses real workflow reliability concerns from actual broken workflows + +#### Test Coverage + +Added 8 new test cases covering: +- alwaysOutputData suggestion for all HTTP Request nodes +- responseFormat suggestion for API endpoint detection (various patterns) +- responseFormat NOT suggested when already configured +- URL protocol validation for expression-based URLs +- Protocol warnings for missing http:// in expressions +- No false positives when protocol is correctly included + +#### Technical Details + +**Files Modified:** +- `src/services/enhanced-config-validator.ts` - Added `enhanceHttpRequestValidation()` implementation +- `tests/unit/services/enhanced-config-validator.test.ts` - Added 8 comprehensive test cases + +**Validation Flow:** +1. Check for alwaysOutputData suggestion (all HTTP Request nodes) +2. Detect API endpoints by URL patterns +3. Check for explicit responseFormat configuration +4. Validate expression-based URLs for protocol issues + +#### Related + +- **Issue**: #361 - validate_node_operation: Missing critical HTTP Request node configuration checks +- **Analysis**: Deep investigation with @agent-Explore and @agent-n8n-mcp-tester +- **Workflows Analyzed**: + - SwjKJsJhe8OsYfBk (mixed syntax test) + - mBmkyj460i5rYTG4 (broken workflow) + - hQI9pby3nSFtk4TV (fixed workflow) + +Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en + +--- + ### 🐛 Bug Fixes **Issue #360: Enhanced Warnings for If/Switch Node Connection Parameters** diff --git a/data/nodes.db b/data/nodes.db index b67f146..e22fcad 100644 Binary files a/data/nodes.db and b/data/nodes.db differ diff --git a/package.json b/package.json index 4a608cf..6ff5306 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "n8n-mcp", - "version": "2.22.1", + "version": "2.22.2", "description": "Integration between n8n workflow automation and Model Context Protocol (MCP)", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/src/services/enhanced-config-validator.ts b/src/services/enhanced-config-validator.ts index beff9a2..6c652ce 100644 --- a/src/services/enhanced-config-validator.ts +++ b/src/services/enhanced-config-validator.ts @@ -401,7 +401,59 @@ export class EnhancedConfigValidator extends ConfigValidator { config: Record, result: EnhancedValidationResult ): void { - // Examples removed - validation provides error messages and fixes instead + const url = String(config.url || ''); + const options = config.options || {}; + + // 1. Suggest alwaysOutputData for better error handling (node-level property) + // Note: We can't check if it exists (it's node-level, not in parameters), + // but we can suggest it as a best practice + if (!result.suggestions.some(s => typeof s === 'string' && s.includes('alwaysOutputData'))) { + result.suggestions.push( + 'Consider adding alwaysOutputData: true at node level (not in parameters) for better error handling. ' + + 'This ensures the node produces output even when HTTP requests fail, allowing downstream error handling.' + ); + } + + // 2. Suggest responseFormat for API endpoints + const lowerUrl = url.toLowerCase(); + const isApiEndpoint = + // Subdomain patterns (api.example.com) + /^https?:\/\/api\./i.test(url) || + // Path patterns with word boundaries to prevent false positives like "therapist", "restaurant" + /\/api[\/\?]|\/api$/i.test(url) || + /\/rest[\/\?]|\/rest$/i.test(url) || + // Known API service domains + lowerUrl.includes('supabase.co') || + lowerUrl.includes('firebase') || + lowerUrl.includes('googleapis.com') || + // Versioned API paths (e.g., example.com/v1, example.com/v2) + /\.com\/v\d+/i.test(url); + + if (isApiEndpoint && !options.response?.response?.responseFormat) { + result.suggestions.push( + 'API endpoints should explicitly set options.response.response.responseFormat to "json" or "text" ' + + 'to prevent confusion about response parsing. Example: ' + + '{ "options": { "response": { "response": { "responseFormat": "json" } } } }' + ); + } + + // 3. Enhanced URL protocol validation for expressions + if (url && url.startsWith('=')) { + // Expression-based URL - check for common protocol issues + const expressionContent = url.slice(1); // Remove = prefix + const lowerExpression = expressionContent.toLowerCase(); + + // Check for missing protocol in expression (case-insensitive) + if (expressionContent.startsWith('www.') || + (expressionContent.includes('{{') && !lowerExpression.includes('http'))) { + result.warnings.push({ + type: 'invalid_value', + property: 'url', + message: 'URL expression appears to be missing http:// or https:// protocol', + suggestion: 'Include protocol in your expression. Example: ={{ "https://" + $json.domain + ".com" }}' + }); + } + } } /** diff --git a/tests/unit/services/enhanced-config-validator.test.ts b/tests/unit/services/enhanced-config-validator.test.ts index 1998d46..64df123 100644 --- a/tests/unit/services/enhanced-config-validator.test.ts +++ b/tests/unit/services/enhanced-config-validator.test.ts @@ -802,4 +802,335 @@ describe('EnhancedConfigValidator', () => { expect(result.errors[0].property).toBe('test'); }); }); + + describe('enhanceHttpRequestValidation', () => { + it('should suggest alwaysOutputData for HTTP Request nodes', () => { + const nodeType = 'nodes-base.httpRequest'; + const config = { + url: 'https://api.example.com/data', + method: 'GET' + }; + const properties = [ + { name: 'url', type: 'string', required: true }, + { name: 'method', type: 'options', required: false } + ]; + + const result = EnhancedConfigValidator.validateWithMode( + nodeType, + config, + properties, + 'operation', + 'ai-friendly' + ); + + expect(result.valid).toBe(true); + expect(result.suggestions).toContainEqual( + expect.stringContaining('alwaysOutputData: true at node level') + ); + expect(result.suggestions).toContainEqual( + expect.stringContaining('ensures the node produces output even when HTTP requests fail') + ); + }); + + it('should suggest responseFormat for API endpoint URLs', () => { + const nodeType = 'nodes-base.httpRequest'; + const config = { + url: 'https://api.example.com/data', + method: 'GET', + options: {} // Empty options, no responseFormat + }; + const properties = [ + { name: 'url', type: 'string', required: true }, + { name: 'method', type: 'options', required: false }, + { name: 'options', type: 'collection', required: false } + ]; + + const result = EnhancedConfigValidator.validateWithMode( + nodeType, + config, + properties, + 'operation', + 'ai-friendly' + ); + + expect(result.valid).toBe(true); + expect(result.suggestions).toContainEqual( + expect.stringContaining('responseFormat') + ); + expect(result.suggestions).toContainEqual( + expect.stringContaining('options.response.response.responseFormat') + ); + }); + + it('should suggest responseFormat for Supabase URLs', () => { + const nodeType = 'nodes-base.httpRequest'; + const config = { + url: 'https://xxciwnthnnywanbplqwg.supabase.co/rest/v1/messages', + method: 'GET', + options: {} + }; + const properties = [ + { name: 'url', type: 'string', required: true } + ]; + + const result = EnhancedConfigValidator.validateWithMode( + nodeType, + config, + properties, + 'operation', + 'ai-friendly' + ); + + expect(result.suggestions).toContainEqual( + expect.stringContaining('responseFormat') + ); + }); + + it('should NOT suggest responseFormat when already configured', () => { + const nodeType = 'nodes-base.httpRequest'; + const config = { + url: 'https://api.example.com/data', + method: 'GET', + options: { + response: { + response: { + responseFormat: 'json' + } + } + } + }; + const properties = [ + { name: 'url', type: 'string', required: true }, + { name: 'options', type: 'collection', required: false } + ]; + + const result = EnhancedConfigValidator.validateWithMode( + nodeType, + config, + properties, + 'operation', + 'ai-friendly' + ); + + const responseFormatSuggestion = result.suggestions.find( + (s: string) => s.includes('responseFormat') + ); + expect(responseFormatSuggestion).toBeUndefined(); + }); + + it('should warn about missing protocol in expression-based URLs', () => { + const nodeType = 'nodes-base.httpRequest'; + const config = { + url: '=www.{{ $json.domain }}.com', + method: 'GET' + }; + const properties = [ + { name: 'url', type: 'string', required: true } + ]; + + const result = EnhancedConfigValidator.validateWithMode( + nodeType, + config, + properties, + 'operation', + 'ai-friendly' + ); + + expect(result.warnings).toContainEqual( + expect.objectContaining({ + type: 'invalid_value', + property: 'url', + message: expect.stringContaining('missing http:// or https://') + }) + ); + }); + + it('should warn about missing protocol in expressions with template markers', () => { + const nodeType = 'nodes-base.httpRequest'; + const config = { + url: '={{ $json.domain }}/api/data', + method: 'GET' + }; + const properties = [ + { name: 'url', type: 'string', required: true } + ]; + + const result = EnhancedConfigValidator.validateWithMode( + nodeType, + config, + properties, + 'operation', + 'ai-friendly' + ); + + expect(result.warnings).toContainEqual( + expect.objectContaining({ + type: 'invalid_value', + property: 'url', + message: expect.stringContaining('missing http:// or https://') + }) + ); + }); + + it('should NOT warn when expression includes http protocol', () => { + const nodeType = 'nodes-base.httpRequest'; + const config = { + url: '={{ "https://" + $json.domain + ".com" }}', + method: 'GET' + }; + const properties = [ + { name: 'url', type: 'string', required: true } + ]; + + const result = EnhancedConfigValidator.validateWithMode( + nodeType, + config, + properties, + 'operation', + 'ai-friendly' + ); + + const urlWarning = result.warnings.find( + (w: any) => w.property === 'url' && w.message.includes('protocol') + ); + expect(urlWarning).toBeUndefined(); + }); + + it('should NOT suggest responseFormat for non-API URLs', () => { + const nodeType = 'nodes-base.httpRequest'; + const config = { + url: 'https://example.com/page.html', + method: 'GET', + options: {} + }; + const properties = [ + { name: 'url', type: 'string', required: true } + ]; + + const result = EnhancedConfigValidator.validateWithMode( + nodeType, + config, + properties, + 'operation', + 'ai-friendly' + ); + + const responseFormatSuggestion = result.suggestions.find( + (s: string) => s.includes('responseFormat') + ); + expect(responseFormatSuggestion).toBeUndefined(); + }); + + it('should detect missing protocol in expressions with uppercase HTTP', () => { + const nodeType = 'nodes-base.httpRequest'; + const config = { + url: '={{ "HTTP://" + $json.domain + ".com" }}', + method: 'GET' + }; + const properties = [ + { name: 'url', type: 'string', required: true } + ]; + + const result = EnhancedConfigValidator.validateWithMode( + nodeType, + config, + properties, + 'operation', + 'ai-friendly' + ); + + // Should NOT warn because HTTP:// is present (case-insensitive) + expect(result.warnings).toHaveLength(0); + }); + + it('should NOT suggest responseFormat for false positive URLs', () => { + const nodeType = 'nodes-base.httpRequest'; + const testUrls = [ + 'https://example.com/therapist-directory', + 'https://restaurant-bookings.com/reserve', + 'https://forest-management.org/data' + ]; + + testUrls.forEach(url => { + const config = { + url, + method: 'GET', + options: {} + }; + const properties = [ + { name: 'url', type: 'string', required: true } + ]; + + const result = EnhancedConfigValidator.validateWithMode( + nodeType, + config, + properties, + 'operation', + 'ai-friendly' + ); + + const responseFormatSuggestion = result.suggestions.find( + (s: string) => s.includes('responseFormat') + ); + expect(responseFormatSuggestion).toBeUndefined(); + }); + }); + + it('should suggest responseFormat for case-insensitive API paths', () => { + const nodeType = 'nodes-base.httpRequest'; + const testUrls = [ + 'https://example.com/API/users', + 'https://example.com/Rest/data', + 'https://example.com/REST/v1/items' + ]; + + testUrls.forEach(url => { + const config = { + url, + method: 'GET', + options: {} + }; + const properties = [ + { name: 'url', type: 'string', required: true } + ]; + + const result = EnhancedConfigValidator.validateWithMode( + nodeType, + config, + properties, + 'operation', + 'ai-friendly' + ); + + expect(result.suggestions).toContainEqual( + expect.stringContaining('responseFormat') + ); + }); + }); + + it('should handle null and undefined URLs gracefully', () => { + const nodeType = 'nodes-base.httpRequest'; + const testConfigs = [ + { url: null, method: 'GET' }, + { url: undefined, method: 'GET' }, + { url: '', method: 'GET' } + ]; + + testConfigs.forEach(config => { + const properties = [ + { name: 'url', type: 'string', required: true } + ]; + + expect(() => { + EnhancedConfigValidator.validateWithMode( + nodeType, + config, + properties, + 'operation', + 'ai-friendly' + ); + }).not.toThrow(); + }); + }); + }); }); \ No newline at end of file