-
-
Notifications
You must be signed in to change notification settings - Fork 23.2k
fix: [FLOWISE-1] Unstable Redis, ECONNREFUSED 127.0.0.1 #5493
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
03ab5ca
c579108
1a9db88
beec968
c88ff12
2ef53dd
b6f9e28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,179 @@ | ||||||||||||||||||
| import { InternalFlowiseError } from '../../server/src/errors/internalFlowiseError' | ||||||||||||||||||
| import logger from '../../server/src/utils/logger' | ||||||||||||||||||
| import { MODE } from '../../server/src/Interface' | ||||||||||||||||||
| import { Redis } from 'ioredis' | ||||||||||||||||||
| import { StatusCodes } from 'http-status-codes' | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Class used to initialize and connect to Redis instance. | ||||||||||||||||||
| * | ||||||||||||||||||
| * Sync usage: | ||||||||||||||||||
| * const connector = new RedisConnector() | ||||||||||||||||||
| * const redis = connector.getRedisClient() | ||||||||||||||||||
| * | ||||||||||||||||||
| * Async usage: | ||||||||||||||||||
| * const connector = new RedisConnector() | ||||||||||||||||||
| * await connector.ready() // fully waits for Redis init | ||||||||||||||||||
| * const redis = connector.getRedisClient() | ||||||||||||||||||
| */ | ||||||||||||||||||
| export class RedisConnector { | ||||||||||||||||||
| /** | ||||||||||||||||||
| * @type {Redis} | ||||||||||||||||||
| */ | ||||||||||||||||||
| private redis!: Redis | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * @type {Record<string, unknown>} | ||||||||||||||||||
| */ | ||||||||||||||||||
| private connection!: Record<string, unknown> | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * @type {Promise<void>} | ||||||||||||||||||
| */ | ||||||||||||||||||
| private initPromise: Promise<void> | null = null | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Sync constructor | ||||||||||||||||||
| * | ||||||||||||||||||
| * @constructor | ||||||||||||||||||
| */ | ||||||||||||||||||
| constructor() {} | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Initializes Redis lazily (runs once). | ||||||||||||||||||
| * | ||||||||||||||||||
| * @returns {Promise<void>} | ||||||||||||||||||
| */ | ||||||||||||||||||
| private async init(): Promise<void> { | ||||||||||||||||||
| if (this.initPromise) return this.initPromise | ||||||||||||||||||
|
|
||||||||||||||||||
| this.initPromise = (async () => { | ||||||||||||||||||
| const keepAlive = | ||||||||||||||||||
| process.env.REDIS_KEEP_ALIVE && !isNaN(parseInt(process.env.REDIS_KEEP_ALIVE, 10)) | ||||||||||||||||||
| ? parseInt(process.env.REDIS_KEEP_ALIVE, 10) | ||||||||||||||||||
| : 0 | ||||||||||||||||||
|
|
||||||||||||||||||
| const tlsOptions = | ||||||||||||||||||
| process.env.REDIS_TLS === 'true' | ||||||||||||||||||
| ? { | ||||||||||||||||||
| cert: process.env.REDIS_CERT ? Buffer.from(process.env.REDIS_CERT, 'base64') : undefined, | ||||||||||||||||||
| key: process.env.REDIS_KEY ? Buffer.from(process.env.REDIS_KEY, 'base64') : undefined, | ||||||||||||||||||
| ca: process.env.REDIS_CA ? Buffer.from(process.env.REDIS_CA, 'base64') : undefined | ||||||||||||||||||
| } | ||||||||||||||||||
| : {} | ||||||||||||||||||
|
|
||||||||||||||||||
| switch (process.env.MODE) { | ||||||||||||||||||
| case MODE.QUEUE: | ||||||||||||||||||
| await this.initializeQueueMode(keepAlive, tlsOptions) | ||||||||||||||||||
| break | ||||||||||||||||||
|
|
||||||||||||||||||
| case MODE.MAIN: | ||||||||||||||||||
| throw new InternalFlowiseError(StatusCodes.INTERNAL_SERVER_ERROR, | ||||||||||||||||||
| `[server]: MODE ${process.env.MODE} not implemented` | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
| default: | ||||||||||||||||||
| throw new InternalFlowiseError(StatusCodes.INTERNAL_SERVER_ERROR, | ||||||||||||||||||
| `Unrecognized MODE - ${process.env.MODE}` | ||||||||||||||||||
| ) | ||||||||||||||||||
| } | ||||||||||||||||||
| })() | ||||||||||||||||||
|
|
||||||||||||||||||
| return this.initPromise | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Queue mode initialization. | ||||||||||||||||||
| * | ||||||||||||||||||
| * @param {number} keepAlive - Keep alive in milliseconds (see https://redis.github.io/ioredis/index.html#RedisOptions) | ||||||||||||||||||
| * @param {Record<string, unknown>} tlsOptions - Record with key-value pairs (see https://redis.github.io/ioredis/index.html#RedisOptions) | ||||||||||||||||||
| */ | ||||||||||||||||||
| private async initializeQueueMode(keepAlive: number, tlsOptions: Record<string, unknown>): Promise<void> { | ||||||||||||||||||
| if (process.env.REDIS_URL) { | ||||||||||||||||||
| logger.info('[server] Queue mode using REDIS_URL.') | ||||||||||||||||||
|
|
||||||||||||||||||
| tlsOptions.rejectUnauthorized = | ||||||||||||||||||
| !(process.env.REDIS_URL.startsWith('rediss://') && process.env.REDIS_TLS !== 'true') | ||||||||||||||||||
|
|
||||||||||||||||||
| this.connection = { | ||||||||||||||||||
| keepAlive, | ||||||||||||||||||
| tls: tlsOptions, | ||||||||||||||||||
| enableReadyCheck: true, | ||||||||||||||||||
| reconnectOnError: this.connectOnError.bind(this) | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| this.redis = new Redis(process.env.REDIS_URL, this.connection) | ||||||||||||||||||
|
|
||||||||||||||||||
| } else { | ||||||||||||||||||
| logger.info('[server] Queue mode using HOST or localhost.') | ||||||||||||||||||
|
|
||||||||||||||||||
| this.connection = { | ||||||||||||||||||
| host: process.env.REDIS_HOST ?? 'localhost', | ||||||||||||||||||
| port: parseInt(process.env.REDIS_PORT || '6379'), | ||||||||||||||||||
| username: process.env.REDIS_USERNAME || undefined, | ||||||||||||||||||
| password: process.env.REDIS_PASSWORD || undefined, | ||||||||||||||||||
| keepAlive, | ||||||||||||||||||
| tls: tlsOptions, | ||||||||||||||||||
| enableReadyCheck: true, | ||||||||||||||||||
| reconnectOnError: this.connectOnError.bind(this) | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| this.redis = new Redis(this.connection) | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| try { | ||||||||||||||||||
| await this.redis.connect() | ||||||||||||||||||
| } catch (err: any) { | ||||||||||||||||||
| logger.error(`[server]: Redis connection failed - ${err.message}`) | ||||||||||||||||||
| throw new InternalFlowiseError(StatusCodes.INTERNAL_SERVER_ERROR, err.message) | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Function to handle Redis failure, used as callback. | ||||||||||||||||||
| * https://redis.github.io/ioredis/interfaces/CommonRedisOptions.html#reconnectOnError | ||||||||||||||||||
| * @param {Error} err | ||||||||||||||||||
| * @returns {number} 1 - Always reconnect to Redis in case of errors (does not retry the failed command) | ||||||||||||||||||
| * @see https://redis.github.io/ioredis/interfaces/CommonRedisOptions.html#reconnectOnError | ||||||||||||||||||
| */ | ||||||||||||||||||
| private connectOnError(err: Error): number { | ||||||||||||||||||
| logger.error(`[server]: Redis connection error - ${err.message}`) | ||||||||||||||||||
| return 1 | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+139
to
+142
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment on line 136 states that this function does not retry the failed command. However, according to the This mismatch could lead to unexpected behavior, especially when dealing with non-idempotent commands. Please update the return value to align with the intended behavior described in the comment.
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrong feedback. The source code and documentation confirm the number 1 should be used for reconnection without command retry.
|
||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Sync-safe access: | ||||||||||||||||||
| * - If Redis isn't initialized: triggers async initialization. | ||||||||||||||||||
| * - Always returns the Redis instance synchronously. | ||||||||||||||||||
| * | ||||||||||||||||||
| * @returns {Redis} | ||||||||||||||||||
| */ | ||||||||||||||||||
| public getRedisClient(): Redis { | ||||||||||||||||||
| // Trigger async init if not yet started | ||||||||||||||||||
| void this.init() | ||||||||||||||||||
| return this.redis | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+151
to
+155
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method has a critical race condition. It calls To fix this, the A possible approach:
With such a change,
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The race condition is addressed via the ready() method which must be called during async requests as documented. For example: |
||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Fully async safe usage: | ||||||||||||||||||
| * await connector.ready() | ||||||||||||||||||
| * | ||||||||||||||||||
| * @returns {Promise<void>} | ||||||||||||||||||
| */ | ||||||||||||||||||
| public async ready(): Promise<void> { | ||||||||||||||||||
| await this.init() | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Sync-safe access | ||||||||||||||||||
| * | ||||||||||||||||||
| * @returns {Record<string, unknown>} | ||||||||||||||||||
| */ | ||||||||||||||||||
| public getRedisConnection(): Record<string, unknown> { | ||||||||||||||||||
| // Trigger async init if not yet started | ||||||||||||||||||
| void this.init() | ||||||||||||||||||
| return this.connection | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+172
to
+176
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to The fix is the same as for
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The race condition is addressed via the ready() method which must be called during async requests as documented. For example: |
||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| export default RedisConnector | ||||||||||||||||||

Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in
components, we can't import stuff fromserverusing relative path like this - it will likely cause circular dependency. Relative path imports can only be used if you're importing from the samecomponentsfolder - this is how it works in a monoreposearch for
isPathTraversalyou can see how a common function can be created incomponents, and exposed via index.ts, then later used in files insideserverSince you are trying to create a singleton Redis instance that can be used throughout the application, I recommend to move this to
server/src