diff --git a/redisinsight/api/src/constants/error-messages.ts b/redisinsight/api/src/constants/error-messages.ts index 7b20bd2e8c..681f535aff 100644 --- a/redisinsight/api/src/constants/error-messages.ts +++ b/redisinsight/api/src/constants/error-messages.ts @@ -106,6 +106,8 @@ export default { `Node ${node} not exist in OSS Cluster.`, REDIS_MODULE_IS_REQUIRED: (module: string) => `Required ${module} module is not loaded.`, + UNABLE_TO_LIST_VECTOR_SET_ELEMENTS: (commands: string[]) => + `Unable to list vector set elements. The following commands failed: ${commands.join(', ')}.`, APP_SETTINGS_NOT_FOUND: () => 'Could not find application settings.', SERVER_INFO_NOT_FOUND: () => 'Could not find server info.', INCREASE_MINIMUM_LIMIT: (count?: number) => diff --git a/redisinsight/api/src/modules/browser/vector-set/vector-set.service.spec.ts b/redisinsight/api/src/modules/browser/vector-set/vector-set.service.spec.ts index ff94da8b76..bbffc9097a 100644 --- a/redisinsight/api/src/modules/browser/vector-set/vector-set.service.spec.ts +++ b/redisinsight/api/src/modules/browser/vector-set/vector-set.service.spec.ts @@ -1,6 +1,7 @@ import { BadRequestException, ForbiddenException, + InternalServerErrorException, NotFoundException, } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; @@ -39,6 +40,7 @@ import { BrowserToolKeysCommands } from 'src/modules/browser/constants/browser-t import { VectorSetService } from 'src/modules/browser/vector-set/vector-set.service'; import { DatabaseClientFactory } from 'src/modules/database/providers/database.client.factory'; import { RedisFeature } from 'src/modules/redis/client'; +import ERROR_MESSAGES from 'src/constants/error-messages'; describe('VectorSetService', () => { const client = mockStandaloneRedisClient; @@ -326,8 +328,6 @@ describe('VectorSetService', () => { ]); beforeEach(() => { - client.isFeatureSupported = jest.fn().mockResolvedValue(true); - when(client.sendCommand) .calledWith([BrowserToolKeysCommands.Exists, mockDto.keyName]) .mockResolvedValue(true); @@ -357,9 +357,6 @@ describe('VectorSetService', () => { mockDto, ); - expect(client.isFeatureSupported).toHaveBeenCalledWith( - RedisFeature.VRangeCommand, - ); expect(client.sendCommand).toHaveBeenCalledWith([ BrowserToolVectorSetCommands.VRange, mockDto.keyName, @@ -465,8 +462,16 @@ describe('VectorSetService', () => { ).rejects.toThrow(ForbiddenException); }); - it('should fallback to VRANDMEMBER when VRANGE is not supported', async () => { - client.isFeatureSupported = jest.fn().mockResolvedValue(false); + it('should fallback to VRANDMEMBER when VRANGE is an unknown command', async () => { + when(client.sendCommand) + .calledWith([ + BrowserToolVectorSetCommands.VRange, + mockDto.keyName, + mockDto.start, + mockDto.end, + mockDto.count, + ]) + .mockRejectedValue(new Error("ERR unknown command 'VRANGE'")); when(client.sendCommand) .calledWith([ @@ -481,9 +486,6 @@ describe('VectorSetService', () => { mockDto, ); - expect(client.isFeatureSupported).toHaveBeenCalledWith( - RedisFeature.VRangeCommand, - ); expect(client.sendCommand).toHaveBeenCalledWith([ BrowserToolVectorSetCommands.VRandMember, mockDto.keyName, @@ -494,6 +496,62 @@ describe('VectorSetService', () => { expect(result.isPaginationSupported).toBe(false); }); + it('should not fallback and should surface the error when VRANGE fails with a non-command error', async () => { + when(client.sendCommand) + .calledWith([ + BrowserToolVectorSetCommands.VRange, + mockDto.keyName, + mockDto.start, + mockDto.end, + mockDto.count, + ]) + .mockRejectedValue({ + ...mockRedisNoPermError, + command: 'VRANGE', + }); + + await expect( + service.getElements(mockBrowserClientMetadata, mockDto), + ).rejects.toThrow(ForbiddenException); + + expect(client.sendCommand).not.toHaveBeenCalledWith([ + BrowserToolVectorSetCommands.VRandMember, + mockDto.keyName, + mockDto.count, + ]); + }); + + it('should throw a server error when both VRANGE and VRANDMEMBER are unknown commands', async () => { + when(client.sendCommand) + .calledWith([ + BrowserToolVectorSetCommands.VRange, + mockDto.keyName, + mockDto.start, + mockDto.end, + mockDto.count, + ]) + .mockRejectedValue(new Error("ERR unknown command 'VRANGE'")); + + when(client.sendCommand) + .calledWith([ + BrowserToolVectorSetCommands.VRandMember, + mockDto.keyName, + mockDto.count, + ]) + .mockRejectedValue(new Error("ERR unknown command 'VRANDMEMBER'")); + + await expect( + service.getElements(mockBrowserClientMetadata, mockDto), + ).rejects.toThrow( + new InternalServerErrorException( + ERROR_MESSAGES.UNABLE_TO_LIST_VECTOR_SET_ELEMENTS([ + BrowserToolVectorSetCommands.VRange, + BrowserToolVectorSetCommands.VRandMember, + ]), + ), + ); + }); + it('should keep attributes undefined when VGETATTR fails for an element', async () => { client.sendPipeline = jest .fn() diff --git a/redisinsight/api/src/modules/browser/vector-set/vector-set.service.ts b/redisinsight/api/src/modules/browser/vector-set/vector-set.service.ts index 53cf094017..890ab6a386 100644 --- a/redisinsight/api/src/modules/browser/vector-set/vector-set.service.ts +++ b/redisinsight/api/src/modules/browser/vector-set/vector-set.service.ts @@ -1,6 +1,12 @@ -import { BadRequestException, Injectable, Logger } from '@nestjs/common'; +import { + BadRequestException, + Injectable, + InternalServerErrorException, + Logger, +} from '@nestjs/common'; import { catchAclError, catchMultiTransactionError } from 'src/utils'; import { RedisErrorCodes } from 'src/constants'; +import ERROR_MESSAGES from 'src/constants/error-messages'; import { ReplyError } from 'src/models'; import { BrowserToolKeysCommands, @@ -135,7 +141,7 @@ export class VectorSetService { 'Getting elements of the VectorSet data type stored at key.', clientMetadata, ); - const { keyName, start, end, count } = dto; + const { keyName } = dto; const client: RedisClient = await this.databaseClientFactory.getOrCreateClient(clientMetadata); @@ -146,32 +152,8 @@ export class VectorSetService { keyName, ])) as number; - const isVRangeSupported = await client.isFeatureSupported( - RedisFeature.VRangeCommand, - ); - - let elementNames: string[]; - let nextCursor: string | undefined; - - if (isVRangeSupported) { - elementNames = (await client.sendCommand([ - BrowserToolVectorSetCommands.VRange, - keyName, - start || '-', - end || '+', - count, - ])) as string[]; - - if (elementNames.length === count && elementNames.length > 0) { - nextCursor = `(${elementNames[elementNames.length - 1]}`; - } - } else { - elementNames = (await client.sendCommand([ - BrowserToolVectorSetCommands.VRandMember, - keyName, - count, - ])) as string[]; - } + const { elementNames, nextCursor, isPaginationSupported } = + await this.listElementNames(client, dto); const elements: { name: string | Buffer; attributes?: string }[] = elementNames.map((name) => ({ name })); @@ -188,7 +170,7 @@ export class VectorSetService { keyName, total, nextCursor, - isPaginationSupported: isVRangeSupported, + isPaginationSupported, elements, }); } catch (error) { @@ -455,6 +437,91 @@ export class VectorSetService { } } + /** + * Resolve the element names for `getElements`, preferring the ordered, + * paginated VRANGE and falling back to the unordered VRANDMEMBER when VRANGE + * is unavailable. When neither command can list the elements, throws so the + * caller surfaces a server error. + */ + private async listElementNames( + client: RedisClient, + dto: GetVectorSetElementsDto, + ): Promise<{ + elementNames: string[]; + nextCursor?: string; + isPaginationSupported: boolean; + }> { + const { keyName, start, end, count } = dto; + + const vRangeCommand: RedisClientCommand = [ + BrowserToolVectorSetCommands.VRange, + keyName, + start || '-', + end || '+', + count, + ]; + const vRangeResult = await this.trySendCommand(client, vRangeCommand); + if (vRangeResult !== null) { + const nextCursor = + vRangeResult.length === count && vRangeResult.length > 0 + ? `(${vRangeResult[vRangeResult.length - 1]}` + : undefined; + + return { + elementNames: vRangeResult, + nextCursor, + isPaginationSupported: true, + }; + } + + const vRandMemberCommand: RedisClientCommand = [ + BrowserToolVectorSetCommands.VRandMember, + keyName, + count, + ]; + const vRandMemberResult = await this.trySendCommand( + client, + vRandMemberCommand, + ); + if (vRandMemberResult !== null) { + return { elementNames: vRandMemberResult, isPaginationSupported: false }; + } + + throw new InternalServerErrorException( + ERROR_MESSAGES.UNABLE_TO_LIST_VECTOR_SET_ELEMENTS([ + `${BrowserToolVectorSetCommands.VRange}`, + `${BrowserToolVectorSetCommands.VRandMember}`, + ]), + ); + } + + /** + * Send a command and return its reply, or `null` when the command itself is + * unavailable (unknown/unsupported on the connected Redis version or proxy) + * so callers can fall back. `null` is distinct from an empty reply, which is a + * valid result. Any other error (permission, bad arguments, transient) is + * rethrown so it surfaces to the caller instead of triggering a silent + * fallback. + */ + private async trySendCommand( + client: RedisClient, + command: RedisClientCommand, + ): Promise { + try { + return (await client.sendCommand(command)) as string[]; + } catch (error) { + if (this.isUnsupportedCommandError(error)) { + return null; + } + throw error; + } + } + + private isUnsupportedCommandError(error: unknown): boolean { + const message = (error as ReplyError)?.message ?? ''; + return message.includes(RedisErrorCodes.UnknownCommand); + } + /** * Translate a thrown Redis error into the appropriate HTTP exception: * `WRONGTYPE` becomes a 400 (the client targeted a non-vector-set key), and diff --git a/redisinsight/api/src/modules/redis/client/redis.client.ts b/redisinsight/api/src/modules/redis/client/redis.client.ts index 32fcdc07f6..5e151d25a8 100644 --- a/redisinsight/api/src/modules/redis/client/redis.client.ts +++ b/redisinsight/api/src/modules/redis/client/redis.client.ts @@ -57,7 +57,6 @@ export type RedisClientCommandReply = export enum RedisFeature { HashFieldsExpiration = 'HashFieldsExpiration', UnlinkCommand = 'UnlinkCommand', - VRangeCommand = 'VRangeCommand', VsimWithAttribs = 'VsimWithAttribs', ArrayCommands = 'ArrayCommands', } @@ -185,9 +184,6 @@ export abstract class RedisClient extends EventEmitter2 { case RedisFeature.UnlinkCommand: // UNLINK command was introduced in Redis 4.0.0 return this.isRedisVersionAtLeast('4.0.0'); - case RedisFeature.VRangeCommand: - // VRANGE command was introduced in Redis 8.4 - return this.isRedisVersionAtLeast('8.4'); case RedisFeature.VsimWithAttribs: // VSIM WITHATTRIBS option is broken on 8.0.0–8.0.2 and was fixed in 8.0.3 return this.isRedisVersionAtLeast('8.0.3');