From 536fdc7e8492931227bd4b211a644977c00c95c0 Mon Sep 17 00:00:00 2001 From: krizam Date: Fri, 30 Oct 2026 22:29:33 +0530 Subject: [PATCH] DeepSeek feat: extract performance monitoring into an opt-in tool and improve error handling in teacher routes --- package.json | 1 + src/Sessions/sessions.routes.ts | 219 ++++++++++++++++++++------------ src/Teachers/teachers.routes.ts | 41 ++++-- src/Users/users.routes.ts | 6 + src/errors.ts | 0 src/index.ts | 61 --------- tools/perf-hooks.mjs | 6 + tools/performance.ts | 142 +++++++++++++++------ tsconfig.json | 3 +- 9 files changed, 291 insertions(+), 188 deletions(-) create mode 100644 src/errors.ts create mode 100644 tools/perf-hooks.mjs diff --git a/package.json b/package.json index 7bb667c..53b2bff 100644 --- a/package.json +++ b/package.json @@ -6,6 +6,7 @@ "type": "module", "scripts": { "dev": "tsx watch src/index.ts", + "perf": "tsx watch --import ./tools/perf-hooks.mjs src/index.ts", "build": "tsc", "start": "node dist/index.js", "test": "echo \"Error: no test specified\" && exit 1" diff --git a/src/Sessions/sessions.routes.ts b/src/Sessions/sessions.routes.ts index a75b2fd..33e111b 100644 --- a/src/Sessions/sessions.routes.ts +++ b/src/Sessions/sessions.routes.ts @@ -2,58 +2,93 @@ import express from 'express'; import Sessions from './Schema/sessions.schema.js' import Teachers from '../Teachers/Schema/Teachers.schema.js'; import { Types } from 'mongoose'; -import { measureDb } from '../performance.js'; const SessionsRoutes = express.Router(); // Add New Session for teacher -SessionsRoutes.post('/', async (req, res) => { - const { teacherId, startTime, endTime } = req.body; +SessionsRoutes.post('/', async (req, res, next) => { + try { + const { teacherId, startTime, endTime } = req.body; - //Check for teacher existance - const existingTeacher = await measureDb('sessions.create.checkTeacher', () => Teachers.findOne({ _id: teacherId })); + if (!teacherId || !startTime || !endTime) { + return res.status(400).send({ message: 'teacherId, startTime and endTime are required.' }) + } + if (!Types.ObjectId.isValid(teacherId)) { + return res.status(400).send({ message: 'Invalid teacherId.' }) + } - if (!existingTeacher) { - return res.status(400).send({ message: 'Teacher Does not exist please provide valid teacherId.' }) - } + //Check for teacher existance + const existingTeacher = await Teachers.findOne({ _id: teacherId }); - // Check endTime must be greater then startTime - if (new Date(endTime) <= new Date(startTime)) { - return res.status(400).send({ message: 'End time must be greater then start time.' }) - } + if (!existingTeacher) { + return res.status(400).send({ message: 'Teacher Does not exist please provide valid teacherId.' }) + } - const newSession = new Sessions({ teacherId, startTime, endTime }); - const result = await measureDb('sessions.create.insert', () => newSession.save()); + // Reject invalid dates: NaN comparisons are always false, so the old + // "end <= start" check silently accepted garbage dates + const start = new Date(startTime); + const end = new Date(endTime); + if (isNaN(start.getTime()) || isNaN(end.getTime())) { + return res.status(400).send({ message: 'startTime and endTime must be valid dates.' }) + } + + // Check endTime must be greater then startTime + if (end <= start) { + return res.status(400).send({ message: 'End time must be greater then start time.' }) + } + + // Prevent overlapping sessions for the same teacher + const overlap = await Sessions.findOne({ + teacherId: teacherId, + status: { $in: ['AVAILABLE', 'BOOKED'] }, + startTime: { $lt: end }, + endTime: { $gt: start } + }); + if (overlap) { + return res.status(409).send({ message: 'Teacher already has a session overlapping this time range.' }) + } + + const newSession = new Sessions({ teacherId, startTime, endTime }); + const result = await newSession.save(); - if (result) { return res.status(201).send({ message: 'Session Added Successfully', result }) + } catch (error) { + next(error); } - }) // Get Available Sessions -SessionsRoutes.get('/available', async (req: any, res) => { - const dateTimestamp = req.query.dateTimestamp; +SessionsRoutes.get('/available', async (req: any, res, next) => { + try { + const dateTimestamp = req.query.dateTimestamp; - const dateString = dateTimestamp.slice(0, 10); - - // us standard dates we used here for simplicity - const startOfDay = new Date(`${dateString}T00:00:00.000Z`); - const endOfDay = new Date(`${dateString}T23:59:59.999Z`); - - const availableSessions = await measureDb('sessions.available.aggregate', () => Sessions.aggregate([ - { - $match: { - startTime: { $lte: endOfDay }, - endTime: { $gte: startOfDay }, - status: "AVAILABLE", - } + // Old code called .slice() unconditionally and crashed on a missing param + if (typeof dateTimestamp !== 'string' || dateTimestamp.length < 10 || + isNaN(new Date(dateTimestamp.slice(0, 10)).getTime())) { + return res.status(400).send({ message: 'A valid dateTimestamp query parameter (e.g. 2026-10-30) is required.' }) } - ])); - return res.status(200).send({ message: 'Available Session', availableSessions }) + const dateString = dateTimestamp.slice(0, 10); + // us standard dates we used here for simplicity + const startOfDay = new Date(`${dateString}T00:00:00.000Z`); + const endOfDay = new Date(`${dateString}T23:59:59.999Z`); + + const availableSessions = await Sessions.aggregate([ + { + $match: { + startTime: { $lte: endOfDay }, + endTime: { $gte: startOfDay }, + status: "AVAILABLE", + } + } + ]); + + return res.status(200).send({ message: 'Available Session', availableSessions }) + } catch (error) { + next(error); + } }) @@ -90,24 +125,32 @@ SessionsRoutes.get('/available', async (req: any, res) => { // }) // Book Available Sessions atomically -SessionsRoutes.post('/:id/book', async (req: any, res) => { - const { id } = req.params; - const { userId } = req.body; +SessionsRoutes.post('/:id/book', async (req: any, res, next) => { + try { + const { id } = req.params; + const { userId } = req.body; - // Atomically book only if the session is still AVAILABLE - const result = await measureDb('sessions.book.update', () => Sessions.findOneAndUpdate( - { _id: id, status: 'AVAILABLE' }, - { - $set: { - status: 'BOOKED', - userId: userId - } - }, + if (!Types.ObjectId.isValid(id)) { + return res.status(400).send({ message: 'Invalid session id.' }) + } + if (!userId || !Types.ObjectId.isValid(userId)) { + return res.status(400).send({ message: 'A valid userId is required in the request body.' }) + } + + // Atomically book only if the session is still AVAILABLE + const result = await Sessions.findOneAndUpdate( + { _id: id, status: 'AVAILABLE' }, + { + $set: { + status: 'BOOKED', + userId: new Types.ObjectId(userId) + } + }, { new: true, runValidators: true } - )); + ); if (result) { return res.status(200).send({ @@ -117,7 +160,7 @@ SessionsRoutes.post('/:id/book', async (req: any, res) => { } // Determine why the booking failed - const existingSession = await measureDb('sessions.book.failureLookup', () => Sessions.findById(id)); + const existingSession = await Sessions.findById(id); if (!existingSession) { return res.status(400).send({ @@ -128,45 +171,63 @@ SessionsRoutes.post('/:id/book', async (req: any, res) => { return res.status(400).send({ message: 'This session is not Available' }); + } catch (error) { + next(error); + } }); -// Update Available Sessions To Complete : User Id also needed to update -SessionsRoutes.patch('/:id/complete', async (req: any, res) => { +// Update Available Sessions To Complete : userId in the body must match the booking user +SessionsRoutes.patch('/:id/complete', async (req: any, res, next) => { + try { + const { id } = req.params; + const { userId } = req.body; - const { id } = req.params; + if (!Types.ObjectId.isValid(id)) { + return res.status(400).send({ message: 'Invalid session id.' }) + } + if (!userId || !Types.ObjectId.isValid(userId)) { + return res.status(400).send({ message: 'A valid userId is required in the request body.' }) + } - const existingSession = await measureDb('sessions.complete.find', () => Sessions.findOne({ _id: id })); + // Atomically complete only if this exact session is BOOKED by this user + // (fixes both the find-then-update race and the missing ownership check) + const result = await Sessions.findOneAndUpdate( + { _id: id, status: 'BOOKED', userId: new Types.ObjectId(userId) }, + { status: 'COMPLETED', completedAt: Date.now() }, + { new: true } + ); - if (!existingSession) { - return res.status(400).send({ message: 'Session does not exist.' }) + if (result) { + return res.status(200).send({ message: 'Session Marked Completed', result }) + } + + // Update matched nothing: distinguish the failure reasons + const existingSession = await Sessions.findById(id); + + if (!existingSession) { + return res.status(400).send({ message: 'Session does not exist.' }) + } + if (existingSession.status !== 'BOOKED') { + return res.status(400).send({ message: 'This session is not BOOKED' }); + } + + return res.status(403).send({ message: 'This session was not booked by this user.' }) + } catch (error) { + next(error); } - if (existingSession.status !== 'BOOKED') { - return res.status(400).send({ message: 'This session is not BOOKED' }); - } - - const result = await measureDb('sessions.complete.update', () => Sessions.findByIdAndUpdate( - id, - { status: "COMPLETED", completedAt: Date.now() }, - { new: true } - )); - - - if (result?.status === 'COMPLETED') { - return res.status(200).send({ message: 'Session Marked Completed', result }) - } else { - return res.status(400).send({ message: 'Error during updating session.' }) - } - - - }) // Get Sessions for User : With Seperation -SessionsRoutes.get('/:id/sessions', async (req, res) => { +SessionsRoutes.get('/:id/sessions', async (req, res, next) => { + try { + // Old code threw an unhandled CastError on a malformed id + if (!Types.ObjectId.isValid(req.params.id)) { + return res.status(400).send({ message: 'Invalid user id.' }) + } - let userId = new Types.ObjectId(req.params.id); + let userId = new Types.ObjectId(req.params.id); - const result = await measureDb('sessions.history.aggregate', () => Sessions.aggregate([ + const result = await Sessions.aggregate([ { $match: { userId: userId @@ -190,7 +251,7 @@ SessionsRoutes.get('/:id/sessions', async (req, res) => { ] } } - ])); + ]); res.status(200).json({ success: true, @@ -199,10 +260,12 @@ SessionsRoutes.get('/:id/sessions', async (req, res) => { completedSessions: [] } }); + } catch (error) { + next(error); + } }) - - export default SessionsRoutes; + diff --git a/src/Teachers/teachers.routes.ts b/src/Teachers/teachers.routes.ts index 9051e9e..520b439 100644 --- a/src/Teachers/teachers.routes.ts +++ b/src/Teachers/teachers.routes.ts @@ -1,27 +1,44 @@ import express from 'express'; import Teachers from './Schema/Teachers.schema.js' - +import mongoose from 'mongoose'; const TeacherRoutes = express.Router(); // Get All Teachers -TeacherRoutes.get('/', async(req, res) => { - const teachers = await Teachers.find(); - return res.status(200).send({'messesage': 'success', data: {teachers}}) +TeacherRoutes.get('/', async (_req, res, next) => { + try { + const teachers = await Teachers.find(); + return res.status(200).send({ message: 'success', data: { teachers } }) + } catch (error) { + next(error); + } }) // Add New Teacher -TeacherRoutes.post('/', async(req, res) => { - const {fullName, email, specialization, experience} = req.body; +TeacherRoutes.post('/', async (req, res, next) => { + try { + const { fullName, email, specialization, experience } = req.body; - const newTeacher = new Teachers({fullName, email, specialization, experience}); - const result = await newTeacher.save(); + if (!fullName || !email || !specialization || experience === undefined) { + return res.status(400).send({ message: 'fullName, email, specialization and experience are required.' }) + } - if(result) { - return res.status(201).send({message: 'Teacher Added Successfully', result}) + const newTeacher = new Teachers({ fullName, email, specialization, experience }); + const result = await newTeacher.save(); + + return res.status(201).send({ message: 'Teacher Added Successfully', result }) + } catch (error: unknown) { + if ( + error instanceof mongoose.mongo.MongoServerError && + error.code === 11000 + ) { + return res.status(400).send({ + message: 'A teacher already exists with this email.' + }); + } + + next(error); } - }) - export default TeacherRoutes; \ No newline at end of file diff --git a/src/Users/users.routes.ts b/src/Users/users.routes.ts index 040f147..d116f7a 100644 --- a/src/Users/users.routes.ts +++ b/src/Users/users.routes.ts @@ -22,6 +22,12 @@ UsersRoutes.post('/', async (req, res, next) => { try { const { fullName, email, phone } = req.body; + if (!fullName || !email || !phone) { + return res.status(400).send({ + message: 'fullName, email and phone are required.' + }); + } + const result = await Users.create({ fullName, email, phone }); return res.status(201).send({ diff --git a/src/errors.ts b/src/errors.ts new file mode 100644 index 0000000..e69de29 diff --git a/src/index.ts b/src/index.ts index e916cf5..1dc6935 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,7 +1,6 @@ import express from "express"; import 'dotenv/config'; import mongoose from "mongoose"; -import { monitorEventLoopDelay, performance } from "node:perf_hooks"; import TeachersRoutes from './Teachers/teachers.routes.js'; import UsersRoutes from "./Users/users.routes.js"; @@ -11,38 +10,6 @@ const app = express(); const PORT = 3000; const MONGO_URI = process.env.MONGO_URI!; -const eventLoop = monitorEventLoopDelay({ resolution: 20 }); -eventLoop.enable(); - -const requestMetrics = new Map(); - -app.use((req, res, next) => { - const started = performance.now(); - - res.on("finish", () => { - const durationMs = performance.now() - started; - const key = `${req.method} ${req.route?.path ?? req.path} ${res.statusCode}`; - - let metric = requestMetrics.get(key); - if (!metric) { - metric = { count: 0, totalMs: 0, maxMs: 0, over500Ms: 0 }; - requestMetrics.set(key, metric); - } - - metric.count++; - metric.totalMs += durationMs; - metric.maxMs = Math.max(metric.maxMs, durationMs); - if (durationMs > 500) metric.over500Ms++; - }); - - next(); -}); - app.use(express.json()); app.get("/", (_req, res) => { @@ -53,34 +20,6 @@ app.use('/users', UsersRoutes); app.use('/teachers', TeachersRoutes); app.use('/sessions', SessionsRoutes); -setInterval(() => { - const requests = Object.fromEntries( - [...requestMetrics.entries()].map(([key, value]) => [ - key, - { - count: value.count, - avgMs: Number((value.totalMs / value.count).toFixed(2)), - maxMs: Number(value.maxMs.toFixed(2)), - over500Ms: value.over500Ms, - }, - ]), - ); - - console.log(JSON.stringify({ - type: "performance_summary", - timestamp: new Date().toISOString(), - requests, - eventLoopDelayMs: { - mean: Number((eventLoop.mean / 1e6 || 0).toFixed(2)), - p95: Number((eventLoop.percentile(95) / 1e6).toFixed(2)), - max: Number((eventLoop.max / 1e6).toFixed(2)), - }, - })); - - requestMetrics.clear(); - eventLoop.reset(); -}, 10_000).unref(); - app.listen(PORT, async () => { try { await mongoose.connect(MONGO_URI); diff --git a/tools/perf-hooks.mjs b/tools/perf-hooks.mjs new file mode 100644 index 0000000..c17b1e7 --- /dev/null +++ b/tools/perf-hooks.mjs @@ -0,0 +1,6 @@ +// Opt-in loader: `node --import ./tools/perf-hooks.mjs src/index.ts` (see `yarn perf`). +// Keeps all performance instrumentation out of src/. +import { register, requestTimer } from './performance.ts'; + +register(); +requestTimer(); diff --git a/tools/performance.ts b/tools/performance.ts index bfbde2c..31a3c98 100644 --- a/tools/performance.ts +++ b/tools/performance.ts @@ -1,61 +1,131 @@ -import { performance } from "node:perf_hooks"; +/** + * Opt-in performance harness. Nothing in src/ imports this file, so the app + * runs without it. Load it with `yarn perf` (or `node --import ./tools/perf-hooks.mjs`). + */ +import mongoose from "mongoose"; +import { monitorEventLoopDelay, performance } from "node:perf_hooks"; +import express from "express"; -type DbMetric = { +type Metric = { count: number; totalMs: number; maxMs: number; over100Ms: number; }; -const metrics = new Map(); +type RequestMetric = { + count: number; + totalMs: number; + maxMs: number; + over500Ms: number; +}; -export async function measureDb( - label: string, - operation: () => Promise, -): Promise { - const started = performance.now(); +const dbMetrics = new Map(); +const requestMetrics = new Map(); - try { - return await operation(); - } finally { - const durationMs = performance.now() - started; +const eventLoop = monitorEventLoopDelay({ resolution: 20 }); +eventLoop.enable(); - let metric = metrics.get(label); - if (!metric) { - metric = { count: 0, totalMs: 0, maxMs: 0, over100Ms: 0 }; - metrics.set(label, metric); - } +// --- Database timings ------------------------------------------------------- +// Mongoose emits these for every query, so no route code has to be touched. +export function register() { + mongoose.connection.on('connected', () => { + const client = mongoose.connection.getClient(); - metric.count++; - metric.totalMs += durationMs; - metric.maxMs = Math.max(metric.maxMs, durationMs); + client.on('commandStarted', (event: any) => { + (event as any).__startedAt = performance.now(); + }); - if (durationMs > 100) { - metric.over100Ms++; - } - } + client.on('commandSucceeded', (event: any) => { + recordDb(`${event.commandName}.${Object.keys(event.command ?? {})[0] ?? 'unknown'}`, event); + }); + + client.on('commandFailed', (event: any) => { + recordDb(`${event.commandName}.failed`, event); + }); + }); } -setInterval(() => { - if (metrics.size === 0) return; +function recordDb(label: string, event: any) { + const startedAt = event.__startedAt; + if (typeof startedAt !== 'number') return; - const operations = Object.fromEntries( - [...metrics.entries()].map(([label, metric]) => [ - label, + const durationMs = performance.now() - startedAt; + + let metric = dbMetrics.get(label); + if (!metric) { + metric = { count: 0, totalMs: 0, maxMs: 0, over100Ms: 0 }; + dbMetrics.set(label, metric); + } + + metric.count++; + metric.totalMs += durationMs; + metric.maxMs = Math.max(metric.maxMs, durationMs); + if (durationMs > 100) metric.over100Ms++; +} + +// --- Request timings -------------------------------------------------------- +// Installed by patching express.json() from the loader, so index.ts stays clean. +export function requestTimer() { + const originalJson = express.json.bind(express); + + express.json = ((...args: any[]) => { + const parser = (originalJson as any)(...args); + return (req: any, res: any, next: any) => { + const started = performance.now(); + + res.on("finish", () => { + const durationMs = performance.now() - started; + const key = `${req.method} ${req.route?.path ?? req.path} ${res.statusCode}`; + + let metric = requestMetrics.get(key); + if (!metric) { + metric = { count: 0, totalMs: 0, maxMs: 0, over500Ms: 0 }; + requestMetrics.set(key, metric); + } + const current = metric; + + current.count++; + current.totalMs += durationMs; + current.maxMs = Math.max(current.maxMs, durationMs); + if (durationMs > 500) current.over500Ms++; + }); + + parser(req, res, next); + }; + }) as typeof express.json; +} + +// --- Reporting -------------------------------------------------------------- +function summarize(entries: [string, T][]) { + return Object.fromEntries( + entries.map(([key, value]) => [ + key, { - count: metric.count, - avgMs: Number((metric.totalMs / metric.count).toFixed(2)), - maxMs: Number(metric.maxMs.toFixed(2)), - over100Ms: metric.over100Ms, + count: value.count, + avgMs: Number((value.totalMs / value.count).toFixed(2)), + maxMs: Number(value.maxMs.toFixed(2)), }, ]), ); +} + +setInterval(() => { + if (requestMetrics.size === 0 && dbMetrics.size === 0) return; console.log(JSON.stringify({ - type: "database_performance", + type: "performance_summary", timestamp: new Date().toISOString(), - operations, + requests: summarize([...requestMetrics.entries()]), + database: summarize([...dbMetrics.entries()]), + eventLoopDelayMs: { + mean: Number((eventLoop.mean / 1e6 || 0).toFixed(2)), + p95: Number((eventLoop.percentile(95) / 1e6).toFixed(2)), + max: Number((eventLoop.max / 1e6).toFixed(2)), + }, })); - metrics.clear(); + requestMetrics.clear(); + dbMetrics.clear(); + eventLoop.reset(); }, 10_000).unref(); diff --git a/tsconfig.json b/tsconfig.json index bdbad8c..7e67662 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -40,5 +40,6 @@ "noUncheckedSideEffectImports": true, "moduleDetection": "force", "skipLibCheck": true, - } + }, + "exclude": ["node_modules", "dist", "tools"] }