🚰 ci: Close Leaked Redis Clients in Cache Integration Tests (#13649)

* 🧹 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
This commit is contained in:
Danny Avila
2026-06-10 08:59:13 -04:00
committed by GitHub
parent da6b74e8eb
commit 56281ece30
13 changed files with 71 additions and 39 deletions

View File

@@ -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();
});

View File

@@ -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();
});

View File

@@ -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();
});

View File

@@ -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();
});

View File

@@ -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();

View File

@@ -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<void> {
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();
}

View File

@@ -1,4 +1,5 @@
import { batchDeleteKeys, scanKeys } from '../redisUtils';
import { closeRedisClients } from './redisClients.helper';
describe('redisUtils Integration Tests', () => {
let keyvRedisClient: Awaited<typeof import('../redisClients')>['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', () => {

View File

@@ -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;
});
}

View File

@@ -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<typeof import('~/cache/redisClients')>['keyvRedisClient'];
let redisClientsModule: RedisClientsModule;
let LeaderElection: typeof import('~/cluster/LeaderElection').LeaderElection;
let leaderInstance: InstanceType<typeof import('~/cluster/LeaderElection').LeaderElection>;
@@ -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()', () => {

View File

@@ -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

View File

@@ -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', () => {

View File

@@ -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<typeof import('~/cache/redisClients')>['keyvRedisClient'];
let redisClientsModule: RedisClientsModule;
let cache: InstanceType<typeof import('../ServerConfigsCacheRedis').ServerConfigsCacheRedis>;
@@ -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', () => {

View File

@@ -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', () => {