🐛 fix: Redis Cluster Bug + 🧪 Enhance Test Coverage (#10518)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run

*  feat: Implement scanIterator method for Redis cluster client
This resolves the bug where `ServerConfigsCacheRedis#getAll` returns an empty object when a Redis Cluster (instead of a single node server is used)

*  feat: Update cache integration tests for Redis cluster support
This commit is contained in:
Theo N. Truong 2025-11-16 09:58:52 -07:00 committed by GitHub
parent f228f2a91d
commit 8c531b921e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 81 additions and 134 deletions

View file

@ -23,6 +23,7 @@
"test:cache-integration:core": "jest --testPathPattern=\"src/cache/.*\\.cache_integration\\.spec\\.ts$\" --coverage=false",
"test:cache-integration:cluster": "jest --testPathPattern=\"src/cluster/.*\\.cache_integration\\.spec\\.ts$\" --coverage=false --runInBand",
"test:cache-integration:mcp": "jest --testPathPattern=\"src/mcp/.*\\.cache_integration\\.spec\\.ts$\" --coverage=false",
"test:cache-integration": "npm run test:cache-integration:core && npm run test:cache-integration:cluster && npm run test:cache-integration:mcp",
"verify": "npm run test:ci",
"b:clean": "bun run rimraf dist",
"b:build": "bun run b:clean && bun run rollup -c --silent --bundleConfigAsCjs",

View file

@ -7,17 +7,13 @@ describe('limiterCache', () => {
beforeEach(() => {
originalEnv = { ...process.env };
// Clear cache-related env vars
delete process.env.USE_REDIS;
delete process.env.REDIS_URI;
delete process.env.USE_REDIS_CLUSTER;
delete process.env.REDIS_PING_INTERVAL;
delete process.env.REDIS_KEY_PREFIX;
// Set test configuration
// Set test configuration with fallback defaults for local testing
process.env.REDIS_PING_INTERVAL = '0';
process.env.REDIS_KEY_PREFIX = 'Cache-Integration-Test';
process.env.REDIS_RETRY_MAX_ATTEMPTS = '5';
process.env.USE_REDIS = process.env.USE_REDIS || 'true';
process.env.USE_REDIS_CLUSTER = process.env.USE_REDIS_CLUSTER || 'false';
process.env.REDIS_URI = process.env.REDIS_URI || 'redis://127.0.0.1:6379';
// Clear require cache to reload modules
jest.resetModules();
@ -43,10 +39,6 @@ describe('limiterCache', () => {
});
test('should return RedisStore with sendCommand when USE_REDIS is true', async () => {
process.env.USE_REDIS = 'true';
process.env.USE_REDIS_CLUSTER = 'false';
process.env.REDIS_URI = 'redis://127.0.0.1:6379';
const cacheFactory = await import('../../cacheFactory');
const redisClients = await import('../../redisClients');
const { ioredisClient } = redisClients;

View file

@ -33,17 +33,13 @@ describe('sessionCache', () => {
beforeEach(() => {
originalEnv = { ...process.env };
// Clear cache-related env vars
delete process.env.USE_REDIS;
delete process.env.REDIS_URI;
delete process.env.USE_REDIS_CLUSTER;
delete process.env.REDIS_PING_INTERVAL;
delete process.env.REDIS_KEY_PREFIX;
// Set test configuration
// Set test configuration with fallback defaults for local testing
process.env.REDIS_PING_INTERVAL = '0';
process.env.REDIS_KEY_PREFIX = 'Cache-Integration-Test';
process.env.REDIS_RETRY_MAX_ATTEMPTS = '5';
process.env.USE_REDIS = process.env.USE_REDIS || 'true';
process.env.USE_REDIS_CLUSTER = process.env.USE_REDIS_CLUSTER || 'false';
process.env.REDIS_URI = process.env.REDIS_URI || 'redis://127.0.0.1:6379';
// Clear require cache to reload modules
jest.resetModules();
@ -55,10 +51,6 @@ describe('sessionCache', () => {
});
test('should return ConnectRedis store when USE_REDIS is true', async () => {
process.env.USE_REDIS = 'true';
process.env.USE_REDIS_CLUSTER = 'false';
process.env.REDIS_URI = 'redis://127.0.0.1:6379';
const cacheFactory = await import('../../cacheFactory');
const redisClients = await import('../../redisClients');
const { ioredisClient } = redisClients;
@ -138,10 +130,6 @@ describe('sessionCache', () => {
});
test('should handle namespace with and without trailing colon', async () => {
process.env.USE_REDIS = 'true';
process.env.USE_REDIS_CLUSTER = 'false';
process.env.REDIS_URI = 'redis://127.0.0.1:6379';
const cacheFactory = await import('../../cacheFactory');
const store1 = cacheFactory.sessionCache('namespace1');
@ -152,10 +140,6 @@ describe('sessionCache', () => {
});
test('should register error handler for Redis connection', async () => {
process.env.USE_REDIS = 'true';
process.env.USE_REDIS_CLUSTER = 'false';
process.env.REDIS_URI = 'redis://127.0.0.1:6379';
const cacheFactory = await import('../../cacheFactory');
const redisClients = await import('../../redisClients');
const { ioredisClient } = redisClients;
@ -173,10 +157,6 @@ describe('sessionCache', () => {
});
test('should handle session expiration with TTL', async () => {
process.env.USE_REDIS = 'true';
process.env.USE_REDIS_CLUSTER = 'false';
process.env.REDIS_URI = 'redis://127.0.0.1:6379';
const cacheFactory = await import('../../cacheFactory');
const redisClients = await import('../../redisClients');
const { ioredisClient } = redisClients;

View file

@ -30,18 +30,13 @@ describe('standardCache', () => {
beforeEach(() => {
originalEnv = { ...process.env };
// Clear cache-related env vars
delete process.env.USE_REDIS;
delete process.env.REDIS_URI;
delete process.env.USE_REDIS_CLUSTER;
delete process.env.REDIS_PING_INTERVAL;
delete process.env.REDIS_KEY_PREFIX;
delete process.env.FORCED_IN_MEMORY_CACHE_NAMESPACES;
// Set test configuration
// Set test configuration with fallback defaults for local testing
process.env.REDIS_PING_INTERVAL = '0';
process.env.REDIS_KEY_PREFIX = 'Cache-Integration-Test';
process.env.REDIS_RETRY_MAX_ATTEMPTS = '5';
process.env.USE_REDIS = process.env.USE_REDIS || 'true';
process.env.USE_REDIS_CLUSTER = 'false';
process.env.REDIS_URI = 'redis://127.0.0.1:6379';
// Clear require cache to reload modules
jest.resetModules();
@ -119,10 +114,6 @@ describe('standardCache', () => {
describe('when connecting to a Redis server', () => {
test('should handle different namespaces with correct prefixes', async () => {
process.env.USE_REDIS = 'true';
process.env.USE_REDIS_CLUSTER = 'false';
process.env.REDIS_URI = 'redis://127.0.0.1:6379';
const cacheFactory = await import('../../cacheFactory');
const cache1 = cacheFactory.standardCache('namespace-one');
@ -148,9 +139,6 @@ describe('standardCache', () => {
});
test('should respect FORCED_IN_MEMORY_CACHE_NAMESPACES', async () => {
process.env.USE_REDIS = 'true';
process.env.USE_REDIS_CLUSTER = 'false';
process.env.REDIS_URI = 'redis://127.0.0.1:6379';
process.env.FORCED_IN_MEMORY_CACHE_NAMESPACES = 'ROLES'; // Use a valid cache key
const cacheFactory = await import('../../cacheFactory');
@ -167,10 +155,6 @@ describe('standardCache', () => {
});
test('should handle TTL correctly', async () => {
process.env.USE_REDIS = 'true';
process.env.USE_REDIS_CLUSTER = 'false';
process.env.REDIS_URI = 'redis://127.0.0.1:6379';
const cacheFactory = await import('../../cacheFactory');
testCache = cacheFactory.standardCache('ttl-test', 1000); // 1 second TTL

View file

@ -26,17 +26,13 @@ describe('violationCache', () => {
beforeEach(() => {
originalEnv = { ...process.env };
// Clear cache-related env vars
delete process.env.USE_REDIS;
delete process.env.REDIS_URI;
delete process.env.USE_REDIS_CLUSTER;
delete process.env.REDIS_PING_INTERVAL;
delete process.env.REDIS_KEY_PREFIX;
// Set test configuration
// Set test configuration with fallback defaults for local testing
process.env.REDIS_PING_INTERVAL = '0';
process.env.REDIS_KEY_PREFIX = 'Cache-Integration-Test';
process.env.REDIS_RETRY_MAX_ATTEMPTS = '5';
process.env.USE_REDIS = process.env.USE_REDIS || 'true';
process.env.USE_REDIS_CLUSTER = process.env.USE_REDIS_CLUSTER || 'false';
process.env.REDIS_URI = process.env.REDIS_URI || 'redis://127.0.0.1:6379';
// Clear require cache to reload modules
jest.resetModules();
@ -48,10 +44,6 @@ describe('violationCache', () => {
});
test('should create violation cache with Redis when USE_REDIS is true', async () => {
process.env.USE_REDIS = 'true';
process.env.USE_REDIS_CLUSTER = 'false';
process.env.REDIS_URI = 'redis://127.0.0.1:6379';
const cacheFactory = await import('../../cacheFactory');
const redisClients = await import('../../redisClients');
const { ioredisClient } = redisClients;
@ -119,10 +111,6 @@ describe('violationCache', () => {
});
test('should respect namespace prefixing', async () => {
process.env.USE_REDIS = 'true';
process.env.USE_REDIS_CLUSTER = 'false';
process.env.REDIS_URI = 'redis://127.0.0.1:6379';
const cacheFactory = await import('../../cacheFactory');
const redisClients = await import('../../redisClients');
const { ioredisClient } = redisClients;
@ -157,10 +145,6 @@ describe('violationCache', () => {
});
test('should respect TTL settings', async () => {
process.env.USE_REDIS = 'true';
process.env.USE_REDIS_CLUSTER = 'false';
process.env.REDIS_URI = 'redis://127.0.0.1:6379';
const cacheFactory = await import('../../cacheFactory');
const redisClients = await import('../../redisClients');
const { ioredisClient } = redisClients;
@ -193,10 +177,6 @@ describe('violationCache', () => {
});
test('should handle complex violation data structures', async () => {
process.env.USE_REDIS = 'true';
process.env.USE_REDIS_CLUSTER = 'false';
process.env.REDIS_URI = 'redis://127.0.0.1:6379';
const cacheFactory = await import('../../cacheFactory');
const redisClients = await import('../../redisClients');
const { ioredisClient } = redisClients;

View file

@ -9,9 +9,13 @@ describe('redisClients Integration Tests', () => {
let keyvRedisClient: RedisClientType | RedisClusterType | null = null;
// Helper function to test set/get/delete operations
const testRedisOperations = async (client: RedisClient, keyPrefix: string): Promise<void> => {
// Wait cluster to fully initialize
await new Promise((resolve) => setTimeout(resolve, 1000));
const testRedisOperations = async (
client: RedisClient,
keyPrefix: string,
readyPromise?: Promise<void>,
): Promise<void> => {
// Wait for connection and topology discovery to complete
if (readyPromise) await readyPromise;
const testKey = `${keyPrefix}-test-key`;
const testValue = `${keyPrefix}-test-value`;
@ -35,18 +39,13 @@ describe('redisClients Integration Tests', () => {
beforeEach(() => {
originalEnv = { ...process.env };
// Clear Redis-related env vars
delete process.env.USE_REDIS;
delete process.env.REDIS_URI;
delete process.env.USE_REDIS_CLUSTER;
delete process.env.REDIS_PING_INTERVAL;
delete process.env.REDIS_KEY_PREFIX;
// Set common test configuration
process.env.REDIS_PING_INTERVAL = '0';
// Set common test configuration with fallback defaults for local testing
process.env.REDIS_PING_INTERVAL = '1000';
process.env.REDIS_KEY_PREFIX = 'Redis-Integration-Test';
process.env.REDIS_RETRY_MAX_ATTEMPTS = '5';
process.env.REDIS_PING_INTERVAL = '1000';
process.env.USE_REDIS = process.env.USE_REDIS || 'true';
process.env.USE_REDIS_CLUSTER = process.env.USE_REDIS_CLUSTER || 'false';
process.env.REDIS_URI = process.env.REDIS_URI || 'redis://127.0.0.1:6379';
// Clear module cache to reload module
jest.resetModules();
@ -105,10 +104,6 @@ describe('redisClients Integration Tests', () => {
describe('when connecting to a Redis instance', () => {
test('should connect and perform set/get/delete operations', async () => {
process.env.USE_REDIS = 'true';
process.env.USE_REDIS_CLUSTER = 'false';
process.env.REDIS_URI = 'redis://127.0.0.1:6379';
const clients = await import('../redisClients');
ioredisClient = clients.ioredisClient;
await testRedisOperations(ioredisClient!, 'ioredis-single');
@ -117,7 +112,6 @@ describe('redisClients Integration Tests', () => {
describe('when connecting to a Redis cluster', () => {
test('should connect to cluster and perform set/get/delete operations', async () => {
process.env.USE_REDIS = 'true';
process.env.USE_REDIS_CLUSTER = 'true';
process.env.REDIS_URI =
'redis://127.0.0.1:7001,redis://127.0.0.1:7002,redis://127.0.0.1:7003';
@ -142,26 +136,21 @@ describe('redisClients Integration Tests', () => {
describe('when connecting to a Redis instance', () => {
test('should connect and perform set/get/delete operations', async () => {
process.env.USE_REDIS = 'true';
process.env.USE_REDIS_CLUSTER = 'false';
process.env.REDIS_URI = 'redis://127.0.0.1:6379';
const clients = await import('../redisClients');
keyvRedisClient = clients.keyvRedisClient;
await testRedisOperations(keyvRedisClient!, 'keyv-single');
await testRedisOperations(keyvRedisClient!, 'keyv-single', clients.keyvRedisClientReady!);
});
});
describe('when connecting to a Redis cluster', () => {
test('should connect to cluster and perform set/get/delete operations', async () => {
process.env.USE_REDIS = 'true';
process.env.USE_REDIS_CLUSTER = 'true';
process.env.REDIS_URI =
'redis://127.0.0.1:7001,redis://127.0.0.1:7002,redis://127.0.0.1:7003';
const clients = await import('../redisClients');
keyvRedisClient = clients.keyvRedisClient;
await testRedisOperations(keyvRedisClient!, 'keyv-cluster');
await testRedisOperations(keyvRedisClient!, 'keyv-cluster', clients.keyvRedisClientReady!);
});
});
});

View file

@ -3,6 +3,7 @@ import type { Redis, Cluster } from 'ioredis';
import { logger } from '@librechat/data-schemas';
import { createClient, createCluster } from '@keyv/redis';
import type { RedisClientType, RedisClusterType } from '@redis/client';
import type { ScanCommandOptions } from '@redis/client/dist/lib/commands/SCAN';
import { cacheConfig } from './cacheConfig';
const urls = cacheConfig.REDIS_URI?.split(',').map((uri) => new URL(uri)) || [];
@ -121,6 +122,11 @@ if (cacheConfig.USE_REDIS) {
}
let keyvRedisClient: RedisClientType | RedisClusterType | null = null;
let keyvRedisClientReady:
| Promise<void>
| Promise<RedisClientType<Record<string, never>, Record<string, never>, Record<string, never>>>
| null = null;
if (cacheConfig.USE_REDIS) {
/**
* ** WARNING ** Keyv Redis client does not support Prefix like ioredis above.
@ -162,6 +168,22 @@ if (cacheConfig.USE_REDIS) {
defaults: redisOptions,
});
// Add scanIterator method to cluster client for API consistency with standalone client
if (!('scanIterator' in keyvRedisClient)) {
const clusterClient = keyvRedisClient as RedisClusterType;
(keyvRedisClient as unknown as RedisClientType).scanIterator = async function* (
options?: ScanCommandOptions,
) {
const masters = clusterClient.masters;
for (const master of masters) {
const nodeClient = await clusterClient.nodeClient(master);
for await (const key of nodeClient.scanIterator(options)) {
yield key;
}
}
};
}
keyvRedisClient.setMaxListeners(cacheConfig.REDIS_MAX_LISTENERS);
keyvRedisClient.on('error', (err) => {
@ -184,10 +206,13 @@ if (cacheConfig.USE_REDIS) {
logger.warn('@keyv/redis client disconnected');
});
keyvRedisClient.connect().catch((err) => {
// Start connection immediately
keyvRedisClientReady = keyvRedisClient.connect();
keyvRedisClientReady.catch((err): void => {
logger.error('@keyv/redis initial connection failed:', err);
throw err;
});
}
export { ioredisClient, keyvRedisClient };
export { ioredisClient, keyvRedisClient, keyvRedisClientReady };

View file

@ -25,10 +25,8 @@ describe('LeaderElection with Redis', () => {
throw new Error('Redis client is not initialized');
}
// Wait for Redis to be ready
if (!keyvRedisClient.isOpen) {
await keyvRedisClient.connect();
}
// Wait for connection and topology discovery to complete
await redisClients.keyvRedisClientReady;
// Increase max listeners to handle many instances in tests
process.setMaxListeners(200);

View file

@ -121,8 +121,8 @@ describe('MCPServersInitializer Redis Integration Tests', () => {
// Ensure Redis is connected
if (!keyvRedisClient) throw new Error('Redis client is not initialized');
// Wait for Redis to be ready
if (!keyvRedisClient.isOpen) await keyvRedisClient.connect();
// Wait for connection and topology discovery to complete
await redisClients.keyvRedisClientReady;
// Become leader so we can perform write operations
leaderInstance = new LeaderElection();

View file

@ -50,8 +50,8 @@ describe('MCPServersRegistry Redis Integration Tests', () => {
// Ensure Redis is connected
if (!keyvRedisClient) throw new Error('Redis client is not initialized');
// Wait for Redis to be ready
if (!keyvRedisClient.isOpen) await keyvRedisClient.connect();
// Wait for connection and topology discovery to complete
await redisClients.keyvRedisClientReady;
// Become leader so we can perform write operations
leaderInstance = new LeaderElection();

View file

@ -73,6 +73,8 @@ export class ServerConfigsCacheRedis extends BaseRegistryCache {
entries.push([keyName, value as ParsedServerConfig]);
}
}
} else {
throw new Error('Redis client with scanIterator not available.');
}
return fromPairs(entries);

View file

@ -25,8 +25,8 @@ describe('RegistryStatusCache Integration Tests', () => {
// Ensure Redis is connected
if (!keyvRedisClient) throw new Error('Redis client is not initialized');
// Wait for Redis to be ready
if (!keyvRedisClient.isOpen) await keyvRedisClient.connect();
// Wait for connection and topology discovery to complete
await redisClients.keyvRedisClientReady;
// Become leader so we can perform write operations
leaderInstance = new LeaderElection();

View file

@ -31,7 +31,10 @@ describe('ServerConfigsCacheRedis Integration Tests', () => {
beforeAll(async () => {
// Set up environment variables for Redis (only if not already set)
process.env.USE_REDIS = process.env.USE_REDIS ?? 'true';
process.env.REDIS_URI = process.env.REDIS_URI ?? 'redis://127.0.0.1:6379';
process.env.USE_REDIS_CLUSTER = process.env.USE_REDIS_CLUSTER ?? 'true';
process.env.REDIS_URI =
process.env.REDIS_URI ??
'redis://127.0.0.1:7001,redis://127.0.0.1:7002,redis://127.0.0.1:7003';
process.env.REDIS_KEY_PREFIX =
process.env.REDIS_KEY_PREFIX ?? 'ServerConfigsCacheRedis-IntegrationTest';
@ -49,8 +52,8 @@ describe('ServerConfigsCacheRedis Integration Tests', () => {
// Ensure Redis is connected
if (!keyvRedisClient) throw new Error('Redis client is not initialized');
// Wait for Redis to be ready
if (!keyvRedisClient.isOpen) await keyvRedisClient.connect();
// Wait for connection and topology discovery to complete
await redisClients.keyvRedisClientReady;
// Clear any existing leader key to ensure clean state
await keyvRedisClient.del(LeaderElection.LEADER_KEY);