Initialize Redis Client in Classifier Service #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Hello @tasin2610 Kindly check my PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
package.json (1)
1-10: Add basic metadata and scripts to build/run the Redis testProject lacks name/license/scripts/engines. Add them to enable compile + test execution and ensure Node compatibility.
-{ - "dependencies": { - "dotenv": "^17.2.2", - "redis": "^5.8.2" - }, - "devDependencies": { - "@types/node": "^24.3.1", - "typescript": "^5.9.2" - } -} +{ + "name": "classifier-service", + "version": "0.1.0", + "private": true, + "license": "MIT", + "engines": { "node": ">=18.18.0" }, + "scripts": { + "build": "tsc -p tsconfig.json", + "start:redis-test": "node dist/src/redisTest.js" + }, + "dependencies": { + "dotenv": "^17.2.2", + "redis": "^5.8.2" + }, + "devDependencies": { + "@types/node": "^24.3.1", + "typescript": "^5.9.2" + } +}tsconfig.json (1)
1-9: Tighten TS config for Node service buildsSet rootDir to keep clean outputs; align target with modern Node; include Node types; add sensible defaults.
{ "compilerOptions": { - "module": "CommonJS", - "target": "ES6", - "outDir": "dist", - "esModuleInterop": true, - "strict": true + "module": "CommonJS", + "moduleResolution": "node", + "target": "ES2020", + "lib": ["ES2020"], + "rootDir": "src", + "outDir": "dist", + "types": ["node"], + "esModuleInterop": true, + "strict": true, + "skipLibCheck": true, + "resolveJsonModule": true } }src/redisTest.ts (1)
1-1: Remove unused import to avoid noise
getRedisClientisn’t used here.-import { initRedis, getRedisClient } from "./redisClient"; +import { initRedis } from "./redisClient";src/redisClient.ts (3)
6-8: Avoid brittle string replace for localhost; parse and normalize URLDirect
.replace("localhost", "127.0.0.1")can mangle credentials or non-host parts. Parse withURLand only adjust hostname.-// Prefer IPv4 loopback by default to avoid ::1/IPv6 resolution issues on some systems -const REDIS_URL = (process.env.REDIS_URL || "redis://127.0.0.1:6379").replace("localhost", "127.0.0.1"); +// Prefer IPv4 loopback by default to avoid ::1/IPv6 resolution issues on some systems +const buildRedisUrl = (raw: string): string => { + const fallback = "redis://127.0.0.1:6379"; + const val = raw && raw.trim() ? raw : fallback; + try { + const u = new URL(val); + if (u.hostname === "localhost") u.hostname = "127.0.0.1"; + return u.toString(); + } catch { + return fallback; + } +}; +const REDIS_URL = buildRedisUrl(process.env.REDIS_URL ?? "");
43-46: Guard against returning a closed clientIf the client was initialized but later closed,
getRedisClient()should throw.export function getRedisClient(): RedisClientType { - if (!client) throw new Error("Redis not initialized. Call initRedis() first."); - return client; + if (!client?.isOpen) { + throw new Error("Redis not initialized or not open. Call initRedis() first."); + } + return client; }
41-46: Provide a clean shutdown helper for servicesExpose
stopRedis()for graceful service termination.throw new Error("Redis connect failed"); } export function getRedisClient(): RedisClientType { if (!client) throw new Error("Redis not initialized. Call initRedis() first."); return client; } + +export async function stopRedis(): Promise<void> { + if (client?.isOpen) { + try { + await client.quit(); + } finally { + client = null; + } + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.json(1 hunks)src/redisClient.ts(1 hunks)src/redisTest.ts(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/redisTest.ts (1)
src/redisClient.ts (1)
initRedis(11-41)
🔇 Additional comments (1)
src/redisClient.ts (1)
16-18: Do not log secrets: mask credentials in URLs
console.log("Redis connected:", REDIS_URL)can leak username/password. Mask before logging.- client.on("error", (err: unknown) => { - console.error("Redis Client Error:", err); - }); + client.on("error", (err: unknown) => { + console.error("Redis Client Error:", err); + }); + + const maskUrl = (u: string): string => { + try { + const parsed = new URL(u); + if (parsed.password) parsed.password = "****"; + if (parsed.username) parsed.username = "****"; + return parsed.toString(); + } catch { + return u.replace(/\/\/[^@]*@/, "//****:****@"); + } + };Likely an incorrect or invalid review comment.
Comment on lines
+11
to
+15
| export async function initRedis(): Promise<RedisClientType> { | ||
| if (client && client.isOpen) return client; | ||
|
|
||
| client = createClient({ url: REDIS_URL }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent duplicate concurrent connections and set connection timeout
If two callers invoke initRedis() concurrently, you can create two clients. Also, set a connectTimeout and IPv4 family to ensure timely failure.
-let client: RedisClientType | null = null;
+let client: RedisClientType | null = null;
+let initPromise: Promise<RedisClientType> | null = null;
@@
export async function initRedis(): Promise<RedisClientType> {
- if (client && client.isOpen) return client;
+ if (client?.isOpen) return client;
+ if (initPromise) return initPromise;
- client = createClient({ url: REDIS_URL });
+ client = createClient({
+ url: REDIS_URL,
+ socket: {
+ // Ensure connect() rejects promptly on unreachable hosts
+ connectTimeout: 5_000,
+ family: 4
+ }
+ });
@@
- for (let attempt = 1; attempt <= maxRetries; attempt++) {
+ initPromise = (async () => {
+ for (let attempt = 1; attempt <= maxRetries; attempt++) {
try {
await client.connect();
- console.log("Redis connected:", REDIS_URL);
+ console.log("Redis connected:", maskUrl(REDIS_URL));
return client;
} catch (err) {
console.error(`Redis connect attempt ${attempt} failed:`, err);
if (attempt === maxRetries) {
- client = null;
- console.error("All Redis connection attempts failed.");
- throw err;
+ // Ensure resources are closed before dropping the reference
+ try { await client.disconnect(); } catch {}
+ client = null;
+ console.error("All Redis connection attempts failed.");
+ throw err;
}
const delay = baseDelayMs * 2 ** (attempt - 1);
await new Promise((res) => setTimeout(res, delay));
}
- }
- // unreachable, but satisfy TypeScript
- throw new Error("Redis connect failed");
+ }
+ // Unreachable
+ throw new Error("Redis connect failed");
+ })();
+ try {
+ return await initPromise;
+ } finally {
+ initPromise = null;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export async function initRedis(): Promise<RedisClientType> { | |
| if (client && client.isOpen) return client; | |
| client = createClient({ url: REDIS_URL }); | |
| let client: RedisClientType | null = null; | |
| let initPromise: Promise<RedisClientType> | null = null; | |
| export async function initRedis(): Promise<RedisClientType> { | |
| if (client?.isOpen) return client; | |
| if (initPromise) return initPromise; | |
| client = createClient({ | |
| url: REDIS_URL, | |
| socket: { | |
| // Ensure connect() rejects promptly on unreachable hosts | |
| connectTimeout: 5_000, | |
| family: 4 | |
| } | |
| }); | |
| initPromise = (async () => { | |
| for (let attempt = 1; attempt <= maxRetries; attempt++) { | |
| try { | |
| await client.connect(); | |
| console.log("Redis connected:", maskUrl(REDIS_URL)); | |
| return client; | |
| } catch (err) { | |
| console.error(`Redis connect attempt ${attempt} failed:`, err); | |
| if (attempt === maxRetries) { | |
| // Ensure resources are closed before dropping the reference | |
| try { await client.disconnect(); } catch {} | |
| client = null; | |
| console.error("All Redis connection attempts failed."); | |
| throw err; | |
| } | |
| const delay = baseDelayMs * 2 ** (attempt - 1); | |
| await new Promise((res) => setTimeout(res, delay)); | |
| } | |
| } | |
| // Unreachable | |
| throw new Error("Redis connect failed"); | |
| })(); | |
| try { | |
| return await initPromise; | |
| } finally { | |
| initPromise = null; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/redisClient.ts around lines 11-15, avoid creating duplicate clients on
concurrent initRedis() calls and add a connection timeout + IPv4 family:
introduce a module-scoped "connectingPromise" (Promise<RedisClientType> | null)
so initRedis first returns existing open client, then if connectingPromise
exists await and return it, otherwise set connectingPromise before creating the
client, pass socket options to createClient e.g. { url: REDIS_URL, socket: {
connectTimeout: <reasonable-ms>, family: 4 } }, await client.connect(), assign
the resolved client to the module-scoped client, clear connectingPromise in
finally or on error (re-throw error), and return the singleton client.
Comment on lines
+3
to
+12
| async function testRedis() { | ||
| const client = await initRedis(); | ||
|
|
||
| const pong = await client.ping(); | ||
| console.log("Ping Response:", pong); // Output: "PONG" means connection success | ||
|
|
||
| await client.disconnect(); | ||
| } | ||
|
|
||
| testRedis(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make the script fail-fast and exit cleanly; prefer QUIT over disconnect
Wrap in try/catch, set exit code, and use quit() for graceful shutdown.
async function testRedis() {
- const client = await initRedis();
-
- const pong = await client.ping();
- console.log("Ping Response:", pong); // Output: "PONG" means connection success
-
- await client.disconnect();
+ const client = await initRedis();
+ try {
+ const pong = await client.ping();
+ console.log("Ping Response:", pong); // "PONG" means connection success
+ } finally {
+ await client.quit();
+ }
}
-testRedis();
+testRedis().then(() => process.exit(0)).catch((err) => {
+ console.error("Redis test failed:", err);
+ process.exit(1);
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function testRedis() { | |
| const client = await initRedis(); | |
| const pong = await client.ping(); | |
| console.log("Ping Response:", pong); // Output: "PONG" means connection success | |
| await client.disconnect(); | |
| } | |
| testRedis(); | |
| async function testRedis() { | |
| const client = await initRedis(); | |
| try { | |
| const pong = await client.ping(); | |
| console.log("Ping Response:", pong); // "PONG" means connection success | |
| } finally { | |
| await client.quit(); | |
| } | |
| } | |
| testRedis() | |
| .then(() => process.exit(0)) | |
| .catch((err) => { | |
| console.error("Redis test failed:", err); | |
| process.exit(1); | |
| }); |
🤖 Prompt for AI Agents
In src/redisTest.ts around lines 3 to 12, the testRedis script should be made
fail-fast and shut down gracefully: wrap the logic in an async
try/catch/finally, create the Redis client before the try if needed, in try
await client.ping() and log the response, in catch log the error and set
process.exitCode = 1, and in finally if client exists await client.quit() (not
disconnect) to ensure graceful QUIT; after the async test call, await completion
and call process.exit(process.exitCode || 0) if you want an explicit exit.
tasin2610
approved these changes
Sep 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
src/redisClient.tsfor the classifier servicesrc/redisTest.tsto verify connection with the local Redis instanceSummary by CodeRabbit