From ee52eb4fe7a2c24ac56827c5457de6bc37f8802c Mon Sep 17 00:00:00 2001 From: booleanhunter Date: Sat, 16 May 2026 12:36:41 +0530 Subject: [PATCH] fix(redis): accept inferred client types in SearchIndex; reject RESP=3 Issue: Calling new SearchIndex(schema, client) previously failed to compile when client came directly from createClient or createCluster without an explicit type annotation. The inferred client's RESP generic resolved to the 2 | 3 union, which forces callers to manually pin RESP: 2 by doing createClient({ ..., RESP: 2 }) to satisfy TypeScript. Fix: - new SearchIndex(schema, client) now compiles when client is the inferred return of createClient/createCluster, with no explicit type annotation required at the call site. - throw a RedisVLError at SearchIndex construction for Clients configured with RESP: 3 - Add unit tests for the protocol guard and the RESP=3 rejection --- src/indexes/search-index.ts | 23 ++++--- src/redis/client-types.ts | 35 +++++++++++ src/redis/protocol.ts | 53 ++++++++++++++++ src/storage/base-storage.ts | 3 +- tests/unit/indexes/search-index.test.ts | 17 +++++ tests/unit/redis/protocol.test.ts | 82 +++++++++++++++++++++++++ 6 files changed, 202 insertions(+), 11 deletions(-) create mode 100644 src/redis/client-types.ts create mode 100644 src/redis/protocol.ts create mode 100644 tests/unit/redis/protocol.test.ts diff --git a/src/indexes/search-index.ts b/src/indexes/search-index.ts index c10a883..488f6c8 100644 --- a/src/indexes/search-index.ts +++ b/src/indexes/search-index.ts @@ -3,8 +3,10 @@ * Main search index class for managing Redis search indexes. */ -import type { RedisClientType, RedisClusterType } from 'redis'; import type { RediSearchSchema } from '@redis/search'; +import type { RedisClientType, RedisClusterType } from 'redis'; +import type { AnyRedisClient } from '../redis/client-types.js'; +import { assertSupportedProtocol } from '../redis/protocol.js'; import { IndexSchema } from '../schema/schema.js'; import { buildRedisVLSchemaFromRedisIndexInfo } from '../redis/index-info-parser.js'; import { BaseStorage, HashStorage, JsonStorage } from '../storage/index.js'; @@ -156,11 +158,7 @@ export class SearchIndex { * @throws {Error} If schema is not a valid IndexSchema instance * @throws {Error} If client is not provided */ - constructor( - schema: IndexSchema, - client: RedisClientType | RedisClusterType, - validateOnLoad = false - ) { + constructor(schema: IndexSchema, client: AnyRedisClient, validateOnLoad = false) { if (!(schema instanceof IndexSchema)) { throw new RedisVLError('Must provide a valid IndexSchema object'); } @@ -169,8 +167,12 @@ export class SearchIndex { throw new RedisVLError('Must provide a valid Redis client'); } + assertSupportedProtocol(client); + this.schema = schema; - this.client = client; + // Narrow the publicly-accepted wide client surface to the RESP=2 + // shape this library is implemented against. See client-types.ts. + this.client = client as RedisClientType | RedisClusterType; this.validateOnLoad = validateOnLoad; // Initialize appropriate storage based on storage type @@ -201,13 +203,14 @@ export class SearchIndex { */ static async fromExisting( name: string, - client: RedisClientType | RedisClusterType, + client: AnyRedisClient, validateOnLoad = false ): Promise { + assertSupportedProtocol(client); + const ftClient = client as RedisClientType | RedisClusterType; let info: Awaited>; try { - // Fetch index info from Redis - info = await client.ft.info(name); + info = await ftClient.ft.info(name); } catch (error) { const message = error instanceof Error ? error.message : String(error); throw new RedisVLError(`Failed to load index '${name}' via FT.INFO: ${message}`, { diff --git a/src/redis/client-types.ts b/src/redis/client-types.ts new file mode 100644 index 0000000..dfda426 --- /dev/null +++ b/src/redis/client-types.ts @@ -0,0 +1,35 @@ +/** + * Redis Client Type Definitions + * + * The bare `RedisClientType` / `RedisClusterType` aliases re-exported from + * the `redis` package default their `RESP` generic to `2`. When consumers + * call `createClient(...)` without an explicit annotation, TypeScript + * infers `RESP` as the full `RespVersions` union (`2 | 3`), making the + * inferred client unassignable to APIs typed against the bare alias. + * + * `AnyRedisClient` widens the public-facing client surface so any client + * produced by `createClient` / `createCluster` is accepted regardless of + * inferred RESP version, modules, functions, scripts, or type mapping. + */ + +import type { RedisClientType, RedisClusterType } from 'redis'; + +/** + * Public-facing union accepted by SearchIndex and storage constructors. + * + * The five generic parameters of `RedisClientType` / `RedisClusterType` + * (modules, functions, scripts, RESP version, type mapping) behave + * invariantly in @redis/client, so a bare `RedisClientType` cannot be + * assigned to `RedisClientType<…, RespVersions, …>` and vice versa. + * Filling each parameter with `any` is the standard idiom for "accept + * any specialization of this generic" and is necessary here so callers + * can pass either an inferred `createClient(...)` result or the bare + * alias without compile-time friction. Internal call sites narrow the + * value back to `RedisClientType | RedisClusterType` via a plain `as` + * cast so this widening never leaks beyond the public surface. + */ +export type AnyRedisClient = + // eslint-disable-next-line @typescript-eslint/no-explicit-any + | RedisClientType + // eslint-disable-next-line @typescript-eslint/no-explicit-any + | RedisClusterType; diff --git a/src/redis/protocol.ts b/src/redis/protocol.ts new file mode 100644 index 0000000..f6cab0d --- /dev/null +++ b/src/redis/protocol.ts @@ -0,0 +1,53 @@ +/** + * Runtime detection of the active RESP protocol version on a node-redis + * client or cluster, plus a guard that rejects RESP=3 with an explicit + * error. + * + * `@redis/search` does not yet ship stable RESP3 reply transformers for + * FT.* commands — every command is published with `unstableResp3: true` + * and `transformReply.3 = undefined`, meaning node-redis explicitly + * does not parse RESP3 replies for the search module. Until that + * changes upstream, RedisVL refuses RESP=3 clients at construction + * time rather than letting them fail later with confusing reply-shape + * errors. + * + * Last verified upstream: `@redis/search` 5.12.1 (transitive of the + * pinned `redis` dependency). Revisit when upstream stabilizes RESP3 + * reply parsing for the search module; at that point this guard + * should be removed. + */ + +import type { RespVersions } from 'redis'; +import type { AnyRedisClient } from './client-types.js'; +import { RedisVLError } from '../errors.js'; + +/** + * Read the active RESP protocol version from a node-redis client or + * cluster. Returns `2` when not explicitly set, mirroring node-redis's + * own default. + */ +export function getProtocolVersion(client: AnyRedisClient): RespVersions { + const standalone = (client as { options?: { RESP?: unknown } }).options; + const cluster = (client as { _options?: { RESP?: unknown } })._options; + const resp = standalone?.RESP ?? cluster?.RESP; + return resp === 3 ? 3 : 2; +} + +/** + * Throw a `RedisVLError` if the supplied client is configured for + * RESP=3. RedisVL only supports RESP=2 today; see the module-level + * comment for the upstream `@redis/search` constraint that drives + * this restriction. + */ +export function assertSupportedProtocol(client: AnyRedisClient): void { + if (getProtocolVersion(client) === 3) { + throw new RedisVLError( + 'RedisVL does not support RESP=3 clients yet. ' + + '`@redis/search` does not yet ship stable RESP3 reply transformers ' + + 'for FT.* commands, so index introspection and search results would ' + + 'have undefined shape. ' + + 'Use the default `RESP=2` (omit the option, or set `RESP: 2` explicitly) ' + + 'when calling `createClient` / `createCluster`.' + ); + } +} diff --git a/src/storage/base-storage.ts b/src/storage/base-storage.ts index b7cce11..a1855de 100644 --- a/src/storage/base-storage.ts +++ b/src/storage/base-storage.ts @@ -7,7 +7,8 @@ import { randomBytes } from 'crypto'; import { vectorElementByteLength } from '../redis/utils.js'; /** - * Redis client type (standalone or cluster) + * Redis client type (standalone or cluster) used internally by storage. + * Matches the RESP=2 reply shapes this library is implemented against. */ export type RedisClient = RedisClientType | RedisClusterType; diff --git a/tests/unit/indexes/search-index.test.ts b/tests/unit/indexes/search-index.test.ts index 4e3c845..403afa7 100644 --- a/tests/unit/indexes/search-index.test.ts +++ b/tests/unit/indexes/search-index.test.ts @@ -83,6 +83,23 @@ describe('SearchIndex', () => { new SearchIndex(schema, null as unknown as RedisClientType); }).toThrow(/Must provide a valid Redis client/); }); + + it('should throw RedisVLError when the client is configured for RESP=3', () => { + const resp3Client = { + ...mockClient, + options: { RESP: 3 }, + } as unknown as RedisClientType; + expect(() => new SearchIndex(schema, resp3Client)).toThrow(RedisVLError); + expect(() => new SearchIndex(schema, resp3Client)).toThrow(/RESP=3/); + }); + + it('should accept a client whose options omit RESP (default RESP=2)', () => { + const defaultClient = { + ...mockClient, + options: {}, + } as unknown as RedisClientType; + expect(() => new SearchIndex(schema, defaultClient)).not.toThrow(); + }); }); describe('create()', () => { diff --git a/tests/unit/redis/protocol.test.ts b/tests/unit/redis/protocol.test.ts new file mode 100644 index 0000000..22d974a --- /dev/null +++ b/tests/unit/redis/protocol.test.ts @@ -0,0 +1,82 @@ +import { describe, it, expect } from 'vitest'; +import { getProtocolVersion, assertSupportedProtocol } from '../../../src/redis/protocol.js'; +import { RedisVLError } from '../../../src/errors.js'; +import type { AnyRedisClient } from '../../../src/redis/client-types.js'; + +describe('getProtocolVersion', () => { + it('returns 2 when standalone client.options.RESP is 2', () => { + const client = { options: { RESP: 2 } } as unknown as AnyRedisClient; + expect(getProtocolVersion(client)).toBe(2); + }); + + it('returns 3 when standalone client.options.RESP is 3', () => { + const client = { options: { RESP: 3 } } as unknown as AnyRedisClient; + expect(getProtocolVersion(client)).toBe(3); + }); + + it('returns 2 when standalone client.options.RESP is undefined', () => { + const client = { options: {} } as unknown as AnyRedisClient; + expect(getProtocolVersion(client)).toBe(2); + }); + + it('returns 2 when standalone client has no options at all', () => { + const client = {} as unknown as AnyRedisClient; + expect(getProtocolVersion(client)).toBe(2); + }); + + it('returns 3 when cluster client._options.RESP is 3', () => { + const client = { _options: { RESP: 3 } } as unknown as AnyRedisClient; + expect(getProtocolVersion(client)).toBe(3); + }); + + it('returns 2 when cluster client._options.RESP is 2', () => { + const client = { _options: { RESP: 2 } } as unknown as AnyRedisClient; + expect(getProtocolVersion(client)).toBe(2); + }); + + it('returns 2 when cluster client._options.RESP is undefined', () => { + const client = { _options: {} } as unknown as AnyRedisClient; + expect(getProtocolVersion(client)).toBe(2); + }); + + it('prefers standalone options over cluster _options when both exist', () => { + const client = { + options: { RESP: 2 }, + _options: { RESP: 3 }, + } as unknown as AnyRedisClient; + expect(getProtocolVersion(client)).toBe(2); + }); +}); + +describe('assertSupportedProtocol', () => { + it('does not throw for RESP=2 standalone client', () => { + const client = { options: { RESP: 2 } } as unknown as AnyRedisClient; + expect(() => assertSupportedProtocol(client)).not.toThrow(); + }); + + it('does not throw when RESP is unset (defaults to 2)', () => { + const client = { options: {} } as unknown as AnyRedisClient; + expect(() => assertSupportedProtocol(client)).not.toThrow(); + }); + + it('does not throw for RESP=2 cluster client', () => { + const client = { _options: { RESP: 2 } } as unknown as AnyRedisClient; + expect(() => assertSupportedProtocol(client)).not.toThrow(); + }); + + it('throws RedisVLError for RESP=3 standalone client', () => { + const client = { options: { RESP: 3 } } as unknown as AnyRedisClient; + expect(() => assertSupportedProtocol(client)).toThrow(RedisVLError); + }); + + it('throws RedisVLError for RESP=3 cluster client', () => { + const client = { _options: { RESP: 3 } } as unknown as AnyRedisClient; + expect(() => assertSupportedProtocol(client)).toThrow(RedisVLError); + }); + + it('error message names the upstream @redis/search unstableResp3 constraint', () => { + const client = { options: { RESP: 3 } } as unknown as AnyRedisClient; + expect(() => assertSupportedProtocol(client)).toThrow(/RESP=3/); + expect(() => assertSupportedProtocol(client)).toThrow(/unstableResp3|@redis\/search/); + }); +});