Skip to content

Commit 2a97e77

Browse files
fix(server): handle Redis idle disconnects in session-store client (#20143)
## Summary The session-store node-redis client doesn't attach an `'error'` event listener, so when Redis closes an idle connection (server-side `timeout` setting), node-redis emits an unhandled `'error'` event and the entire Node process crashes with `SocketClosedUnexpectedlyError`. ## Reproduction 1. Deploy twenty-server against a Redis instance with `timeout 300` (5 min idle close). 2. Don't log in (or otherwise keep the session store completely idle). 3. ~5 minutes after `Nest application successfully started`, the process crashes: ``` node:events:487 throw er; // Unhandled 'error' event ^ SocketClosedUnexpectedlyError: Socket closed unexpectedly at Socket.<anonymous> (/app/node_modules/@redis/client/dist/lib/client/socket.js:194:118) ... Emitted 'error' event on Commander instance at: at RedisSocket._RedisSocket_onSocketError (/app/node_modules/@redis/client/dist/lib/client/socket.js:218:10) ``` Kubernetes restarts the pod and the loop repeats every ~5 minutes (12 restarts in 95 min in our environment). `twenty-worker` is unaffected — BullMQ's ioredis client has its own keep-alive and the queue keeps it busy. ## Root cause `packages/twenty-server/src/engine/core-modules/session-storage/session-storage.module-factory.ts` constructs the node-redis client with no error listener: ```ts const redisClient = createClient({ url: connectionString }); redisClient.connect().catch((err) => { throw new Error(`Redis connection failed: ${err}`); }); ``` In Node.js, an unhandled `'error'` event on an `EventEmitter` becomes an uncaught exception. node-redis emits `'error'` on socket close. With no listener, the process exits 1 — even though node-redis would otherwise reconnect on its own. ## Fix 1. Attach a `client.on('error', ...)` listener so disconnect errors are logged. node-redis' built-in `reconnectStrategy` then takes over. 2. Set `pingInterval: 60_000` so the connection is never idle long enough to be reaped by any reasonable Redis `timeout`. Defense in depth. ## Verification Reproduced locally with Redis `CONFIG SET timeout 30` (30s for fast reproduction). Without the fix: process exits 30s after boot. With the fix: client logs the disconnect, reconnects, and the process keeps running. ## Notes / out of scope - `cache-storage.module-factory.ts` uses `cache-manager-redis-yet` (which wraps node-redis under the hood). It may exhibit the same vulnerability under sufficiently idle conditions; recommend a follow-up to confirm and similarly harden it. - `redis-client.service.ts` uses ioredis, which has built-in keepalive and reconnect — no immediate crash risk, but adding error logging there would be a nice consistency win. ## Test plan - [ ] Existing tests still pass - [ ] Manual: deploy with low Redis `timeout` (e.g. `30`), confirm process survives - [ ] Manual: kill Redis briefly, confirm twenty-server reconnects instead of exiting --------- Co-authored-by: Charles Bochet <charles@twenty.com>
1 parent bbd9720 commit 2a97e77

2 files changed

Lines changed: 36 additions & 3 deletions

File tree

packages/twenty-server/src/engine/core-modules/cache-storage/cache-storage.module-factory.ts

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,17 @@
1+
import { Logger } from '@nestjs/common';
2+
13
import { type CacheModuleOptions } from '@nestjs/cache-manager';
24

3-
import { redisStore } from 'cache-manager-redis-yet';
5+
import { redisInsStore } from 'cache-manager-redis-yet';
6+
import { createClient } from 'redis';
47

58
import { CacheStorageType } from 'src/engine/core-modules/cache-storage/types/cache-storage-type.enum';
69
import { type TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
710

11+
const cacheStorageLogger = new Logger('CacheStorage');
12+
13+
const REDIS_PING_INTERVAL_MS = 60_000;
14+
815
export const cacheStorageModuleFactory = (
916
twentyConfigService: TwentyConfigService,
1017
): CacheModuleOptions => {
@@ -30,8 +37,23 @@ export const cacheStorageModuleFactory = (
3037

3138
return {
3239
...cacheModuleOptions,
33-
store: redisStore,
34-
url: redisUrl,
40+
store: async () => {
41+
const redisClient = createClient({
42+
url: redisUrl,
43+
pingInterval: REDIS_PING_INTERVAL_MS,
44+
});
45+
46+
redisClient.on('error', (err) => {
47+
cacheStorageLogger.error('Redis cache-storage client error', err);
48+
});
49+
50+
await redisClient.connect();
51+
52+
return redisInsStore(
53+
redisClient as Parameters<typeof redisInsStore>[0],
54+
{ ttl: cacheStorageTtl * 1000 },
55+
);
56+
},
3557
};
3658
}
3759
default:

packages/twenty-server/src/engine/core-modules/session-storage/session-storage.module-factory.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { createHash } from 'crypto';
22

3+
import { Logger } from '@nestjs/common';
4+
35
import RedisStore from 'connect-redis';
46
import { createClient } from 'redis';
57

@@ -8,6 +10,10 @@ import type session from 'express-session';
810
import { CacheStorageType } from 'src/engine/core-modules/cache-storage/types/cache-storage-type.enum';
911
import { type TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
1012

13+
const sessionStorageLogger = new Logger('SessionStorage');
14+
15+
const REDIS_PING_INTERVAL_MS = 60_000;
16+
1117
export const getSessionStorageOptions = (
1218
twentyConfigService: TwentyConfigService,
1319
): session.SessionOptions => {
@@ -57,6 +63,11 @@ export const getSessionStorageOptions = (
5763

5864
const redisClient = createClient({
5965
url: connectionString,
66+
pingInterval: REDIS_PING_INTERVAL_MS,
67+
});
68+
69+
redisClient.on('error', (err) => {
70+
sessionStorageLogger.error('Redis session-store client error', err);
6071
});
6172

6273
redisClient.connect().catch((err) => {

0 commit comments

Comments
 (0)