feat: Add test infrastructure, fix bugs, and improve code quality #11
Conversation
- Add Vitest test framework with coverage support - Fix EPUB temp file cleanup memory leak - Add connection idle timeout (30s) to prevent connection leaks - Add comprehensive error handling with specific Couchbase error types - Add input validation for document IDs, queries, and result limits - Fix type safety issues in document parsing - Improve credential security with required fields and descriptions - Fix fragile constructor.name type checking in processDocuments - Fix vector store update operation cleanup - Standardize empty search results response format - Add retry utility for transient failures - Add shared constants file for configuration - Add JSDoc documentation to key functions
Summary of ChangesHello @teetangh, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a substantial upgrade to the n8n-nodes-couchbase package, focusing on enhancing its stability, usability, and maintainability. It integrates a new testing framework, addresses several critical bugs, significantly improves error handling and input validation, and refines overall code quality through better documentation and utility functions. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an excellent and comprehensive pull request that significantly improves the n8n-nodes-couchbase package. The addition of a test suite with Vitest, robust error handling, input validation, and fixes for memory and connection leaks are all major enhancements. The code quality is much improved with the introduction of shared constants and a retry utility. My review includes a few suggestions to further improve maintainability by reducing code duplication and to fix a minor issue in value parsing.
| // Validation constants | ||
| const MAX_DOCUMENT_ID_LENGTH = 250; // Couchbase max key length in bytes | ||
| const MIN_RESULTS_LIMIT = 1; | ||
| const MAX_RESULTS_LIMIT = 10000; |
There was a problem hiding this comment.
These constants are also defined in the new utils/constants.ts file. To avoid code duplication and improve maintainability, please remove these local definitions and import them from @utils/constants instead.
| function parseDocumentValue( | ||
| context: IExecuteFunctions, | ||
| value: unknown, | ||
| ): IDataObject | IDataObject[] | string | number | boolean { | ||
| // If it's already an object (not null, not array), return it | ||
| if (typeof value === 'object' && value !== null && !Array.isArray(value)) { | ||
| return value as IDataObject; | ||
| } | ||
|
|
||
| // If it's an array, return it | ||
| if (Array.isArray(value)) { | ||
| return value as IDataObject[]; | ||
| } | ||
|
|
||
| // If it's a string, try to parse as JSON | ||
| if (typeof value === 'string') { | ||
| const trimmed = value.trim(); | ||
| // Check if it looks like JSON (starts with { or [) | ||
| if (trimmed.startsWith('{') || trimmed.startsWith('[')) { | ||
| try { | ||
| return JSON.parse(trimmed) as IDataObject | IDataObject[]; | ||
| } catch { | ||
| throw new NodeOperationError( | ||
| context.getNode(), | ||
| 'Invalid JSON in document value', | ||
| { | ||
| description: 'The document value appears to be JSON but could not be parsed. Please check the syntax.', | ||
| }, | ||
| ); | ||
| } | ||
| } | ||
| // Return string as-is if it's not JSON-like (Couchbase supports non-JSON values) | ||
| return value; | ||
| } | ||
|
|
||
| // Return primitives as-is (numbers, booleans, etc.) | ||
| if (typeof value === 'number' || typeof value === 'boolean') { | ||
| return value; | ||
| } | ||
|
|
||
| // Default fallback - return empty object | ||
| return {} as IDataObject; | ||
| } |
There was a problem hiding this comment.
The current implementation of parseDocumentValue converts a null input into an empty object {}. Since Couchbase can store null values, it would be more accurate to handle null explicitly and return it, allowing users to store null values in their documents. This requires updating the function's return type to include null.
function parseDocumentValue(
context: IExecuteFunctions,
value: unknown,
): IDataObject | IDataObject[] | string | number | boolean | null {
// Handle null explicitly, as Couchbase supports it.
if (value === null) {
return null;
}
// If it's already an object (not null, not array), return it
if (typeof value === 'object' && !Array.isArray(value)) {
return value as IDataObject;
}
// If it's an array, return it
if (Array.isArray(value)) {
return value as IDataObject[];
}
// If it's a string, try to parse as JSON
if (typeof value === 'string') {
const trimmed = value.trim();
// Check if it looks like JSON (starts with { or [)
if (trimmed.startsWith('{') || trimmed.startsWith('[')) {
try {
return JSON.parse(trimmed) as IDataObject | IDataObject[];
} catch {
throw new NodeOperationError(
context.getNode(),
'Invalid JSON in document value',
{
description: 'The document value appears to be JSON but could not be parsed. Please check the syntax.',
},
);
}
}
// Return string as-is if it's not JSON-like (Couchbase supports non-JSON values)
return value;
}
// Return primitives as-is (numbers, booleans, etc.)
if (typeof value === 'number' || typeof value === 'boolean') {
return value;
}
// Default fallback - return empty object for undefined or other unhandled types
return {} as IDataObject;
}| // Connection configuration constants | ||
| const CONNECTION_TIMEOUT_MS = 10000; // 10 seconds | ||
| const CONNECTION_IDLE_TIMEOUT_MS = 30000; // 30 seconds - auto-close after inactivity |
There was a problem hiding this comment.
These constants are also defined in utils/constants.ts. The version in utils/constants.ts is preferable as it allows overriding via environment variables. Please remove these local definitions and import them from ../constants at the top of the file.
1 participant
Summary
This PR introduces comprehensive improvements to the n8n-nodes-couchbase package including test infrastructure, bug fixes, and code quality enhancements.
Test Infrastructure
Bug Fixes
constructor.namechecks with proper type guardsError Handling
Input Validation
Code Quality
Test Plan
npx tsc --noEmit)pnpm lint)pnpm build)pnpm test- 2/2 tests passing)