# Batch 09: Controllers 81-90 Analysis - Unhandled Exception & Crash Loop Risks ## Executive Summary พบจุดเสี่ยงระดับ **CRITICAL** ที่อาจทำให้เกิด **Unhandled Exception** และ **Crash Loop** ในระบบ Microservices จำนวน **5 จุด** จากการตรวจสอบ 10 Controllers ในชุดที่ 9 --- ## Critical Issues Found ### 1. **CRITICAL** - Unhandled External API Call with Silent Failure #### **File & Location** - **File:** `src/controllers/ProfileEditController.ts` - Lines 360-372: `newProfileEdit()` method - **File:** `src/controllers/ProfileEditEmployeeController.ts` - Lines 360-372: `profileEdit()` method #### **Problem Type** 1. **Unhandled Exception** 2. **Silent Error Swallowing** 3. **Data Inconsistency Risk** #### **Root Cause** ```typescript // ProfileEditController.ts:360-372 await new CallAPI() .PostData(req, "/org/workflow/add-workflow", { refId: data.id, sysName: "REGISTRY_PROFILE", posLevelName: profile.posLevel.posLevelName, posTypeName: profile.posType.posTypeName, fullName: `${profile.prefix}${profile.firstName} ${profile.lastName}`, isDeputy: orgRoot?.isDeputy ?? false, orgRootId: orgRoot?.id ?? null }) .catch((error) => { console.error("Error calling API:", error); }); // ❌ No re-throw, no proper error handling ``` **รายละเอียดปัญหา:** 1. **Silent Failure**: มีการใช้ `.catch()` แค่ log error แต่ไม่ throw หรือ handle error 2. **Data Inconsistency**: ข้อมูล ProfileEdit ถูกบันทึกแล้ว แต่ Workflow ไม่ได้ถูกสร้าง 3. **No Transaction**: ไม่มีการใช้ Transaction เพื่อ roll back ข้อมูลเมื่อ API ล้มเหลว 4. **User Confusion**: ผู้ใช้จะเห็นว่าบันทึกสำเร็จ แต่จริงๆ แล้ว Workflow ไม่ได้ทำงาน **ผลกระทบ:** - ข้อมูลใน Database ไม่สมบูรณ์ (ProfileEdit มีแต่ไม่มี Workflow) - ผู้ใช้ไม่ทราบว่าเกิด Error จริงๆ - ระบบอาจทำงานผิดปกติในภายหลังเมื่อมีการดำเนินการกับข้อมูลที่ไม่สมบูรณ์ --- ### 2. **CRITICAL** - Potential Null Pointer Exception in Optional Chaining #### **File & Location** - **File:** `src/controllers/ProfileEditController.ts` - Line 336-344: `newProfileEdit()` method - **File:** `src/controllers/ProfileEditEmployeeController.ts` - Line 337-345: `profileEdit()` method #### **Problem Type** 1. **Unhandled Exception** 2. **TypeError Risk** 3. **Potential Crash** #### **Root Cause** ```typescript // ProfileEditController.ts:336-344 const orgRoot = await this.orgRootRepo.findOne({ select: { id: true, isDeputy: true }, where: { id: profile.current_holders.find(x => x.orgRootId)!.orgRootId ?? "" // ^ // Non-null assertion without check } }); ``` **รายละเอียดปัญหา:** 1. **Unsafe Array Access**: ใช้ `.find()` แล้วใช้ `!` (non-null assertion) โดยไม่มีการ check 2. **Potential TypeError**: หาก `.find()` return `undefined` การพยายามเข้าถึง `.orgRootId` จะทำให้เกิด `TypeError: Cannot read property 'orgRootId' of undefined` 3. **Unhandled Exception**: Error นี้จะทำให้ Service Crash ทันที **สถานการณ์ที่อาจเกิดขึ้น:** ```typescript // หาก current_holders เป็น empty array หรือไม่พบ element profile.current_holders.find(x => x.orgRootId) // returns undefined undefined!.orgRootId // ❌ CRASH: TypeError ``` --- ### 3. **HIGH** - Unsafe Array Access in Multiple Locations #### **File & Location** - **File:** `src/controllers/ProfileEditController.ts` - Line 278: `detailProfileEdit()` method - **File:** `src/controllers/ProfileEditEmployeeController.ts` - Line 277: `detailProfileEditEmp()` method #### **Problem Type** 1. **Unhandled Exception** 2. **TypeError Risk** #### **Root Cause** ```typescript // ProfileEditController.ts:278-292 let orgRoot: OrgRoot | null = null; if(getProfileEdit.profile) { const empPosMaster = await this.posMasterRepo.findOne({ where: { current_holderId: getProfileEdit.profile.id, orgRevision: { orgRevisionIsCurrent: true, orgRevisionIsDraft: false } }, relations: { orgRevision: true } }); if(empPosMaster) { orgRoot = await this.orgRootRepo.findOne({ select: { isDeputy: true }, where: { id: empPosMaster.orgRootId ?? "" } // ^^^^^^^^^^^^^^^^^^^ // May be null, using "" as fallback }); } } ``` **รายละเอียดปัญหา:** 1. **Unsafe Fallback**: ใช้ empty string `""` เป็น fallback สำหรับ `orgRootId` 2. **Silent Failure**: การ query ด้วย ID ว่างจะ return `null` แต่ไม่มีการแจ้งเตือน 3. **Data Integrity**: อาจทำให้ข้อมูล `isDeputy` ไม่ถูกต้อง --- ### 4. **HIGH** - Missing Error Handling in Database Update Operations #### **File & Location** - **File:** `src/controllers/ProfileDisciplineController.ts` - Lines 167-172: `editDiscipline()` method - **File:** `src/controllers/ProfileDisciplineEmployeeController.ts` - Lines 172-177: `editDiscipline()` method - **File:** `src/controllers/ProfileDisciplineEmployeeTempController.ts` - Lines 162-167: `editDiscipline()` method - **File:** `src/controllers/ProfileDutyController.ts` - Lines 143-148: `editDuty()` method - **File:** `src/controllers/ProfileDutyEmployeeController.ts` - Lines 152-157: `editDuty()` method - **File:** `src/controllers/ProfileDutyEmployeeTempController.ts` - Lines 141-146: `editDuty()` method #### **Problem Type** 1. **Missing Error Handle** 2. **Data Loss Risk** #### **Root Cause** ```typescript // Pattern found across multiple controllers this.disciplineRepository.save(record, { data: req }); setLogDataDiff(req, { before, after: record }); if (!(Object.keys(body).length === 1 && body.isUpload)) { this.disciplineHistoryRepository.save(history, { data: req }); // ❌ No await, no error handling } ``` **รายละเอียดปัญหา:** 1. **Missing await**: ไม่มีการ `await` การ save history ทำให้ไม่รู้ว่า save สำเร็จหรือไม่ 2. **No Error Handling**: หากการ save history ล้มเหลว จะไม่มีการ catch error 3. **Silent Failure**: History อาจไม่ถูกบันทึก แต่ไม่มีใครรู้ **ผลกระทบ:** - History audit trail ไม่สมบูรณ์ - ไม่สามารถ trace back การเปลี่ยนแปลงได้ - การ audit และ debugging ยากขึ้น --- ### 5. **MEDIUM** - Complex Nested Query Without Error Handling #### **File & Location** - **File:** `src/controllers/ProfileEditController.ts` - Lines 112-255: `detailProfileEditAdmin()` method - **File:** `src/controllers/ProfileEditEmployeeController.ts` - Lines 110-254: `detailProfileEditAdminEmp()` method #### **Problem Type** 1. **Missing Error Handle** 2. **Performance Risk** 3. **Query Complexity Risk** #### **Root Cause** ```typescript // ProfileEditController.ts:122-193 const orgRevisionPublish = await this.orgRevisionRepository .createQueryBuilder("orgRevision") .where("orgRevision.orgRevisionIsDraft = false") .andWhere("orgRevision.orgRevisionIsCurrent = true") .getOne(); // ❌ No null check, used in query below let query = await AppDataSource.getRepository(ProfileEdit) .createQueryBuilder("ProfileEdit") .leftJoinAndSelect("ProfileEdit.profile", "profile") .leftJoinAndSelect("profile.current_holders", "current_holders") .leftJoinAndSelect("current_holders.orgRevision", "orgRevision") .where((qb) => { if (status != "" && status != null) { qb.andWhere("ProfileEdit.status = :status", { status: status }); } qb.andWhere("ProfileEdit.profileId IS NOT NULL"); }) .andWhere(orgRevisionPublish ? `current_holders.orgRevisionId = :revisionId` : "1=1", { revisionId: orgRevisionPublish?.id, // ❌ Could be undefined }) .andWhere( data.root != undefined && data.root != null ? data.root[0] != null ? `current_holders.orgRootId IN (:...root)` : `current_holders.orgRootId is null` : "1=1", { root: data.root, // ❌ Could cause SQL error if undefined }, ) // ... more complex conditions ``` **รายละเอียดปัญหา:** 1. **No Null Check**: `orgRevisionPublish` อาจเป็น `null` แต่ถูกใช้ใน query 2. **Complex Query Logic**: Query ที่ซับซ้อนมากหลายเงื่อนไข ไม่มีการ validate input 3. **SQL Injection Risk**: แม้จะใช้ Parameterized query แต่ยังมี dynamic SQL ที่อาจเสี่ยง 4. **No Timeout**: Query ขนาดใหญ่ไม่มี timeout อาจทำให้ connection hang --- ## Recommended Fixes ### Fix 1: Proper Error Handling for External API Calls **Before:** ```typescript await this.profileEditRepo.save(data); await new CallAPI() .PostData(req, "/org/workflow/add-workflow", {...}) .catch((error) => { console.error("Error calling API:", error); }); return new HttpSuccess(data.id); ``` **After:** ```typescript // Option 1: Use Transaction Pattern await AppDataSource.transaction(async (transactionalEntityManager) => { // Save main data const savedData = await transactionalEntityManager.save(ProfileEdit, data); try { // Call external API await new CallAPI().PostData(req, "/org/workflow/add-workflow", { refId: savedData.id, sysName: "REGISTRY_PROFILE", posLevelName: profile.posLevel.posLevelName, posTypeName: profile.posType.posTypeName, fullName: `${profile.prefix}${profile.firstName} ${profile.lastName}`, isDeputy: orgRoot?.isDeputy ?? false, orgRootId: orgRoot?.id ?? null }); } catch (error) { console.error("Failed to create workflow:", error); // Rollback by throwing error throw new HttpError( HttpStatus.SERVICE_UNAVAILABLE, "ไม่สามารถสร้าง Workflow ได้ กรุณาลองใหม่ภายหลัง" ); } }); return new HttpSuccess(data.id); // Option 2: Async Pattern with Queue (Recommended for Production) // Save data first, then process workflow asynchronously const savedData = await this.profileEditRepo.save(data); // Emit event for workflow creation // await this.eventEmitter.emit('profile.edit.created', { // profileEditId: savedData.id, // profileId: profile.id, // // ... other data // }); return new HttpSuccess(savedData.id); ``` --- ### Fix 2: Safe Array Access with Proper Null Checks **Before:** ```typescript const orgRoot = await this.orgRootRepo.findOne({ select: { id: true, isDeputy: true }, where: { id: profile.current_holders.find(x => x.orgRootId)!.orgRootId ?? "" } }); ``` **After:** ```typescript // Safe access with proper null checks const currentHolder = profile.current_holders?.find(x => x.orgRootId); if (!currentHolder || !currentHolder.orgRootId) { throw new HttpError( HttpStatus.BAD_REQUEST, "ไม่พบข้อมูลตำแหน่งปัจจุบัน กรุณาติดต่อ HR" ); } const orgRoot = await this.orgRootRepo.findOne({ select: { id: true, isDeputy: true }, where: { id: currentHolder.orgRootId } }); if (!orgRoot) { console.warn(`OrgRoot not found for id: ${currentHolder.orgRootId}`); // Continue with default values or throw error based on business logic } ``` --- ### Fix 3: Add Proper Error Handling for Database Operations **Before:** ```typescript this.disciplineRepository.save(record, { data: req }); setLogDataDiff(req, { before, after: record }); if (!(Object.keys(body).length === 1 && body.isUpload)) { this.disciplineHistoryRepository.save(history, { data: req }); } ``` **After:** ```typescript try { // Save main record await this.disciplineRepository.save(record, { data: req }); setLogDataDiff(req, { before, after: record }); // Save history if needed if (!(Object.keys(body).length === 1 && body.isUpload)) { try { await this.disciplineHistoryRepository.save(history, { data: req }); } catch (historyError) { console.error("Failed to save history:", historyError); // Log error but don't fail the request // Consider using a message queue for audit logging // await this.auditQueue.send({ // action: 'DISCIPLINE_UPDATE', // data: history, // error: historyError.message // }); } } } catch (error) { console.error("Failed to save discipline:", error); throw new HttpError( HttpStatus.INTERNAL_SERVER_ERROR, "ไม่สามารถบันทึกข้อมูลได้ กรุณาลองใหม่" ); } ``` --- ### Fix 4: Add Query Timeout and Null Checks **Before:** ```typescript const orgRevisionPublish = await this.orgRevisionRepository .createQueryBuilder("orgRevision") .where("orgRevision.orgRevisionIsDraft = false") .andWhere("orgRevision.orgRevisionIsCurrent = true") .getOne(); let query = await AppDataSource.getRepository(ProfileEdit) .createQueryBuilder("ProfileEdit") // ... complex query ``` **After:** ```typescript // Add timeout and proper null handling const orgRevisionPublish = await this.orgRevisionRepository .createQueryBuilder("orgRevision") .where("orgRevision.orgRevisionIsDraft = false") .andWhere("orgRevision.orgRevisionIsCurrent = true") .setHint('maxExecutionTime', 5000) // 5 second timeout .getOne(); // Validate permission data if (!data || !data.root) { throw new HttpError( HttpStatus.FORBIDDEN, "ไม่มีสิทธิ์เข้าถึงข้อมูล" ); } // Build query with validation const queryBuilder = AppDataSource.getRepository(ProfileEdit) .createQueryBuilder("ProfileEdit") .leftJoinAndSelect("ProfileEdit.profile", "profile") .leftJoinAndSelect("profile.current_holders", "current_holders") .leftJoinAndSelect("current_holders.orgRevision", "orgRevision") .where((qb) => { if (status != "" && status != null) { qb.andWhere("ProfileEdit.status = :status", { status: status }); } qb.andWhere("ProfileEdit.profileId IS NOT NULL"); }) .setMaxResults(1000) // Prevent large result sets .setHint('maxExecutionTime', 10000); // 10 second timeout // Add revision filter only if valid if (orgRevisionPublish?.id) { queryBuilder.andWhere( `current_holders.orgRevisionId = :revisionId`, { revisionId: orgRevisionPublish.id } ); } // Add root filter with validation if (Array.isArray(data.root) && data.root.length > 0 && data.root[0] !== null) { queryBuilder.andWhere(`current_holders.orgRootId IN (:...root)`, { root: data.root }); } else if (data.root?.[0] === null) { queryBuilder.andWhere(`current_holders.orgRootId IS NULL`); } const [getProfileEdit, total] = await queryBuilder .skip((page - 1) * pageSize) .take(Math.min(pageSize, 100)) // Limit page size .getManyAndCount(); ``` --- ### Fix 5: Implement Global Error Handler **Create/Update `src/middlewares/error-handler.ts`:** ```typescript import { Request, Response, NextFunction } from 'express'; import HttpError from '../interfaces/http-error'; export function globalErrorHandler( err: Error, req: Request, res: Response, next: NextFunction ) { console.error('[Unhandled Exception]', { error: err.message, stack: err.stack, path: req.path, method: req.method, body: req.body, query: req.query }); const isDevelopment = process.env.NODE_ENV === 'development'; if (err instanceof HttpError) { return res.status(err.status).json({ error: err.message, ...(isDevelopment && { stack: err.stack }) }); } // Handle TypeError from unsafe property access if (err instanceof TypeError && err.message.includes("Cannot read")) { return res.status(500).json({ error: 'Data access error', ...(isDevelopment && { details: err.message, stack: err.stack }) }); } // Generic error response res.status(500).json({ error: 'Internal server error', ...(isDevelopment && { message: err.message, stack: err.stack }) }); } // Handle unhandled promise rejections export function setupUnhandledRejectionHandler() { process.on('unhandledRejection', (reason, promise) => { console.error('[Unhandled Rejection] at:', promise, 'reason:', reason); // Send to monitoring service // monitoringService.captureException(reason); }); process.on('uncaughtException', (error) => { console.error('[Uncaught Exception]', error); // Send to monitoring service // monitoringService.captureException(error); // Graceful shutdown cleanup(); process.exit(1); }); } async function cleanup() { // Close database connections await AppDataSource.destroy(); // Close other resources } ``` --- ## Summary Statistics | Issue Type | Count | Severity | |------------|-------|----------| | Unhandled External API Call (Silent Failure) | 2 | CRITICAL | | Unsafe Array Access (Null Pointer Risk) | 2 | CRITICAL | | Missing Error Handling in DB Operations | 12 | HIGH | | Complex Query Without Timeout/Null Check | 2 | MEDIUM | | Data Inconsistency Risk | 4 | HIGH | --- ## Files Requiring Immediate Attention 1. ✅ `src/controllers/ProfileEditController.ts` - CRITICAL (Line 336, 360) 2. ✅ `src/controllers/ProfileEditEmployeeController.ts` - CRITICAL (Line 337, 360) 3. ✅ `src/controllers/ProfileDisciplineController.ts` - HIGH (Line 167) 4. ✅ `src/controllers/ProfileDisciplineEmployeeController.ts` - HIGH (Line 172) 5. ✅ `src/controllers/ProfileDisciplineEmployeeTempController.ts` - HIGH (Line 162) 6. ✅ `src/controllers/ProfileDutyController.ts` - HIGH (Line 143) 7. ✅ `src/controllers/ProfileDutyEmployeeController.ts` - HIGH (Line 152) 8. ✅ `src/controllers/ProfileDutyEmployeeTempController.ts` - HIGH (Line 141) --- ## Priority Recommendations ### P0 (Immediate Action Required) 1. Fix unsafe array access with non-null assertion (`!`) 2. Add proper error handling for external API calls (CallAPI) 3. Implement transaction pattern for multi-step operations ### P1 (This Sprint) 4. Add error handling for all database save operations 5. Implement query timeout for complex queries 6. Add input validation for query parameters ### P2 (Next Sprint) 7. Implement async event queue for external API calls 8. Add comprehensive monitoring and alerting 9. Implement circuit breaker pattern for external services --- ## Testing Recommendations 1. **Unit Tests**: Test null/undefined scenarios for array access 2. **Integration Tests**: Test external API failure scenarios 3. **Load Tests**: Test query performance with large datasets 4. **Chaos Testing**: Test behavior when external services are down 5. **Data Consistency Tests**: Verify transaction rollback behavior --- **Report Generated:** 2026-05-08 **Batch:** 09 (Controllers 81-90) **Total Files Analyzed:** 10 **Critical Issues Found:** 5 **High Priority Issues:** 14