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: 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..284147572d 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 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', 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..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 @@ -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,50 @@ describe('CloudAutodiscoveryService', () => { mockImportCloudDatabaseResponseFixed, ]); }); + it.each([ + { + description: 'null', + publicEndpoint: null, + }, + { + description: 'undefined', + publicEndpoint: undefined, + }, + ])( + '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..dd6ad844b3 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 { @@ -222,6 +223,16 @@ export class CloudAutodiscoveryService { databaseDetails: database, }; } + if (!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/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..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 @@ -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, @@ -138,6 +139,10 @@ export class CreateFreeDatabaseCloudJob extends CloudJob { const { publicEndpoint, name, password } = cloudDatabase; + if (!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..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 @@ -16,7 +16,10 @@ 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'; const cloudConfig = config.get('cloud'); @@ -82,6 +85,10 @@ export class ImportFreeDatabaseCloudJob extends CloudJob { const { publicEndpoint, name, password } = cloudDatabase; + if (!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,