From 56281ece3007a03fc32031b92f5d64b903d09842 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 10 Jun 2026 08:59:13 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=B0=20ci:=20Close=20Leaked=20Redis=20C?= =?UTF-8?q?lients=20in=20Cache=20Integration=20Tests=20(#13649)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ๐Ÿงน fix: Close Leaked Redis Clients in Cache Integration Tests Importing `redisClients` constructs and connects BOTH `ioredisClient` and `keyvRedisClient` as module side effects, but most cache/mcp integration specs disconnected at most one of them โ€” and specs that re-import the module per test via `jest.resetModules()` leaked a fresh pair of connected clients (sockets + ping timers) for every test. On runners where jest resolves to a single worker (2-core machines with `maxWorkers: '50%'`), the suite runs in-band and the leaked handles keep the main process alive after all tests pass โ€” the run hangs until the CI job timeout. On larger runners jest recovers only by force-exiting the leaked worker ("A worker process has failed to exit gracefully..."). - add a `closeRedisClients()` test helper that settles the connect promise and closes both clients of a `redisClients` module instance - call it from every cache/mcp integration spec that creates clients, mirroring what LeaderElection.cache_integration.spec.ts already does - remove the rethrow in the `keyvRedisClientReady.catch(...)` logging handler โ€” rethrowing inside `.catch` creates a new, never-observed rejected promise, turning any failed initial connect into a guaranteed unhandled rejection; callers awaiting `keyvRedisClientReady` still observe the original rejection All four `test:cache-integration` stages now pass AND exit cleanly with `--maxWorkers=1` against both single-node and cluster Redis, with no force-exit warning in worker mode. * ๐Ÿงน chore: Treat testRedisOperations as Assertion in expect-expect Rule * ๐Ÿ—‚๏ธ chore: Sort Imports per Repo Convention --- .../limiterCache.cache_integration.spec.ts | 2 ++ .../sessionCache.cache_integration.spec.ts | 2 ++ .../standardCache.cache_integration.spec.ts | 2 ++ .../violationCache.cache_integration.spec.ts | 3 ++ .../redisClients.cache_integration.spec.ts | 29 +++++-------------- .../cache/__tests__/redisClients.helper.ts | 25 ++++++++++++++++ .../redisUtils.cache_integration.spec.ts | 5 ++-- packages/api/src/cache/redisClients.ts | 5 ++-- ...rversInitializer.cache_integration.spec.ts | 13 ++++++--- ...PServersRegistry.cache_integration.spec.ts | 7 +++-- ...istryStatusCache.cache_integration.spec.ts | 5 ++-- ...onfigsCacheRedis.cache_integration.spec.ts | 9 ++++-- ...edisAggregateKey.cache_integration.spec.ts | 3 +- 13 files changed, 71 insertions(+), 39 deletions(-) create mode 100644 packages/api/src/cache/__tests__/redisClients.helper.ts diff --git a/packages/api/src/cache/__tests__/cacheFactory/limiterCache.cache_integration.spec.ts b/packages/api/src/cache/__tests__/cacheFactory/limiterCache.cache_integration.spec.ts index 52cfee1f07..9ccc830a7b 100644 --- a/packages/api/src/cache/__tests__/cacheFactory/limiterCache.cache_integration.spec.ts +++ b/packages/api/src/cache/__tests__/cacheFactory/limiterCache.cache_integration.spec.ts @@ -1,4 +1,5 @@ import type { RedisStore } from 'rate-limit-redis'; +import { closeRedisClients } from '../redisClients.helper'; describe('limiterCache', () => { let originalEnv: NodeJS.ProcessEnv; @@ -20,6 +21,7 @@ describe('limiterCache', () => { }); afterEach(async () => { + await closeRedisClients(); process.env = originalEnv; jest.resetModules(); }); diff --git a/packages/api/src/cache/__tests__/cacheFactory/sessionCache.cache_integration.spec.ts b/packages/api/src/cache/__tests__/cacheFactory/sessionCache.cache_integration.spec.ts index 99c0d69b37..a16556f75d 100644 --- a/packages/api/src/cache/__tests__/cacheFactory/sessionCache.cache_integration.spec.ts +++ b/packages/api/src/cache/__tests__/cacheFactory/sessionCache.cache_integration.spec.ts @@ -1,5 +1,6 @@ import type { MemoryStore, SessionData } from 'express-session'; import type { RedisStore as ConnectRedis } from 'connect-redis'; +import { closeRedisClients } from '../redisClients.helper'; interface TestSessionData { [key: string]: unknown; @@ -49,6 +50,7 @@ describe('sessionCache', () => { }); afterEach(async () => { + await closeRedisClients(); process.env = originalEnv; jest.resetModules(); }); diff --git a/packages/api/src/cache/__tests__/cacheFactory/standardCache.cache_integration.spec.ts b/packages/api/src/cache/__tests__/cacheFactory/standardCache.cache_integration.spec.ts index 563b52341a..325bcd3e0f 100644 --- a/packages/api/src/cache/__tests__/cacheFactory/standardCache.cache_integration.spec.ts +++ b/packages/api/src/cache/__tests__/cacheFactory/standardCache.cache_integration.spec.ts @@ -1,4 +1,5 @@ import type { Keyv } from 'keyv'; +import { closeRedisClients } from '../redisClients.helper'; // Mock GLOBAL_PREFIX_SEPARATOR from cacheConfig jest.mock('../../cacheConfig', () => { @@ -79,6 +80,7 @@ describe('standardCache', () => { testCache = null; } + await closeRedisClients(); process.env = originalEnv; jest.resetModules(); }); diff --git a/packages/api/src/cache/__tests__/cacheFactory/violationCache.cache_integration.spec.ts b/packages/api/src/cache/__tests__/cacheFactory/violationCache.cache_integration.spec.ts index 1978620c24..03254eb0b5 100644 --- a/packages/api/src/cache/__tests__/cacheFactory/violationCache.cache_integration.spec.ts +++ b/packages/api/src/cache/__tests__/cacheFactory/violationCache.cache_integration.spec.ts @@ -1,3 +1,5 @@ +import { closeRedisClients } from '../redisClients.helper'; + interface ViolationData { count?: number; timestamp?: number; @@ -57,6 +59,7 @@ describe('violationCache', () => { }); afterEach(async () => { + await closeRedisClients(); process.env = originalEnv; jest.resetModules(); }); diff --git a/packages/api/src/cache/__tests__/redisClients.cache_integration.spec.ts b/packages/api/src/cache/__tests__/redisClients.cache_integration.spec.ts index 77e8c01436..b3f1288117 100644 --- a/packages/api/src/cache/__tests__/redisClients.cache_integration.spec.ts +++ b/packages/api/src/cache/__tests__/redisClients.cache_integration.spec.ts @@ -1,5 +1,7 @@ -import type { Redis, Cluster } from 'ioredis'; +/* eslint jest/expect-expect: ["warn", { "assertFunctionNames": ["expect", "testRedisOperations"] }] */ import type { RedisClientType, RedisClusterType } from '@redis/client'; +import type { Redis, Cluster } from 'ioredis'; +import { closeRedisClients } from './redisClients.helper'; type RedisClient = RedisClientType | RedisClusterType | Redis | Cluster; @@ -64,27 +66,10 @@ describe('redisClients Integration Tests', () => { } } - // Cleanup Redis connections - if (ioredisClient) { - try { - if (ioredisClient.status === 'ready') { - ioredisClient.disconnect(); - } - } catch (error) { - console.warn('Error disconnecting ioredis client:', (error as Error).message); - } - ioredisClient = null; - } - - if (keyvRedisClient) { - try { - // Try to disconnect - keyv/redis client doesn't have an isReady property - await keyvRedisClient.disconnect(); - } catch (error) { - console.warn('Error disconnecting keyv redis client:', (error as Error).message); - } - keyvRedisClient = null; - } + // Close BOTH clients created by the module import, not just the one the test exercised + await closeRedisClients(); + ioredisClient = null; + keyvRedisClient = null; process.env = originalEnv; jest.resetModules(); diff --git a/packages/api/src/cache/__tests__/redisClients.helper.ts b/packages/api/src/cache/__tests__/redisClients.helper.ts new file mode 100644 index 0000000000..d9bca706cc --- /dev/null +++ b/packages/api/src/cache/__tests__/redisClients.helper.ts @@ -0,0 +1,25 @@ +export type RedisClientsModule = typeof import('~/cache/redisClients'); + +/** + * Closes the Redis clients owned by a `redisClients` module instance so the jest process can + * exit once the suite finishes. Importing `redisClients` constructs and connects both + * `ioredisClient` and `keyvRedisClient` as module side effects, so every instance created + * through `jest.resetModules()` + re-import must be closed or its sockets and timers keep the + * (in-band) test process alive after all tests pass. + * + * Pass the captured module object when `jest.resetModules()` already dropped the instance from + * the registry (e.g. closing a `beforeAll` instance from `afterAll`); otherwise the current + * registry instance is used. + */ +export async function closeRedisClients(clients?: RedisClientsModule): Promise { + const { ioredisClient, keyvRedisClient, keyvRedisClientReady } = + clients ?? (await import('~/cache/redisClients')); + + if (keyvRedisClientReady) { + await keyvRedisClientReady.catch(() => undefined); + } + if (keyvRedisClient?.isOpen) { + await keyvRedisClient.disconnect().catch(() => undefined); + } + ioredisClient?.disconnect(); +} diff --git a/packages/api/src/cache/__tests__/redisUtils.cache_integration.spec.ts b/packages/api/src/cache/__tests__/redisUtils.cache_integration.spec.ts index 659723c0c1..34e28a91e4 100644 --- a/packages/api/src/cache/__tests__/redisUtils.cache_integration.spec.ts +++ b/packages/api/src/cache/__tests__/redisUtils.cache_integration.spec.ts @@ -1,4 +1,5 @@ import { batchDeleteKeys, scanKeys } from '../redisUtils'; +import { closeRedisClients } from './redisClients.helper'; describe('redisUtils Integration Tests', () => { let keyvRedisClient: Awaited['keyvRedisClient']; @@ -44,8 +45,8 @@ describe('redisUtils Integration Tests', () => { }); afterAll(async () => { - // Close Redis connection - if (keyvRedisClient?.isOpen) await keyvRedisClient.disconnect(); + // Close both Redis clients created by the module import + await closeRedisClients(); }); describe('batchDeleteKeys', () => { diff --git a/packages/api/src/cache/redisClients.ts b/packages/api/src/cache/redisClients.ts index fca4365f7f..eef63689f5 100644 --- a/packages/api/src/cache/redisClients.ts +++ b/packages/api/src/cache/redisClients.ts @@ -1,9 +1,9 @@ import IoRedis from 'ioredis'; -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 type { RedisClientType, RedisClusterType } from '@redis/client'; +import type { Redis, Cluster } from 'ioredis'; import { cacheConfig } from './cacheConfig'; const urls = cacheConfig.REDIS_URI?.split(',').map((uri) => new URL(uri)) || []; @@ -217,7 +217,6 @@ if (cacheConfig.USE_REDIS) { keyvRedisClientReady.catch((err): void => { logger.error('@keyv/redis initial connection failed:', err); - throw err; }); } diff --git a/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.cache_integration.spec.ts b/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.cache_integration.spec.ts index 12d2c9091f..11b7d0cbb9 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.cache_integration.spec.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServersInitializer.cache_integration.spec.ts @@ -1,6 +1,8 @@ -import type * as t from '~/mcp/types'; -import type { MCPConnection } from '~/mcp/connection'; import type { MCPServersRegistry as MCPServersRegistryType } from '../MCPServersRegistry'; +import type { RedisClientsModule } from '~/cache/__tests__/redisClients.helper'; +import type { MCPConnection } from '~/mcp/connection'; +import type * as t from '~/mcp/types'; +import { closeRedisClients } from '~/cache/__tests__/redisClients.helper'; // Mock isLeader to always return true to avoid lock contention during parallel operations jest.mock('~/cluster', () => ({ @@ -28,6 +30,7 @@ describe('MCPServersInitializer Redis Integration Tests', () => { let MCPServerInspector: typeof import('../MCPServerInspector').MCPServerInspector; let MCPConnectionFactory: typeof import('~/mcp/MCPConnectionFactory').MCPConnectionFactory; let keyvRedisClient: Awaited['keyvRedisClient']; + let redisClientsModule: RedisClientsModule; let LeaderElection: typeof import('~/cluster/LeaderElection').LeaderElection; let leaderInstance: InstanceType; @@ -146,6 +149,7 @@ describe('MCPServersInitializer Redis Integration Tests', () => { MCPServerInspector = inspectorModule.MCPServerInspector; MCPConnectionFactory = connectionFactoryModule.MCPConnectionFactory; keyvRedisClient = redisClients.keyvRedisClient; + redisClientsModule = redisClients; LeaderElection = leaderElectionModule.LeaderElection; // Reset singleton and create new instance with mongoose @@ -232,8 +236,9 @@ describe('MCPServersInitializer Redis Integration Tests', () => { // Resign as leader if (leaderInstance) await leaderInstance.resign(); - // Close Redis connection - if (keyvRedisClient?.isOpen) await keyvRedisClient.disconnect(); + // Close both Redis clients; pass the captured module since + // `jest.resetModules()` in beforeEach dropped it from the registry + await closeRedisClients(redisClientsModule); }); describe('initialize()', () => { diff --git a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.cache_integration.spec.ts b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.cache_integration.spec.ts index 7752ff57d2..7b77a064b8 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.cache_integration.spec.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServersRegistry.cache_integration.spec.ts @@ -1,6 +1,7 @@ import { expect } from '@playwright/test'; -import type * as t from '~/mcp/types'; import type { MCPServersRegistry as MCPServersRegistryType } from '../MCPServersRegistry'; +import type * as t from '~/mcp/types'; +import { closeRedisClients } from '~/cache/__tests__/redisClients.helper'; // Mock ServerConfigsDB to avoid needing MongoDB for cache integration tests jest.mock('../db/ServerConfigsDB', () => ({ @@ -137,8 +138,8 @@ describe('MCPServersRegistry Redis Integration Tests', () => { // Resign as leader if (leaderInstance) await leaderInstance.resign(); - // Close Redis connection - if (keyvRedisClient?.isOpen) await keyvRedisClient.disconnect(); + // Close both Redis clients created by the module import + await closeRedisClients(); }); // Tests for the old privateServersCache API have been removed diff --git a/packages/api/src/mcp/registry/cache/__tests__/RegistryStatusCache.cache_integration.spec.ts b/packages/api/src/mcp/registry/cache/__tests__/RegistryStatusCache.cache_integration.spec.ts index 3cfd7f3578..2530e9a0f0 100644 --- a/packages/api/src/mcp/registry/cache/__tests__/RegistryStatusCache.cache_integration.spec.ts +++ b/packages/api/src/mcp/registry/cache/__tests__/RegistryStatusCache.cache_integration.spec.ts @@ -1,4 +1,5 @@ import { expect } from '@playwright/test'; +import { closeRedisClients } from '~/cache/__tests__/redisClients.helper'; describe('RegistryStatusCache Integration Tests', () => { let registryStatusCache: typeof import('../RegistryStatusCache').registryStatusCache; @@ -56,8 +57,8 @@ describe('RegistryStatusCache Integration Tests', () => { // Resign as leader if (leaderInstance) await leaderInstance.resign(); - // Close Redis connection - if (keyvRedisClient?.isOpen) await keyvRedisClient.disconnect(); + // Close both Redis clients created by the module import + await closeRedisClients(); }); describe('Initialization status tracking', () => { diff --git a/packages/api/src/mcp/registry/cache/__tests__/ServerConfigsCacheRedis.cache_integration.spec.ts b/packages/api/src/mcp/registry/cache/__tests__/ServerConfigsCacheRedis.cache_integration.spec.ts index 4e563ab4aa..c853becb18 100644 --- a/packages/api/src/mcp/registry/cache/__tests__/ServerConfigsCacheRedis.cache_integration.spec.ts +++ b/packages/api/src/mcp/registry/cache/__tests__/ServerConfigsCacheRedis.cache_integration.spec.ts @@ -1,9 +1,12 @@ import { expect } from '@playwright/test'; +import type { RedisClientsModule } from '~/cache/__tests__/redisClients.helper'; +import { closeRedisClients } from '~/cache/__tests__/redisClients.helper'; import { ParsedServerConfig } from '~/mcp/types'; describe('ServerConfigsCacheRedis Integration Tests', () => { let ServerConfigsCacheRedis: typeof import('../ServerConfigsCacheRedis').ServerConfigsCacheRedis; let keyvRedisClient: Awaited['keyvRedisClient']; + let redisClientsModule: RedisClientsModule; let cache: InstanceType; @@ -43,6 +46,7 @@ describe('ServerConfigsCacheRedis Integration Tests', () => { ServerConfigsCacheRedis = cacheModule.ServerConfigsCacheRedis; keyvRedisClient = redisClients.keyvRedisClient; + redisClientsModule = redisClients; // Ensure Redis is connected if (!keyvRedisClient) throw new Error('Redis client is not initialized'); @@ -75,8 +79,9 @@ describe('ServerConfigsCacheRedis Integration Tests', () => { }); afterAll(async () => { - // Close Redis connection - if (keyvRedisClient?.isOpen) await keyvRedisClient.disconnect(); + // Close both Redis clients; pass the captured module since + // `jest.resetModules()` in beforeEach dropped it from the registry + await closeRedisClients(redisClientsModule); }); describe('add and get operations', () => { diff --git a/packages/api/src/mcp/registry/cache/__tests__/ServerConfigsCacheRedisAggregateKey.cache_integration.spec.ts b/packages/api/src/mcp/registry/cache/__tests__/ServerConfigsCacheRedisAggregateKey.cache_integration.spec.ts index 4ec30187a2..eff9adb7df 100644 --- a/packages/api/src/mcp/registry/cache/__tests__/ServerConfigsCacheRedisAggregateKey.cache_integration.spec.ts +++ b/packages/api/src/mcp/registry/cache/__tests__/ServerConfigsCacheRedisAggregateKey.cache_integration.spec.ts @@ -1,5 +1,6 @@ import { expect } from '@playwright/test'; import type { ParsedServerConfig } from '~/mcp/types'; +import { closeRedisClients } from '~/cache/__tests__/redisClients.helper'; describe('ServerConfigsCacheRedisAggregateKey Integration Tests', () => { let ServerConfigsCacheRedisAggregateKey: typeof import('../ServerConfigsCacheRedisAggregateKey').ServerConfigsCacheRedisAggregateKey; @@ -56,7 +57,7 @@ describe('ServerConfigsCacheRedisAggregateKey Integration Tests', () => { }); afterAll(async () => { - if (keyvRedisClient?.isOpen) await keyvRedisClient.disconnect(); + await closeRedisClients(); }); describe('add and get operations', () => {