Skip to content

[Fix] Enhance server.js with graceful shutdown, env validation, and improved error handling #111

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.

Sign up for GitHub

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

Open
zaibamachhaliya wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Fix] Enhance server.js with graceful shutdown, env validation, and improved error handling #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions backend/.env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
# Copy this file to .env and fill in your values
MONGO_URI=your_mongodb_connection_string_here
# Database
MONGO_URI=mongodb://localhost:27017/your_database_name

# Server
PORT=5001
NODE_ENV=development

# Redis
UPSTASH_REDIS_REST_URL=your_upstash_redis_url_here
UPSTASH_REDIS_REST_TOKEN=your_upstash_redis_token_here
JWT_SECRET=any_random_string_for_jwt_secret

# JWT
JWT_SECRET=any_random_string_for_jwt_secret

# CORS - Production ke liye
ALLOWED_ORIGINS=http://localhost:5173,http://localhost:3000
182 changes: 155 additions & 27 deletions backend/src/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import cookieParser from "cookie-parser";
import { fileURLToPath } from "url";
import dns from "dns";
import jwt from "jsonwebtoken";
import mongoose from "mongoose";

@coderabbitai coderabbitai Bot Jul 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

Redundant dynamic import("mongoose") despite static import.

mongoose is already statically imported at Line 9, but Lines 99 and 184 re-import it dynamically (await import("mongoose")) and access .default.connection. Since both resolve to the same cached module, this adds unnecessary async overhead on the health-check hot path (frequently polled by load balancers/orchestrators) and in the shutdown path, and creates a confusing shadowed mongoose variable.

♻️ Use the static import directly 
 app.get("/health", async (req, res) => {
   let mongodbStatus = "Disconnected";
   let redisStatus = "Disconnected";

   try {
-    const mongoose = await import("mongoose");
-    mongodbStatus =
-      mongoose.default.connection.readyState === 1
-        ? "Connected"
-        : "Disconnected";
+    mongodbStatus =
+      mongoose.connection.readyState === 1 ? "Connected" : "Disconnected";
   } catch (error) {
     mongodbStatus = "Error";
   }
   try {
-    const mongoose = await import("mongoose");
-    if (mongoose.default.connection.readyState === 1) {
-      await mongoose.default.connection.close();
+    if (mongoose.connection.readyState === 1) {
+      await mongoose.connection.close();
       console.log("MongoDB connection closed");
     }

Also applies to: 99-99, 184-184

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/server.js` at line 9, Remove the redundant dynamic mongoose
imports in the health-check and shutdown paths and use the existing statically
imported mongoose instance instead. Update the logic around the health-check
handler and the shutdown/cleanup code to reference mongoose.connection directly,
eliminating the extra await import("mongoose") calls and the shadowed local
mongoose variable.


// Routes
import notesRoutes from "./routes/notesRoutes.js";
Expand All @@ -25,24 +26,43 @@ dotenv.config();
dns.setServers(["1.1.1.1", "8.8.8.8"]);

const app = express();
// const PORT = process.env.PORT || 5001;

const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);

// ==================== ENVIRONMENT VALIDATION ====================
const requiredEnvVars = ["PORT", "MONGO_URI", "JWT_SECRET", "NODE_ENV"];

const validateEnv = () => {
const missing = requiredEnvVars.filter((varName) => !process.env[varName]);
if (missing.length > 0) {
console.error("Missing required environment variables:");
missing.forEach((varName) => console.error(` - ${varName}`));
process.exit(1);
}
console.log("All required environment variables are present");
};

validateEnv();

// ==================== REDIS CLIENT REFERENCE ====================
let redisClientInstance = null;

// ==================== MIDDLEWARE ====================

// CORS Configuration
if (process.env.NODE_ENV !== "production") {
app.use(
cors({
origin: "http://localhost:5173",
credentials: true, // Allow cookies to be sent
methods: ["GET", "POST", "PUT", "DELETE", "OPTIONS"],
allowedHeaders: ["Content-Type", "Authorization"],
}),
);
}
const corsOptions = {
origin:
process.env.NODE_ENV === "production"
? process.env.ALLOWED_ORIGINS?.split(",") || []
: ["http://localhost:5173", "http://localhost:3000"],
credentials: true,
methods: ["GET", "POST", "PUT", "DELETE", "OPTIONS"],
allowedHeaders: ["Content-Type", "Authorization"],
optionsSuccessStatus: 200,
};

app.use(cors(corsOptions));

// Body parsing middleware
app.use(express.json());
Expand Down Expand Up @@ -70,13 +90,42 @@ app.use("/api", optionalAuthenticateUser);
app.use("/api/notes", notesRoutes);
app.use("/api/auth", authRoutes);

// ==================== HEALTH CHECK ====================
app.get("/health", (req, res) => {
res.status(200).json({
// ==================== IMPROVED HEALTH CHECK ====================
app.get("/health", async (req, res) => {
let mongodbStatus = "Disconnected";
let redisStatus = "Disconnected";

try {
const mongoose = await import("mongoose");
mongodbStatus =
mongoose.default.connection.readyState === 1
? "Connected"
: "Disconnected";
} catch (error) {
mongodbStatus = "Error";
}

try {
if (redisClientInstance && redisClientInstance.isReady) {
redisStatus = "Connected";
}
} catch (error) {
redisStatus = "Error";
}

const healthStatus = {
status: "OK",
message: "Server is running",
timestamp: new Date().toISOString(),
});
uptime: process.uptime(),
services: {
mongodb: mongodbStatus,
redis: redisStatus,
},
environment: process.env.NODE_ENV || "development",
};

const isHealthy = mongodbStatus === "Connected";
res.status(isHealthy ? 200 : 503).json(healthStatus);
Comment on lines +116 to +128

@coderabbitai coderabbitai Bot Jul 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Health check status field is hardcoded to "OK" regardless of actual health.

Line 117 sets status: "OK" unconditionally, while isHealthy (Line 127) is computed afterward and only used for the HTTP status code. A client inspecting the JSON body (rather than the HTTP status) will always see "OK" even when MongoDB/Redis are disconnected — defeating the PR's goal of accurately reflecting service health.

🐛 Proposed fix 
+  const isHealthy = mongodbStatus === "Connected";
+
   const healthStatus = {
-    status: "OK",
+    status: isHealthy ? "OK" : "ERROR",
     timestamp: new Date().toISOString(),
     uptime: process.uptime(),
     services: {
       mongodb: mongodbStatus,
       redis: redisStatus,
     },
     environment: process.env.NODE_ENV || "development",
   };

-  const isHealthy = mongodbStatus === "Connected";
   res.status(isHealthy ? 200 : 503).json(healthStatus);
📝 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
const healthStatus = {
status: "OK",
message: "Server is running",
timestamp: new Date().toISOString(),
});
uptime: process.uptime(),
services: {
mongodb: mongodbStatus,
redis: redisStatus,
},
environment: process.env.NODE_ENV || "development",
};
const isHealthy = mongodbStatus === "Connected";
res.status(isHealthy ? 200 : 503).json(healthStatus);
const isHealthy = mongodbStatus === "Connected";
const healthStatus = {
status: isHealthy ? "OK" : "ERROR",
timestamp: new Date().toISOString(),
uptime: process.uptime(),
services: {
mongodb: mongodbStatus,
redis: redisStatus,
},
environment: process.env.NODE_ENV || "development",
};
res.status(isHealthy ? 200 : 503).json(healthStatus);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/server.js` around lines 116 - 128, The health check response
currently hardcodes healthStatus.status to "OK" in server.js, so the JSON body
never reflects failures even when the HTTP status is 503. Update the
healthStatus object to derive status from the same health decision used by
isHealthy, and make sure the response body in the health check handler reports
an unhealthy value when mongodbStatus or redisStatus is not connected. Use the
existing healthStatus and isHealthy logic in the server.js health endpoint to
keep the body and HTTP status consistent.

});

// ==================== PRODUCTION STATIC FILES ====================
Expand All @@ -87,36 +136,115 @@ if (process.env.NODE_ENV === "production") {
});
}

