From 79749f24618baa7d977470372cafaa4b747f0837 Mon Sep 17 00:00:00 2001 From: Nikolay Karadzhov Date: Mon, 7 Jul 2025 11:06:34 +0300 Subject: [PATCH] fix(sentinel): propagate RESP option to clients (#3011) `createSentinel` takes RESP as an option, but does not propagate down to the actual clients. This creates confusion for the users as they expect the option to be set to all clients, which is reasonable. In case of clientSideCaching, this problem manifests as validation failure because clientSideCaching requires RESP3, but if we dont propagate, clients start with the default RESP2 fixes #3010 --- packages/client/lib/sentinel/index.spec.ts | 23 ++++++++++++++-------- packages/client/lib/sentinel/index.ts | 5 +++++ packages/test-utils/lib/index.ts | 2 ++ 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/packages/client/lib/sentinel/index.spec.ts b/packages/client/lib/sentinel/index.spec.ts index a9bdc8cc95..ef1702eab1 100644 --- a/packages/client/lib/sentinel/index.spec.ts +++ b/packages/client/lib/sentinel/index.spec.ts @@ -23,7 +23,7 @@ describe('RedisSentinel', () => { { host: 'localhost', port: 26379 } ] }; - + it('should throw error when clientSideCache is enabled with RESP 2', () => { assert.throws( () => RedisSentinel.create({ @@ -46,7 +46,7 @@ describe('RedisSentinel', () => { }); it('should not throw when clientSideCache is enabled with RESP 3', () => { - assert.doesNotThrow(() => + assert.doesNotThrow(() => RedisSentinel.create({ ...options, clientSideCache: clientSideCacheConfig, @@ -54,6 +54,16 @@ describe('RedisSentinel', () => { }) ); }); + + testUtils.testWithClientSentinel('should successfully connect to sentinel', async () => { + }, { + ...GLOBAL.SENTINEL.OPEN, + sentinelOptions: { + RESP: 3, + clientSideCache: { ttl: 0, maxEntries: 0, evictPolicy: 'LRU'}, + }, + }) + }); }); }); @@ -417,7 +427,7 @@ async function steadyState(frame: SentinelFramework) { sentinel.setTracer(tracer); await sentinel.connect(); await nodePromise; - + await sentinel.flushAll(); } finally { if (sentinel !== undefined) { @@ -443,7 +453,7 @@ describe('legacy tests', () => { this.timeout(15000); last = Date.now(); - + function deltaMeasurer() { const delta = Date.now() - last; if (delta > longestDelta) { @@ -508,7 +518,7 @@ describe('legacy tests', () => { } stopMeasuringBlocking = true; - + await frame.cleanup(); }) @@ -1032,6 +1042,3 @@ describe('legacy tests', () => { }) }); }); - - - diff --git a/packages/client/lib/sentinel/index.ts b/packages/client/lib/sentinel/index.ts index ec570e64bf..b4a794b871 100644 --- a/packages/client/lib/sentinel/index.ts +++ b/packages/client/lib/sentinel/index.ts @@ -625,6 +625,7 @@ class RedisSentinelInternal< readonly #sentinelClientOptions: RedisClientOptions; readonly #scanInterval: number; readonly #passthroughClientErrorEvents: boolean; + readonly #RESP?: RespVersions; #anotherReset = false; @@ -673,6 +674,7 @@ class RedisSentinelInternal< this.#name = options.name; + this.#RESP = options.RESP; this.#sentinelRootNodes = Array.from(options.sentinelRootNodes); this.#maxCommandRediscovers = options.maxCommandRediscovers ?? 16; this.#masterPoolSize = options.masterPoolSize ?? 1; @@ -716,6 +718,9 @@ class RedisSentinelInternal< #createClient(node: RedisNode, clientOptions: RedisClientOptions, reconnectStrategy?: undefined | false) { return RedisClient.create({ + //first take the globally set RESP + RESP: this.#RESP, + //then take the client options, which can in theory overwrite it ...clientOptions, socket: { ...clientOptions.socket, diff --git a/packages/test-utils/lib/index.ts b/packages/test-utils/lib/index.ts index a41f970e0c..43dd4debfd 100644 --- a/packages/test-utils/lib/index.ts +++ b/packages/test-utils/lib/index.ts @@ -337,6 +337,7 @@ export default class TestUtils { port: promise.port })); + const sentinel = createSentinel({ name: 'mymaster', sentinelRootNodes: rootNodes, @@ -352,6 +353,7 @@ export default class TestUtils { functions: options?.functions || {}, masterPoolSize: options?.masterPoolSize || undefined, reserveClient: options?.reserveClient || false, + ...options?.sentinelOptions }) as RedisSentinelType; if (options.disableClientSetup) {