From dd72366f0f9ef0e6fc25b148ee2725300f0ea263 Mon Sep 17 00:00:00 2001 From: Valentin Kirilov Date: Fri, 28 Nov 2025 13:40:10 +0200 Subject: [PATCH 1/4] fix(api): add proper error messages for invalid cloud database endpoints Validate publicEndpoint before calling split() to prevent null reference errors. Return user-friendly error messages instead of raw JavaScript errors. References: #RI-7778 --- .../api/src/constants/custom-error-codes.ts | 1 + .../api/src/constants/error-messages.ts | 2 + .../cloud-autodiscovery.service.spec.ts | 59 +++++++++++++++++++ .../cloud-autodiscovery.service.ts | 12 ++++ .../database/utils/cloud-data-converter.ts | 8 +++ ...oud-database-endpoint-invalid.exception.ts | 23 ++++++++ .../src/modules/cloud/job/exceptions/index.ts | 1 + .../jobs/create-free-database.cloud-job.ts | 6 ++ .../jobs/import-free-database.cloud-job.ts | 10 +++- .../ui/src/constants/customErrorCodes.ts | 1 + 10 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 redisinsight/api/src/modules/cloud/job/exceptions/cloud-database-endpoint-invalid.exception.ts diff --git a/redisinsight/api/src/constants/custom-error-codes.ts b/redisinsight/api/src/constants/custom-error-codes.ts index 52e4bf1538..174832bc11 100644 --- a/redisinsight/api/src/constants/custom-error-codes.ts +++ b/redisinsight/api/src/constants/custom-error-codes.ts @@ -47,6 +47,7 @@ export enum CustomErrorCodes { CloudJobNotFound = 11_113, CloudSubscriptionAlreadyExistsFree = 11_114, CloudDatabaseImportForbidden = 11_115, + CloudDatabaseEndpointInvalid = 11_116, // General database errors [11200, 11299] DatabaseAlreadyExists = 11_200, diff --git a/redisinsight/api/src/constants/error-messages.ts b/redisinsight/api/src/constants/error-messages.ts index cff9801e61..83e5a04e90 100644 --- a/redisinsight/api/src/constants/error-messages.ts +++ b/redisinsight/api/src/constants/error-messages.ts @@ -144,6 +144,8 @@ export default { CLOUD_DATABASE_ALREADY_EXISTS_FREE: 'Free database already exists', CLOUD_DATABASE_IMPORT_FORBIDDEN: 'Adding your Redis Cloud database to Redis Insight is disabled due to a setting restricting database connection management.', + CLOUD_DATABASE_ENDPOINT_INVALID: + 'Database endpoint is not available or not fully provisioned yet.', CLOUD_PLAN_NOT_FOUND_FREE: 'Unable to find free cloud plan', CLOUD_SUBSCRIPTION_ALREADY_EXISTS_FREE: 'Free subscription already exists', COMMON_DEFAULT_IMPORT_ERROR: 'Unable to import default data', diff --git a/redisinsight/api/src/modules/cloud/autodiscovery/cloud-autodiscovery.service.spec.ts b/redisinsight/api/src/modules/cloud/autodiscovery/cloud-autodiscovery.service.spec.ts index 904915510b..2fdc87a362 100644 --- a/redisinsight/api/src/modules/cloud/autodiscovery/cloud-autodiscovery.service.spec.ts +++ b/redisinsight/api/src/modules/cloud/autodiscovery/cloud-autodiscovery.service.spec.ts @@ -29,6 +29,8 @@ import { CloudApiUnauthorizedException } from 'src/modules/cloud/common/exceptio import { CloudUserCapiService } from 'src/modules/cloud/user/cloud-user.capi.service'; import { CloudSubscriptionCapiService } from 'src/modules/cloud/subscription/cloud-subscription.capi.service'; import { CloudDatabaseCapiService } from 'src/modules/cloud/database/cloud-database.capi.service'; +import ERROR_MESSAGES from 'src/constants/error-messages'; +import { CustomErrorCodes } from 'src/constants'; describe('CloudAutodiscoveryService', () => { let service: CloudAutodiscoveryService; @@ -364,5 +366,62 @@ describe('CloudAutodiscoveryService', () => { mockImportCloudDatabaseResponseFixed, ]); }); + it.each([ + { + description: 'null', + publicEndpoint: null, + }, + { + description: 'undefined', + publicEndpoint: undefined, + }, + { + description: 'empty string', + publicEndpoint: '', + }, + { + description: 'whitespace only', + publicEndpoint: ' ', + }, + { + description: 'does not contain colon', + publicEndpoint: 'hostname', + }, + ])( + 'should return error when publicEndpoint is $description', + async ({ publicEndpoint }) => { + cloudDatabaseCapiService.getDatabase.mockResolvedValueOnce({ + ...mockCloudDatabase, + publicEndpoint, + status: CloudDatabaseStatus.Active, + }); + + const result = await service.addRedisCloudDatabases( + mockSessionMetadata, + mockCloudCapiAuthDto, + [mockImportCloudDatabaseDto], + ); + + expect(result).toEqual([ + { + ...mockImportCloudDatabaseResponse, + status: ActionStatus.Fail, + message: ERROR_MESSAGES.CLOUD_DATABASE_ENDPOINT_INVALID, + error: { + message: ERROR_MESSAGES.CLOUD_DATABASE_ENDPOINT_INVALID, + statusCode: 400, + error: 'CloudDatabaseEndpointInvalid', + errorCode: CustomErrorCodes.CloudDatabaseEndpointInvalid, + }, + databaseDetails: { + ...mockCloudDatabase, + publicEndpoint, + status: CloudDatabaseStatus.Active, + }, + }, + ]); + expect(databaseService.create).not.toHaveBeenCalled(); + }, + ); }); }); diff --git a/redisinsight/api/src/modules/cloud/autodiscovery/cloud-autodiscovery.service.ts b/redisinsight/api/src/modules/cloud/autodiscovery/cloud-autodiscovery.service.ts index b149f89fe9..e2c3b10ade 100644 --- a/redisinsight/api/src/modules/cloud/autodiscovery/cloud-autodiscovery.service.ts +++ b/redisinsight/api/src/modules/cloud/autodiscovery/cloud-autodiscovery.service.ts @@ -3,6 +3,7 @@ import { Logger, ServiceUnavailableException, } from '@nestjs/common'; +import { CloudDatabaseEndpointInvalidException } from 'src/modules/cloud/job/exceptions'; import { uniqBy } from 'lodash'; import ERROR_MESSAGES from 'src/constants/error-messages'; import { @@ -29,6 +30,7 @@ import { CloudDatabase, CloudDatabaseStatus, } from 'src/modules/cloud/database/models'; +import { isValidCloudDatabaseEndpoint } from 'src/modules/cloud/database/utils'; import config from 'src/utils/config'; const cloudConfig = config.get('cloud'); @@ -222,6 +224,16 @@ export class CloudAutodiscoveryService { databaseDetails: database, }; } + if (!isValidCloudDatabaseEndpoint(publicEndpoint)) { + const exception = new CloudDatabaseEndpointInvalidException(); + return { + ...dto, + status: ActionStatus.Fail, + message: exception.message, + error: exception?.getResponse(), + databaseDetails: database, + }; + } const [host, port] = publicEndpoint.split(':'); await this.databaseService.create(sessionMetadata, { diff --git a/redisinsight/api/src/modules/cloud/database/utils/cloud-data-converter.ts b/redisinsight/api/src/modules/cloud/database/utils/cloud-data-converter.ts index f9ea8d202e..edb7d18cf4 100644 --- a/redisinsight/api/src/modules/cloud/database/utils/cloud-data-converter.ts +++ b/redisinsight/api/src/modules/cloud/database/utils/cloud-data-converter.ts @@ -17,6 +17,14 @@ export function convertRedisCloudModuleName(name: string): string { return REDIS_CLOUD_MODULES_NAMES[name] ?? name; } +export function isValidCloudDatabaseEndpoint( + publicEndpoint: string | null | undefined, +): boolean { + return ( + !!publicEndpoint && !!publicEndpoint.trim() && publicEndpoint.includes(':') + ); +} + export const parseCloudDatabaseCapiResponse = ( database: ICloudCapiDatabase, tags: ICloudCapiDatabaseTag[], diff --git a/redisinsight/api/src/modules/cloud/job/exceptions/cloud-database-endpoint-invalid.exception.ts b/redisinsight/api/src/modules/cloud/job/exceptions/cloud-database-endpoint-invalid.exception.ts new file mode 100644 index 0000000000..e2aa16b403 --- /dev/null +++ b/redisinsight/api/src/modules/cloud/job/exceptions/cloud-database-endpoint-invalid.exception.ts @@ -0,0 +1,23 @@ +import { + HttpException, + HttpExceptionOptions, + HttpStatus, +} from '@nestjs/common'; +import ERROR_MESSAGES from 'src/constants/error-messages'; +import { CustomErrorCodes } from 'src/constants'; + +export class CloudDatabaseEndpointInvalidException extends HttpException { + constructor( + message = ERROR_MESSAGES.CLOUD_DATABASE_ENDPOINT_INVALID, + options?: HttpExceptionOptions, + ) { + const response = { + message, + statusCode: HttpStatus.BAD_REQUEST, + error: 'CloudDatabaseEndpointInvalid', + errorCode: CustomErrorCodes.CloudDatabaseEndpointInvalid, + }; + + super(response, response.statusCode, options); + } +} diff --git a/redisinsight/api/src/modules/cloud/job/exceptions/index.ts b/redisinsight/api/src/modules/cloud/job/exceptions/index.ts index 74a05861ba..1f7b1f1093 100644 --- a/redisinsight/api/src/modules/cloud/job/exceptions/index.ts +++ b/redisinsight/api/src/modules/cloud/job/exceptions/index.ts @@ -1,4 +1,5 @@ export * from './cloud-database-already-exists-free.exception'; +export * from './cloud-database-endpoint-invalid.exception'; export * from './cloud-database-import-forbidden.exception'; export * from './cloud-database-in-failed-state.exception'; export * from './cloud-database-in-unexpected-state.exception'; diff --git a/redisinsight/api/src/modules/cloud/job/jobs/create-free-database.cloud-job.ts b/redisinsight/api/src/modules/cloud/job/jobs/create-free-database.cloud-job.ts index d9c52767d0..d929d8596a 100644 --- a/redisinsight/api/src/modules/cloud/job/jobs/create-free-database.cloud-job.ts +++ b/redisinsight/api/src/modules/cloud/job/jobs/create-free-database.cloud-job.ts @@ -15,6 +15,7 @@ import { WaitForActiveDatabaseCloudJob } from 'src/modules/cloud/job/jobs/wait-f import { CloudJobName } from 'src/modules/cloud/job/constants'; import { CloudJobStatus, CloudJobStep } from 'src/modules/cloud/job/models'; import { + CloudDatabaseEndpointInvalidException, CloudDatabaseImportForbiddenException, CloudJobUnexpectedErrorException, CloudTaskNoResourceIdException, @@ -30,6 +31,7 @@ import { ClientContext, SessionMetadata } from 'src/common/models'; import { DatabaseInfoService } from 'src/modules/database/database-info.service'; import { FeatureService } from 'src/modules/feature/feature.service'; import { KnownFeatures } from 'src/modules/feature/constants'; +import { isValidCloudDatabaseEndpoint } from 'src/modules/cloud/database/utils'; const cloudConfig = config.get('cloud'); @@ -138,6 +140,10 @@ export class CreateFreeDatabaseCloudJob extends CloudJob { const { publicEndpoint, name, password } = cloudDatabase; + if (!isValidCloudDatabaseEndpoint(publicEndpoint)) { + throw new CloudDatabaseEndpointInvalidException(); + } + const [host, port] = publicEndpoint.split(':'); const database = await this.dependencies.databaseService.create( diff --git a/redisinsight/api/src/modules/cloud/job/jobs/import-free-database.cloud-job.ts b/redisinsight/api/src/modules/cloud/job/jobs/import-free-database.cloud-job.ts index 4a6d480239..8cb2744915 100644 --- a/redisinsight/api/src/modules/cloud/job/jobs/import-free-database.cloud-job.ts +++ b/redisinsight/api/src/modules/cloud/job/jobs/import-free-database.cloud-job.ts @@ -16,7 +16,11 @@ import { CloudCapiKeyService } from 'src/modules/cloud/capi-key/cloud-capi-key.s import { SessionMetadata } from 'src/common/models'; import { KnownFeatures } from 'src/modules/feature/constants'; import { FeatureService } from 'src/modules/feature/feature.service'; -import { CloudDatabaseImportForbiddenException } from 'src/modules/cloud/job/exceptions'; +import { + CloudDatabaseEndpointInvalidException, + CloudDatabaseImportForbiddenException, +} from 'src/modules/cloud/job/exceptions'; +import { isValidCloudDatabaseEndpoint } from 'src/modules/cloud/database/utils'; const cloudConfig = config.get('cloud'); @@ -82,6 +86,10 @@ export class ImportFreeDatabaseCloudJob extends CloudJob { const { publicEndpoint, name, password } = cloudDatabase; + if (!isValidCloudDatabaseEndpoint(publicEndpoint)) { + throw new CloudDatabaseEndpointInvalidException(); + } + const [host, port] = publicEndpoint.split(':'); const database = await this.dependencies.databaseService.create( diff --git a/redisinsight/ui/src/constants/customErrorCodes.ts b/redisinsight/ui/src/constants/customErrorCodes.ts index 3565a3d25d..e94540c132 100644 --- a/redisinsight/ui/src/constants/customErrorCodes.ts +++ b/redisinsight/ui/src/constants/customErrorCodes.ts @@ -47,6 +47,7 @@ export enum CustomErrorCodes { CloudJobNotFound = 11_113, CloudSubscriptionAlreadyExistsFree = 11_114, CloudDatabaseImportForbidden = 11_115, + CloudDatabaseEndpointInvalid = 11_116, // General database errors [11200, 11299] DatabaseAlreadyExists = 11_200, From d411e7ca96218ec292f96ef341e32e2730e950b1 Mon Sep 17 00:00:00 2001 From: Valentin Kirilov Date: Fri, 28 Nov 2025 13:58:51 +0200 Subject: [PATCH 2/4] docs: add guidelines for custom exceptions and parameterized tests - Introduced a section on custom exceptions, emphasizing their benefits for consistent error handling. - Added examples for creating custom exceptions in TypeScript. - Included a new section on parameterized tests using `it.each`, highlighting its advantages for maintainability and readability. --- .ai/rules/backend.md | 37 +++++++++++++++++++++++++++++++++++++ .ai/rules/commits.md | 1 + .ai/rules/testing.md | 24 ++++++++++++++++++++++++ 3 files changed, 62 insertions(+) diff --git a/.ai/rules/backend.md b/.ai/rules/backend.md index e8a373247d..cc8cbba168 100644 --- a/.ai/rules/backend.md +++ b/.ai/rules/backend.md @@ -141,6 +141,43 @@ Use appropriate exception types: - `ConflictException` - 409 - `InternalServerErrorException` - 500 +### Custom Exceptions + +**Prefer custom exceptions over generic ones** when you need: + +- Consistent error codes for frontend handling +- Specific error messages from constants +- Consistent error structure across the codebase + +Create custom exceptions following existing patterns: + +```typescript +// src/modules/feature/exceptions/feature-invalid.exception.ts +import { + HttpException, + HttpExceptionOptions, + HttpStatus, +} from '@nestjs/common'; +import ERROR_MESSAGES from 'src/constants/error-messages'; +import { CustomErrorCodes } from 'src/constants'; + +export class FeatureInvalidException extends HttpException { + constructor( + message = ERROR_MESSAGES.FEATURE_INVALID, + options?: HttpExceptionOptions, + ) { + const response = { + message, + statusCode: HttpStatus.BAD_REQUEST, + error: 'FeatureInvalid', + errorCode: CustomErrorCodes.FeatureInvalid, + }; + + super(response, response.statusCode, options); + } +} +``` + ### Error Logging ```typescript diff --git a/.ai/rules/commits.md b/.ai/rules/commits.md index 06d9ba9520..89acb8ecb6 100644 --- a/.ai/rules/commits.md +++ b/.ai/rules/commits.md @@ -62,6 +62,7 @@ chore: upgrade React to version 18.2 - Atomic changes (one logical change per commit) - Reference issue/ticket in body - Explain **why**, not just **what** +- **Keep it concise** - Don't list every file change in the body ```bash feat(ui): add user profile editing diff --git a/.ai/rules/testing.md b/.ai/rules/testing.md index 4eb3094c87..042e0ba6d0 100644 --- a/.ai/rules/testing.md +++ b/.ai/rules/testing.md @@ -422,6 +422,30 @@ jest.mock('uiSrc/services/api', () => ({ })); ``` +### Parameterized Tests with `it.each` + +**Use `it.each` for multiple tests with the same body but different inputs:** + +```typescript +// ✅ GOOD: Parameterized tests +it.each([ + { description: 'null', value: null }, + { description: 'undefined', value: undefined }, + { description: 'empty string', value: '' }, + { description: 'whitespace only', value: ' ' }, +])('should return error when input is $description', async ({ value }) => { + const result = await service.processInput(value); + expect(result.status).toBe('error'); +}); +``` + +**Benefits:** + +- DRY: Single test body shared across all cases +- Maintainability: Changes to test logic only need to be made once +- Readability: Test cases are clearly defined in a table +- Easier to extend: Adding new test cases is just adding a new row + ### Test Edge Cases Always test: From c386bae997950917b3a8f9168d7367d465a1a37f Mon Sep 17 00:00:00 2001 From: Valentin Kirilov Date: Mon, 1 Dec 2025 10:55:47 +0200 Subject: [PATCH 3/4] refactor(api): simplify the check for presence of database public endpoint re #RI-7778 --- .../cloud-autodiscovery.service.spec.ts | 12 ------------ .../autodiscovery/cloud-autodiscovery.service.ts | 3 +-- .../cloud/database/utils/cloud-data-converter.ts | 8 -------- .../cloud/job/jobs/create-free-database.cloud-job.ts | 3 +-- .../cloud/job/jobs/import-free-database.cloud-job.ts | 3 +-- 5 files changed, 3 insertions(+), 26 deletions(-) diff --git a/redisinsight/api/src/modules/cloud/autodiscovery/cloud-autodiscovery.service.spec.ts b/redisinsight/api/src/modules/cloud/autodiscovery/cloud-autodiscovery.service.spec.ts index 2fdc87a362..3b3450dd44 100644 --- a/redisinsight/api/src/modules/cloud/autodiscovery/cloud-autodiscovery.service.spec.ts +++ b/redisinsight/api/src/modules/cloud/autodiscovery/cloud-autodiscovery.service.spec.ts @@ -375,18 +375,6 @@ describe('CloudAutodiscoveryService', () => { description: 'undefined', publicEndpoint: undefined, }, - { - description: 'empty string', - publicEndpoint: '', - }, - { - description: 'whitespace only', - publicEndpoint: ' ', - }, - { - description: 'does not contain colon', - publicEndpoint: 'hostname', - }, ])( 'should return error when publicEndpoint is $description', async ({ publicEndpoint }) => { diff --git a/redisinsight/api/src/modules/cloud/autodiscovery/cloud-autodiscovery.service.ts b/redisinsight/api/src/modules/cloud/autodiscovery/cloud-autodiscovery.service.ts index e2c3b10ade..dd6ad844b3 100644 --- a/redisinsight/api/src/modules/cloud/autodiscovery/cloud-autodiscovery.service.ts +++ b/redisinsight/api/src/modules/cloud/autodiscovery/cloud-autodiscovery.service.ts @@ -30,7 +30,6 @@ import { CloudDatabase, CloudDatabaseStatus, } from 'src/modules/cloud/database/models'; -import { isValidCloudDatabaseEndpoint } from 'src/modules/cloud/database/utils'; import config from 'src/utils/config'; const cloudConfig = config.get('cloud'); @@ -224,7 +223,7 @@ export class CloudAutodiscoveryService { databaseDetails: database, }; } - if (!isValidCloudDatabaseEndpoint(publicEndpoint)) { + if (!publicEndpoint) { const exception = new CloudDatabaseEndpointInvalidException(); return { ...dto, diff --git a/redisinsight/api/src/modules/cloud/database/utils/cloud-data-converter.ts b/redisinsight/api/src/modules/cloud/database/utils/cloud-data-converter.ts index edb7d18cf4..f9ea8d202e 100644 --- a/redisinsight/api/src/modules/cloud/database/utils/cloud-data-converter.ts +++ b/redisinsight/api/src/modules/cloud/database/utils/cloud-data-converter.ts @@ -17,14 +17,6 @@ export function convertRedisCloudModuleName(name: string): string { return REDIS_CLOUD_MODULES_NAMES[name] ?? name; } -export function isValidCloudDatabaseEndpoint( - publicEndpoint: string | null | undefined, -): boolean { - return ( - !!publicEndpoint && !!publicEndpoint.trim() && publicEndpoint.includes(':') - ); -} - export const parseCloudDatabaseCapiResponse = ( database: ICloudCapiDatabase, tags: ICloudCapiDatabaseTag[], diff --git a/redisinsight/api/src/modules/cloud/job/jobs/create-free-database.cloud-job.ts b/redisinsight/api/src/modules/cloud/job/jobs/create-free-database.cloud-job.ts index d929d8596a..40b8a68276 100644 --- a/redisinsight/api/src/modules/cloud/job/jobs/create-free-database.cloud-job.ts +++ b/redisinsight/api/src/modules/cloud/job/jobs/create-free-database.cloud-job.ts @@ -31,7 +31,6 @@ import { ClientContext, SessionMetadata } from 'src/common/models'; import { DatabaseInfoService } from 'src/modules/database/database-info.service'; import { FeatureService } from 'src/modules/feature/feature.service'; import { KnownFeatures } from 'src/modules/feature/constants'; -import { isValidCloudDatabaseEndpoint } from 'src/modules/cloud/database/utils'; const cloudConfig = config.get('cloud'); @@ -140,7 +139,7 @@ export class CreateFreeDatabaseCloudJob extends CloudJob { const { publicEndpoint, name, password } = cloudDatabase; - if (!isValidCloudDatabaseEndpoint(publicEndpoint)) { + if (!publicEndpoint) { throw new CloudDatabaseEndpointInvalidException(); } diff --git a/redisinsight/api/src/modules/cloud/job/jobs/import-free-database.cloud-job.ts b/redisinsight/api/src/modules/cloud/job/jobs/import-free-database.cloud-job.ts index 8cb2744915..e3ebf1d0e8 100644 --- a/redisinsight/api/src/modules/cloud/job/jobs/import-free-database.cloud-job.ts +++ b/redisinsight/api/src/modules/cloud/job/jobs/import-free-database.cloud-job.ts @@ -20,7 +20,6 @@ import { CloudDatabaseEndpointInvalidException, CloudDatabaseImportForbiddenException, } from 'src/modules/cloud/job/exceptions'; -import { isValidCloudDatabaseEndpoint } from 'src/modules/cloud/database/utils'; const cloudConfig = config.get('cloud'); @@ -86,7 +85,7 @@ export class ImportFreeDatabaseCloudJob extends CloudJob { const { publicEndpoint, name, password } = cloudDatabase; - if (!isValidCloudDatabaseEndpoint(publicEndpoint)) { + if (!publicEndpoint) { throw new CloudDatabaseEndpointInvalidException(); } From 90f362cf142d2087339bf990b3d07cc8d57b8b09 Mon Sep 17 00:00:00 2001 From: Valentin Kirilov Date: Mon, 1 Dec 2025 15:57:57 +0200 Subject: [PATCH 4/4] fix(api): update error message for cloud database endpoint unavailability References: #RI-7778 --- redisinsight/api/src/constants/error-messages.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redisinsight/api/src/constants/error-messages.ts b/redisinsight/api/src/constants/error-messages.ts index 83e5a04e90..284147572d 100644 --- a/redisinsight/api/src/constants/error-messages.ts +++ b/redisinsight/api/src/constants/error-messages.ts @@ -145,7 +145,7 @@ export default { CLOUD_DATABASE_IMPORT_FORBIDDEN: 'Adding your Redis Cloud database to Redis Insight is disabled due to a setting restricting database connection management.', CLOUD_DATABASE_ENDPOINT_INVALID: - 'Database endpoint is not available or not fully provisioned yet.', + 'Database endpoint is unavailable. It may still be provisioning or has been disabled.', CLOUD_PLAN_NOT_FOUND_FREE: 'Unable to find free cloud plan', CLOUD_SUBSCRIPTION_ALREADY_EXISTS_FREE: 'Free subscription already exists', COMMON_DEFAULT_IMPORT_ERROR: 'Unable to import default data',