// ==================== ERROR HANDLING ====================
// ==================== IMPROVED ERROR HANDLING ====================
class AppError extends Error {
constructor(message, statusCode, details = null) {
super(message);
this.statusCode = statusCode;
this.details = details;
this.isOperational = true;
Error.captureStackTrace(this, this.constructor);
}
}

app.use((err, req, res, next) => {
console.error("Error:", err.message);

if (err instanceof AppError) {
return res.status(err.statusCode).json({
success: false,
message: err.message,
details: err.details,
...(process.env.NODE_ENV === "development" && { stack: err.stack }),
});
}

res.status(500).json({
error: "Internal server error",
message: process.env.NODE_ENV === "development" ? err.message : undefined,
success: false,
message:
process.env.NODE_ENV === "production"
? "Internal server error"
: err.message,
...(process.env.NODE_ENV === "development" && { stack: err.stack }),
});
});

// ==================== GRACEFUL SHUTDOWN ====================
let server = null;

const gracefulShutdown = async (signal) => {
console.log(`Received ${signal}. Shutting down gracefully...`);

const timeout = setTimeout(() => {
console.error("Forced shutdown after timeout");
process.exit(1);
}, 10000);

try {
const mongoose = await import("mongoose");
if (mongoose.default.connection.readyState === 1) {
await mongoose.default.connection.close();
console.log("MongoDB connection closed");
}

if (redisClientInstance && redisClientInstance.isReady) {
await redisClientInstance.quit();
console.log("Redis connection closed");
}

if (server) {
server.close(() => {
console.log("Server closed successfully");
clearTimeout(timeout);
process.exit(0);
});
} else {
clearTimeout(timeout);
process.exit(0);
}
Comment on lines +183 to +204

@coderabbitai coderabbitai Bot Jul 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the relevant file first, then read the shutdown-related section with line numbers.
ast-grep outline backend/src/server.js --view expanded || true
wc -l backend/src/server.js
sed -n '1,260p' backend/src/server.js | cat -n

# Find where the HTTP server is created and how requests depend on Mongo/Redis.
rg -n "createServer|listen\\(|server\\.close|gracefulShutdown|redisClientInstance|mongoose|quit\\(|close\\(" backend/src/server.js backend/src -g '!**/node_modules/**'

Repository: niharika-mente/ThinkBoard

Length of output: 13411

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the route handlers and controllers that run during requests.
ast-grep outline backend/src/routes/notesRoutes.js --view expanded || true
ast-grep outline backend/src/routes/authRoutes.js --view expanded || true
ast-grep outline backend/src/controllers/notesController.js --view expanded || true
ast-grep outline backend/src/controllers/authController.js --view expanded || true

# Read the relevant request-path code with line numbers.
sed -n '1,260p' backend/src/routes/notesRoutes.js | cat -n
sed -n '1,260p' backend/src/routes/authRoutes.js | cat -n
sed -n '1,260p' backend/src/controllers/notesController.js | cat -n
sed -n '1,260p' backend/src/controllers/authController.js | cat -n

Repository: niharika-mente/ThinkBoard

Length of output: 7722

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect auth and Redis-related middleware used on request paths.
ast-grep outline backend/src/middleware/authMiddleware.js --view expanded || true
ast-grep outline backend/src/middleware/rateLimiter.js --view expanded || true
ast-grep outline backend/src/config/redis.js --view expanded || true

# Read the relevant files with line numbers.
sed -n '1,260p' backend/src/middleware/authMiddleware.js | cat -n
sed -n '1,260p' backend/src/middleware/rateLimiter.js | cat -n
sed -n '1,260p' backend/src/config/redis.js | cat -n

Repository: niharika-mente/ThinkBoard

Length of output: 3109

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find all Redis client usages and whether request handlers depend on the server's redisClientInstance.
rg -n "redisClientInstance|connectRedis\\(|ratelimit|Upstash|`@upstash/redis`|redisClient\\." backend/src -g '!**/node_modules/**'

# Inspect the Upstash config referenced by the rate limiter.
ast-grep outline backend/src/config/upstash.js --view expanded || true
sed -n '1,220p' backend/src/config/upstash.js | cat -n

Repository: niharika-mente/ThinkBoard

Length of output: 1843

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the auth controller used by backend/src/routes/authRoutes.js.
ast-grep outline backend/src/controllers/userAuth.js --view expanded || true
sed -n '1,260p' backend/src/controllers/userAuth.js | cat -n

Repository: niharika-mente/ThinkBoard

Length of output: 4600

Drain HTTP before closing MongoDB/Redis. server.close() should happen first; /api/notes and /api/auth handlers still hit MongoDB, and the production rate limiter also uses Upstash Redis. Closing those clients before draining can make in-flight requests fail during shutdown.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/server.js` around lines 183 - 204, Shutdown order in the server
cleanup path is wrong: the MongoDB and Redis clients are being closed before the
HTTP server is drained. Update the shutdown flow in server.js so server.close()
happens first, then wait for it to finish before calling
mongoose.default.connection.close() and redisClientInstance.quit(). Keep the
existing cleanup logic in the server shutdown block, but reorder it to preserve
in-flight /api/notes and /api/auth requests and the rate limiter’s Redis usage
during graceful shutdown.

} catch (error) {
console.error("Error during shutdown:", error.message);
clearTimeout(timeout);
process.exit(1);
}
};

// ==================== START SERVER ====================
const startServer = async () => {
try {
// Connect to MongoDB
await connectDB();
console.log("MongoDB connected successfully");

// Connect to Redis
try {
await connectRedis();
console.log(" Redis connected successfully");
const redis = await connectRedis();
redisClientInstance = redis;
console.log("Redis connected successfully");
} catch (redisError) {
console.warn(" Redis connection failed, continuing without Redis");
console.warn("Redis connection failed, continuing without Redis");
}

// Start Express server
app.listen(process.env.PORT, () => {
server = app.listen(process.env.PORT, () => {
console.log("Server listening at port number: " + process.env.PORT);
console.log("Environment: " + (process.env.NODE_ENV || "development"));
console.log(
"Health check: http://localhost:" + process.env.PORT + "/health",
);
});

process.on("SIGINT", () => gracefulShutdown("SIGINT"));
process.on("SIGTERM", () => gracefulShutdown("SIGTERM"));

process.on("uncaughtException", (error) => {
console.error("Uncaught Exception:", error);
gracefulShutdown("uncaughtException");
});

process.on("unhandledRejection", (reason, promise) => {
console.error("Unhandled Rejection:", reason);
gracefulShutdown("unhandledRejection");
});
} catch (error) {
console.error(" Server startup failed:", error.message);
console.error("Server startup failed:", error.message);
process.exit(1);
}
};
Expand Down
Loading