From ca11007c5ee3d075944b754c13837f4553032bf5 Mon Sep 17 00:00:00 2001 From: Shahnawaz Hussain Date: Wed, 14 Jan 2026 05:04:52 +0530 Subject: [PATCH] fix: resolve critical bugs in transaction handling, error responses, and solver error handling - Add database transaction to getCabDetails for data consistency - Standardize error response format across all controllers and middleware - Add graceful error handling for allocation solver failures - Update all error responses to use consistent { success: false, error: string } format Fixes: - Bug #6: Missing database transaction in getCabDetails (prevents data inconsistency) - Bug #9: Unhandled solver errors causing 500 crashes (now returns 400 with user-friendly message) - Bug #13: Inconsistent error response formats (standardized across 7 files) Note: Bug #4 (Webhook body parsing) was already fixed in previous commits --- .../src/controllers/allocation.controller.ts | 16 ++++++- backend/src/controllers/qr.controller.ts | 24 ++++++++--- backend/src/middleware/auth.middleware.ts | 25 ++++++++--- backend/src/middleware/cron.middleware.ts | 10 ++++- backend/src/middleware/webhook.middleware.ts | 10 ++++- backend/src/routes/auth.routes.ts | 43 +++++++++++++++---- backend/src/routes/webhook.routes.ts | 10 ++++- 7 files changed, 111 insertions(+), 27 deletions(-) diff --git a/backend/src/controllers/allocation.controller.ts b/backend/src/controllers/allocation.controller.ts index ab97ef4..1793b4b 100644 --- a/backend/src/controllers/allocation.controller.ts +++ b/backend/src/controllers/allocation.controller.ts @@ -76,9 +76,21 @@ export const runAllocation = async (req: Request, res: Response): Promise const hallDemand = demandResult.rows; const totalStudents = hallDemand.reduce((sum: number, row: any) => sum + row.count, 0); - // 2. Run solver + // 2. Run solver with error handling const studentsPerRegion = convertHallDemandToRegions(hallDemand); - const solverResult = solveCabAllocation(studentsPerRegion); + let solverResult; + + try { + solverResult = solveCabAllocation(studentsPerRegion); + } catch (error) { + console.error('Solver error:', error); + res.status(400).json({ + success: false, + error: 'Unable to create optimal allocation. The booking configuration may not be feasible. Please try manual allocation or contact support.', + details: error instanceof Error ? error.message : 'Solver failed', + }); + return; + } // 3. Get all students with details for random assignment const studentsResult = await pool.query( diff --git a/backend/src/controllers/qr.controller.ts b/backend/src/controllers/qr.controller.ts index 77b544e..ae2187a 100644 --- a/backend/src/controllers/qr.controller.ts +++ b/backend/src/controllers/qr.controller.ts @@ -321,6 +321,8 @@ export const validateQR = async (req: Request, res: Response): Promise => // GET CAB DETAILS (for student cab view) // ==================================== export const getCabDetails = async (req: Request, res: Response): Promise => { + const client = await pool.connect(); + try { const allocationId = req.params.allocationId as string; @@ -335,8 +337,11 @@ export const getCabDetails = async (req: Request, res: Response): Promise return; } + // Start transaction for consistent read + await client.query('BEGIN'); + // Get cab_id from the allocation - const allocationResult = await pool.query( + const allocationResult = await client.query( `SELECT cab_id, trip_id, user_id FROM cab_allocations WHERE id = $1`, @@ -344,6 +349,7 @@ export const getCabDetails = async (req: Request, res: Response): Promise ); if (allocationResult.rows.length === 0) { + await client.query('ROLLBACK'); res.status(404).json({ success: false, error: 'not_found', @@ -355,7 +361,7 @@ export const getCabDetails = async (req: Request, res: Response): Promise const { cab_id, trip_id, user_id } = allocationResult.rows[0]; // Check if user has paid for this trip - const paymentCheck = await pool.query( + const paymentCheck = await client.query( `SELECT p.payment_status FROM trip_users tu JOIN payments p ON tu.payment_id = p.id @@ -364,6 +370,7 @@ export const getCabDetails = async (req: Request, res: Response): Promise ); if (paymentCheck.rows.length === 0 || paymentCheck.rows[0].payment_status !== 'confirmed') { + await client.query('ROLLBACK'); res.status(403).json({ success: false, error: 'payment_required', @@ -373,7 +380,7 @@ export const getCabDetails = async (req: Request, res: Response): Promise } // Fetch cab details (WITHOUT passkey - students shouldn't see it) - const cabResult = await pool.query( + const cabResult = await client.query( `SELECT c.id, c.cab_number, @@ -388,6 +395,7 @@ export const getCabDetails = async (req: Request, res: Response): Promise ); if (cabResult.rows.length === 0) { + await client.query('ROLLBACK'); res.status(404).json({ success: false, error: 'not_found', @@ -399,7 +407,7 @@ export const getCabDetails = async (req: Request, res: Response): Promise const cab = cabResult.rows[0]; // Fetch all students in this cab - const studentsResult = await pool.query( + const studentsResult = await client.query( `SELECT ca.user_id, ca.id as booking_id, @@ -431,7 +439,7 @@ export const getCabDetails = async (req: Request, res: Response): Promise // Fetch ALL OTHER cabs for this trip (for return journey options) // Only return driver details and starting point - no passenger info - const otherCabsResult = await pool.query( + const otherCabsResult = await client.query( `SELECT c.id, c.cab_number, @@ -447,6 +455,9 @@ export const getCabDetails = async (req: Request, res: Response): Promise const otherCabs = otherCabsResult.rows; + // Commit transaction + await client.query('COMMIT'); + res.status(200).json({ success: true, data: { @@ -456,6 +467,7 @@ export const getCabDetails = async (req: Request, res: Response): Promise }, }); } catch (error: any) { + await client.query('ROLLBACK'); console.error('Error fetching cab details:', error); res.status(500).json({ success: false, @@ -463,6 +475,8 @@ export const getCabDetails = async (req: Request, res: Response): Promise message: 'Failed to fetch cab details', details: error.message, }); + } finally { + client.release(); } }; diff --git a/backend/src/middleware/auth.middleware.ts b/backend/src/middleware/auth.middleware.ts index 43de474..e24c3c3 100644 --- a/backend/src/middleware/auth.middleware.ts +++ b/backend/src/middleware/auth.middleware.ts @@ -14,14 +14,20 @@ export const authenticateUser = async ( const token = req.headers.authorization?.replace('Bearer ', ''); if (!token) { - return res.status(401).json({ error: 'No token provided' }); + return res.status(401).json({ + success: false, + error: 'No token provided' + }); } // Verify JWT token const decoded = verifyToken(token); if (!decoded) { - return res.status(401).json({ error: 'Invalid or expired token' }); + return res.status(401).json({ + success: false, + error: 'Invalid or expired token' + }); } // Fetch user details from database to ensure user still exists @@ -31,7 +37,10 @@ export const authenticateUser = async ( ); if (result.rows.length === 0) { - return res.status(404).json({ error: 'User not found' }); + return res.status(404).json({ + success: false, + error: 'User not found' + }); } const userData = result.rows[0]; @@ -48,7 +57,10 @@ export const authenticateUser = async ( next(); } catch (error) { console.error('Authentication error:', error); - res.status(500).json({ error: 'Authentication failed' }); + res.status(500).json({ + success: false, + error: 'Authentication failed' + }); } }; @@ -58,7 +70,10 @@ export const requireAdmin = ( next: NextFunction ) => { if (!req.user?.isAdmin) { - return res.status(403).json({ error: 'Admin access required' }); + return res.status(403).json({ + success: false, + error: 'Admin access required' + }); } next(); }; diff --git a/backend/src/middleware/cron.middleware.ts b/backend/src/middleware/cron.middleware.ts index 152d4c8..e839430 100644 --- a/backend/src/middleware/cron.middleware.ts +++ b/backend/src/middleware/cron.middleware.ts @@ -16,12 +16,18 @@ export const verifyCronSecret = (req: Request, res: Response, next: NextFunction if (!cronSecret) { console.error('❌ CRON_SECRET environment variable not set'); - return res.status(500).json({ error: 'Server misconfiguration' }); + return res.status(500).json({ + success: false, + error: 'Server misconfiguration' + }); } if (secret !== cronSecret) { console.warn('⚠️ Invalid cron secret attempt'); - return res.status(401).json({ error: 'Unauthorized' }); + return res.status(401).json({ + success: false, + error: 'Unauthorized' + }); } next(); diff --git a/backend/src/middleware/webhook.middleware.ts b/backend/src/middleware/webhook.middleware.ts index a76e7a0..e39f3e1 100644 --- a/backend/src/middleware/webhook.middleware.ts +++ b/backend/src/middleware/webhook.middleware.ts @@ -50,7 +50,10 @@ export const requireWebhookSignature = ( const signature = req.headers['x-razorpay-signature']; if (!signature) { - res.status(401).json({ error: 'Missing webhook signature' }); + res.status(401).json({ + success: false, + error: 'Missing webhook signature' + }); return; } @@ -85,7 +88,10 @@ export const webhookRateLimiter = ( if (record.count >= maxRequests) { console.warn(`Webhook rate limit exceeded for IP: ${ip}`); - res.status(429).json({ error: 'Too many requests' }); + res.status(429).json({ + success: false, + error: 'Too many requests' + }); return; } diff --git a/backend/src/routes/auth.routes.ts b/backend/src/routes/auth.routes.ts index 04860c8..c5523d3 100644 --- a/backend/src/routes/auth.routes.ts +++ b/backend/src/routes/auth.routes.ts @@ -68,13 +68,19 @@ router.get('/me', authenticateUser, async (req, res) => { ); if (result.rows.length === 0) { - return res.status(404).json({ error: 'User not found' }); + return res.status(404).json({ + success: false, + error: 'User not found' + }); } res.json(formatUserResponse(result.rows[0])); } catch (error) { console.error('Error fetching user profile:', error); - res.status(500).json({ error: 'Failed to fetch user' }); + res.status(500).json({ + success: false, + error: 'Failed to fetch user' + }); } }); @@ -143,7 +149,10 @@ router.put('/me', authenticateUser, validateProfileUpdate, async (req, res) => { }); } catch (error) { console.error('Error updating user profile:', error); - res.status(500).json({ error: 'Failed to update user profile' }); + res.status(500).json({ + success: false, + error: 'Failed to update user profile' + }); } }); @@ -161,14 +170,20 @@ router.post('/verify-login', async (req, res) => { try { // Check if verification login is enabled if (process.env.ENABLE_VERIFY_LOGIN !== 'true') { - return res.status(404).json({ error: 'Not found' }); + return res.status(404).json({ + success: false, + error: 'Not found' + }); } const { email, password } = req.body; // Validate input if (!email || !password) { - return res.status(400).json({ error: 'Email and password are required' }); + return res.status(400).json({ + success: false, + error: 'Email and password are required' + }); } // Check against environment variables @@ -177,12 +192,18 @@ router.post('/verify-login', async (req, res) => { if (!verifyEmail || !verifyPassword) { console.error('VERIFY_EMAIL or VERIFY_PASSWORD not configured'); - return res.status(500).json({ error: 'Verification login not configured' }); + return res.status(500).json({ + success: false, + error: 'Verification login not configured' + }); } - // Validate credentials + // Verify credentials if (email !== verifyEmail || password !== verifyPassword) { - return res.status(401).json({ error: 'Invalid email or password' }); + return res.status(401).json({ + success: false, + error: 'Invalid email or password' + }); } // Find user in database @@ -193,6 +214,7 @@ router.post('/verify-login', async (req, res) => { if (result.rows.length === 0) { return res.status(401).json({ + success: false, error: 'User not found. Please ensure the test user exists in the database.' }); } @@ -214,7 +236,10 @@ router.post('/verify-login', async (req, res) => { }); } catch (error) { console.error('Verify login error:', error); - res.status(500).json({ error: 'Login failed' }); + res.status(500).json({ + success: false, + error: 'Login failed' + }); } }); diff --git a/backend/src/routes/webhook.routes.ts b/backend/src/routes/webhook.routes.ts index dcdf67b..d65e0f1 100644 --- a/backend/src/routes/webhook.routes.ts +++ b/backend/src/routes/webhook.routes.ts @@ -40,7 +40,10 @@ router.post('/razorpay', async (req: Request, res: Response) => { if (!signature) { console.error('Webhook missing signature header'); - res.status(400).json({ error: 'Missing signature header' }); + res.status(400).json({ + success: false, + error: 'Missing signature header' + }); return; } @@ -51,7 +54,10 @@ router.post('/razorpay', async (req: Request, res: Response) => { if (!rawBody) { console.error('Webhook missing raw body'); - res.status(400).json({ error: 'Missing request body' }); + res.status(400).json({ + success: false, + error: 'Missing request body' + }); return; }