# รายงานการตรวจสอบ Unhandled Exception - Controllers ชุดที่ 2 (ไฟล์ที่ 11-20) **Project:** BMA EHR Organization Backend **Framework:** TSOA + Express + TypeORM **วันที่ตรวจสอบ:** 2026-05-08 **จำนวน Controllers:** 10 ไฟล์ **สถานะ:** เสร็จสิ้น --- ## สรุปผลการตรวจสอบ | ระดับความรุนแรง | จำนวนจุดเสี่ยง | |---------------------|-------------------| | **CRITICAL** | 1 | | **HIGH** | 3 | | **MEDIUM** | 4 | | **LOW** | 2 | | **BUG** | 2 | | **รวมทั้งหมด** | 12 | --- ## Controllers ที่ตรวจสอบ 11. [CommandOperatorController.ts](src/controllers/CommandOperatorController.ts) 12. [CommandSalaryController.ts](src/controllers/CommandSalaryController.ts) 13. [CommandSysController.ts](src/controllers/CommandSysController.ts) 14. [CommandTypeController.ts](src/controllers/CommandTypeController.ts) 15. [DPISController.ts](src/controllers/DPISController.ts) 16. [DevelopmentRequestController.ts](src/controllers/DevelopmentRequestController.ts) 17. [DistrictController.ts](src/controllers/DistrictController.ts) 18. [EducationLevelController.ts](src/controllers/EducationLevelController.ts) 19. [EmployeePosLevelController.ts](src/controllers/EmployeePosLevelController.ts) 20. [EmployeePosTypeController.ts](src/controllers/EmployeePosTypeController.ts) --- ## รายละเอียดจุดเสี่ยงแต่ละจุด ### #1 - Transaction QueryRunner Not Released on Error (CRITICAL) **File & Location:** [CommandOperatorController.ts:169-222](src/controllers/CommandOperatorController.ts#L169-L222) **Method:** `deleteCommandOperator` **Problem Type:** 1. Unhandled Exception / 2. Missing Error Handle **Root Cause:** - ใช้ QueryRunner และ Transaction แต่มี error handling ที่ไม่ปลอดภัย - ถ้าเกิด error หลังจาก `throw error` ใน catch block แล้ว จะไม่ถึง `finally` block - QueryRunner จะไม่ถูก release ทำให้ connection leak - ในกรณีที่ HttpError ถูก throw ภายใน catch จะไม่มีการ release queryRunner **Code ปัจจุบัน (เสี่ยง):** ```typescript const queryRunner = AppDataSource.createQueryRunner(); await queryRunner.connect(); await queryRunner.startTransaction(); try { // ... operations await queryRunner.commitTransaction(); return new HttpSuccess(true); } catch (error) { await queryRunner.rollbackTransaction(); throw error; // ❌ ถ้า throw HttpError จะไม่ถึง finally } finally { await queryRunner.release(); // ❌ จะไม่ถูกเรียกถ้า throw error ใน catch } ``` **Recommended Fix:** ```typescript const queryRunner = AppDataSource.createQueryRunner(); try { await queryRunner.connect(); await queryRunner.startTransaction(); try { // 1. หา operator const operator = await queryRunner.manager.findOne(CommandOperator, { where: { id: operatorId, commandId: commandId, }, }); if (!operator) { throw new HttpError(HttpStatusCode.NOT_FOUND, "ไม่พบเจ้าหน้าที่ดำเนินการ"); } const removedOrderNo = operator.orderNo; // 3. ลบ await queryRunner.manager.remove(operator); // 4. re orderNumber ตัวที่เหลือ await queryRunner.manager .createQueryBuilder() .update(CommandOperator) .set({ orderNo: () => "orderNo - 1", }) .where("commandId = :commandId", { commandId }) .andWhere("orderNo > :removedOrderNo", { removedOrderNo }) .execute(); await queryRunner.commitTransaction(); return new HttpSuccess(true); } catch (error) { await queryRunner.rollbackTransaction(); // Re-throw after rollback throw error; } } finally { // ✅ ใช้ finally block ระดับนอกสุดเพื่อให้แน่ใจว่าจะถูกเรียกเสมอ if (queryRunner.isReleased) { // Already released, skip } else { await queryRunner.release(); } } ``` --- ### #2 - Promise.all Without Error Handling in Development Request (HIGH) **File & Location:** [DevelopmentRequestController.ts:349-364](src/controllers/DevelopmentRequestController.ts#L349-L364) **Method:** `newDevelopmentRequest` **Problem Type:** 1. Unhandled Exception / 2. Missing Error Handle **Root Cause:** - `Promise.all()` กับการบันทึก development projects หลายรายการ - ถ้ามี project ไหน save ไม่สำเร็จ จะทำให้ unhandled rejection - ไม่มี try-catch รองรับ - External API call ใช้ `.catch()` แต่ไม่ได้ throw error ต่อ **Code ปัจจุบัน (เสี่ยง):** ```typescript if (body.developmentProjects != null) { await Promise.all( body.developmentProjects.map(async (x) => { let developmentProject = new DevelopmentProject(); developmentProject.name = x; developmentProject.createdUserId = req.user.sub; developmentProject.createdFullName = req.user.name; developmentProject.lastUpdateUserId = req.user.sub; developmentProject.lastUpdateFullName = req.user.name; developmentProject.createdAt = new Date(); developmentProject.lastUpdatedAt = new Date(); developmentProject.developmentRequestId = data.id; await this.developmentProjectRepository.save(developmentProject, { data: req }); setLogDataDiff(req, { before, after: developmentProject }); }), ); } await new CallAPI() .PostData(req, "/org/workflow/add-workflow", { refId: data.id, sysName: "REGISTRY_IDP", 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); }); ``` **Recommended Fix:** ```typescript if (body.developmentProjects != null) { try { await Promise.all( body.developmentProjects.map(async (x) => { try { let developmentProject = new DevelopmentProject(); developmentProject.name = x; developmentProject.createdUserId = req.user.sub; developmentProject.createdFullName = req.user.name; developmentProject.lastUpdateUserId = req.user.sub; developmentProject.lastUpdateFullName = req.user.name; developmentProject.createdAt = new Date(); developmentProject.lastUpdatedAt = new Date(); developmentProject.developmentRequestId = data.id; await this.developmentProjectRepository.save(developmentProject, { data: req }); setLogDataDiff(req, { before, after: developmentProject }); } catch (error) { console.error(`Failed to save development project "${x}":`, error); throw error; // Re-throw to be caught by Promise.all } }), ); } catch (error) { console.error("Failed to save some development projects:", error); throw new HttpError( HttpStatus.INTERNAL_SERVER_ERROR, "Failed to save development projects" ); } } // Call external API with proper error handling try { await new CallAPI().PostData(req, "/org/workflow/add-workflow", { refId: data.id, sysName: "REGISTRY_IDP", 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 call workflow API:", error); // Optionally mark the request as having workflow issues // But don't fail the entire request } ``` --- ### #3 - QueryBuilder with Dynamic Conditions Without Error Handling (HIGH) **File & Location:** [DevelopmentRequestController.ts:122-265](src/controllers/DevelopmentRequestController.ts#L122-L265) **Method:** `getDevelopmentRequestAdmin` **Problem Type:** 2. Missing Error Handle **Root Cause:** - Complex QueryBuilder พร้อม dynamic conditions หลายอย่าง - ไม่มี try-catch รองรับ - Permission check อาจ throw error - Null reference risks หลายจุด (`orgRevisionPublish?.id`, `data.root`, etc.) **Recommended Fix:** ```typescript @Get("admin") public async getDevelopmentRequestAdmin( @Request() request: RequestWithUser, @Query("status") status: string, @Query("keyword") keyword: string = "", @Query("page") page: number = 1, @Query("pageSize") pageSize: number = 10, @Query("sortBy") sortBy?: string, @Query("descending") descending?: boolean, ) { try { // Validate inputs if (page < 1) throw new HttpError(HttpStatus.BAD_REQUEST, "Invalid page number"); if (pageSize < 1 || pageSize > 1000) { throw new HttpError(HttpStatus.BAD_REQUEST, "Invalid page size"); } let data = await new permission().PermissionOrgList(request, "SYS_REGISTRY_OFFICER"); const orgRevisionPublish = await this.orgRevisionRepository .createQueryBuilder("orgRevision") .where("orgRevision.orgRevisionIsDraft = false") .andWhere("orgRevision.orgRevisionIsCurrent = true") .getOne(); let query = await AppDataSource.getRepository(DevelopmentRequest) .createQueryBuilder("developmentRequest") .leftJoinAndSelect("developmentRequest.profile", "profile") .leftJoinAndSelect("profile.current_holders", "current_holders") .leftJoinAndSelect("current_holders.orgRevision", "orgRevision") .andWhere( status == undefined || status.trim().toUpperCase() == "ALL" || status == "" ? "1=1" : "developmentRequest.status = :status", { status: status == undefined || status == null ? "" : status.trim().toUpperCase(), }, ) .andWhere( orgRevisionPublish ? `current_holders.orgRevisionId = :revisionId` : "1=1", { revisionId: orgRevisionPublish?.id, }, ) // ... rest of the query const [lists, total] = await query .skip((page - 1) * pageSize) .take(pageSize) .getManyAndCount(); const _data = lists.map((item) => ({ ...item, profile: null })); return new HttpSuccess({ data: _data, total }); } catch (error) { if (error instanceof HttpError) { throw error; } console.error('Failed to get development requests:', error); throw new HttpError( HttpStatus.INTERNAL_SERVER_ERROR, "Failed to retrieve development requests" ); } } ``` --- ### #4 - Promise.all in Edit Development Request Without Error Handling (HIGH) **File & Location:** [DevelopmentRequestController.ts:402-417](src/controllers/DevelopmentRequestController.ts#L402-L417) **Method:** `editUserDevelopmentRequest` **Problem Type:** 1. Unhandled Exception / 2. Missing Error Handle **Root Cause:** - ใช้ `Promise.all()` โดยไม่มี error handling - Similar to #2 but in edit operation **Code ปัจจุบัน (เสี่ยง):** ```typescript await this.developmentProjectRepository.delete({ developmentRequestId: record.id }); if (body.developmentProjects != null) { await Promise.all( body.developmentProjects.map(async (x) => { let developmentProject = new DevelopmentProject(); developmentProject.name = x; developmentProject.createdUserId = req.user.sub; developmentProject.createdFullName = req.user.name; developmentProject.lastUpdateUserId = req.user.sub; developmentProject.lastUpdateFullName = req.user.name; developmentProject.createdAt = new Date(); developmentProject.lastUpdatedAt = new Date(); developmentProject.developmentRequestId = record.id; await this.developmentProjectRepository.save(developmentProject, { data: req }); setLogDataDiff(req, { before: null, after: record }); }), ); } ``` **Recommended Fix:** ```typescript await this.developmentProjectRepository.delete({ developmentRequestId: record.id }); if (body.developmentProjects != null) { try { await Promise.all( body.developmentProjects.map(async (x) => { try { let developmentProject = new DevelopmentProject(); developmentProject.name = x; developmentProject.createdUserId = req.user.sub; developmentProject.createdFullName = req.user.name; developmentProject.lastUpdateUserId = req.user.sub; developmentProject.lastUpdateFullName = req.user.name; developmentProject.createdAt = new Date(); developmentProject.lastUpdatedAt = new Date(); developmentProject.developmentRequestId = record.id; await this.developmentProjectRepository.save(developmentProject, { data: req }); setLogDataDiff(req, { before: null, after: developmentProject }); } catch (error) { console.error(`Failed to update development project "${x}":`, error); throw error; } }), ); } catch (error) { console.error("Failed to update development projects:", error); throw new HttpError( HttpStatus.INTERNAL_SERVER_ERROR, "Failed to update development projects" ); } } ``` --- ### #5 - Null Reference Risk in Profile Query (MEDIUM) **File & Location:** [DPISController.ts:272-275](src/controllers/DPISController.ts#L272-L275) **Method:** `GetProfileCitizenIdAsync` **Problem Type:** 2. Missing Error Handle **Root Cause:** - `findRevision` อาจเป็น null ถ้าไม่พบ current revision - การใช้ `findRevision?.id` ใน `find()` operation จะเป็น undefined - `current_holders?.find()` อาจ return undefined **Code ปัจจุบัน (เสี่ยง):** ```typescript const findRevision = await this.orgRevisionRepo.findOne({ where: { orgRevisionIsCurrent: true }, }); var current_holder = profile.current_holders?.find((x) => x.orgRevisionId == findRevision?.id); const mapProfile: DPISResult = { // ... organization: { orgRootName: current_holder?.orgRoot?.orgRootName || "", // ❌ multiple levels of null checks orgChild1Name: current_holder?.orgChild1?.orgChild1Name || "", // ... }, }; ``` **Recommended Fix:** ```typescript const findRevision = await this.orgRevisionRepo.findOne({ where: { orgRevisionIsCurrent: true }, }); if (!findRevision) { throw new HttpError( HttpStatus.NOT_FOUND, "No current organization revision found" ); } var current_holder = profile.current_holders?.find((x) => x.orgRevisionId == findRevision.id); if (!current_holder) { throw new HttpError( HttpStatus.NOT_FOUND, "No current organization assignment found for this profile" ); } const mapProfile: DPISResult = { // ... organization: { orgRootName: current_holder.orgRoot?.orgRootName || "", orgChild1Name: current_holder.orgChild1?.orgChild1Name || "", orgChild2Name: current_holder.orgChild2?.orgChild2Name || "", orgChild3Name: current_holder.orgChild3?.orgChild3Name || "", orgChild4Name: current_holder.orgChild4?.orgChild4Name || "", }, }; ``` --- ### #6 - Unsafe Optional Chain in OrgRoot Query (MEDIUM) **File & Location:** [DevelopmentRequestController.ts:322-330](src/controllers/DevelopmentRequestController.ts#L322-L330) **Method:** `newDevelopmentRequest` **Problem Type:** 2. Missing Error Handle **Root Cause:** - ใช้ optional chaining (`?.`) และ nullish coalescing ใน query - `find()` อาจ return undefined และการใช้ `!` (non-null assertion) อาจทำให้ runtime error **Code ปัจจุบัน (เสี่ยง):** ```typescript const orgRoot = await this.orgRootRepo.findOne({ select: { id: true, isDeputy: true }, where: { id: profile.current_holders.find(x => x.orgRootId)!.orgRootId ?? "" // ❌ unsafe } }) ``` **Recommended Fix:** ```typescript const currentHolder = profile.current_holders.find(x => x.orgRootId); if (!currentHolder || !currentHolder.orgRootId) { throw new HttpError( HttpStatus.BAD_REQUEST, "Profile must have a current organization assignment" ); } const orgRoot = await this.orgRootRepo.findOne({ select: { id: true, isDeputy: true }, where: { id: currentHolder.orgRootId } }); if (!orgRoot) { throw new HttpError( HttpStatus.NOT_FOUND, "Organization root not found" ); } ``` --- ### #7 - Promise.all in Admin Edit Without Error Handling (MEDIUM) **File & Location:** [DevelopmentRequestController.ts:467-490](src/controllers/DevelopmentRequestController.ts#L467-L490) **Method:** `editAdminDevelopmentRequest` **Problem Type:** 1. Unhandled Exception / 2. Missing Error Handle **Root Cause:** - `Promise.all()` กับ nested save operations - ไม่มี error handling สำหรับ individual promises **Recommended Fix:** ```typescript if (record.developmentProjects != null) { try { await Promise.all( record.developmentProjects.map(async (x) => { try { let developmentProject = new DevelopmentProject(); let developmentProjectHistory = new DevelopmentProject(); Object.assign(developmentProject, { ...meta, id: undefined, name: record.name, profileDevelopmentId: profileDevelopment.id, }); Object.assign(developmentProject, { ...meta, id: undefined, name: record.name, profileDevelopmentHistoryId: history.id, }); await Promise.all([ this.developmentProjectRepository.save(developmentProject, { data: req }), setLogDataDiff(req, { before: null, after: developmentProject }), this.developmentProjectRepository.save(developmentProjectHistory, { data: req }), ]); } catch (error) { console.error(`Failed to save development project for "${record.name}":`, error); throw error; } }), ); } catch (error) { console.error("Failed to save development projects:", error); throw new HttpError( HttpStatus.INTERNAL_SERVER_ERROR, "Failed to save development projects" ); } } ``` --- ### #8 - QueryBuilder Parameters Without Validation (MEDIUM) **File & Location:** [CommandSalaryController.ts:73-108](src/controllers/CommandSalaryController.ts#L73-L108) **Method:** `GetAdmin` **Problem Type:** 2. Missing Error Handle **Root Cause:** - QueryBuilder พร้อม dynamic conditions - ไม่มี input validation - Page number validation เป็น optional (มี default value แต่ไม่ validate range) **Recommended Fix:** ```typescript @Get("admin") async GetAdmin( @Query("page") page: number = 1, @Query("pageSize") pageSize: number = 10, @Query() commandSysId?: string | null, @Query() isActive?: boolean | null, @Query() searchKeyword?: string | null, ) { try { // Validate inputs if (page < 1 || page > 10000) { throw new HttpError(HttpStatus.BAD_REQUEST, "Invalid page number"); } if (pageSize < 1 || pageSize > 1000) { throw new HttpError(HttpStatus.BAD_REQUEST, "Invalid page size"); } const [commandSalarys, total] = await this.commandSalaryRepository .createQueryBuilder("commandSalary") .andWhere( isActive != null && isActive != undefined ? "commandSalary.isActive = :isActive" : "1=1", { isActive: isActive == null || isActive == undefined ? null : `${isActive == true ? 1 : 0}`, }, ) // ... rest of query .getManyAndCount(); return new HttpSuccess({ commandSalarys, total }); } catch (error) { if (error instanceof HttpError) { throw error; } console.error('Failed to get command salaries:', error); throw new HttpError( HttpStatus.INTERNAL_SERVER_ERROR, "Failed to retrieve command salaries" ); } } ``` --- ### #9 - Hardcoded Response Data (LOW) **File & Location:** [CommandTypeController.ts:140-199](src/controllers/CommandTypeController.ts#L140-L199) **Method:** `GetById` **Problem Type:** 3. Code Quality **Root Cause:** - Hardcoded template data ใน code - ไม่ flexible และยากต่อการ maintain - ควรเก็บใน database หรือ configuration **Code ปัจจุบัน (เสี่ยง):** ```typescript if (_commandType.code == "C-PM-10") { let _commandType10: any; _commandType10 = { // ... hardcoded fields name1: "๑. ..........................ประธาน", name2: "๒. ..........................กรรมการ", // ... }; _commandType = _commandType10; } else if (["C-PM-21", "C-PM-23"].includes(_commandType.code)) { let _commandType21and23: any; _commandType21and23 = { // ... hardcoded fields persons: [ { no: "", org: "", // ... }, ], }; _commandType = _commandType21and23; } ``` **Recommended Fix:** ```typescript // Move these templates to database or configuration const commandTemplates = await this.commandTemplateRepository.find({ where: { commandTypeCode: _commandType.code } }); if (commandTemplates.length > 0) { const template = commandTemplates[0]; return new HttpSuccess({ ..._commandType, ...template.templateData }); } return new HttpSuccess(_commandType); ``` --- ### #10 - Missing Transaction for Related Operations (LOW) **File & Location:** [CommandOperatorController.ts:109-112](src/controllers/CommandOperatorController.ts#L109-L112) **Method:** `swapCommandOperator` **Problem Type:** 2. Missing Error Handle **Root Cause:** - มีการ swap orderNo ระหว่าง 2 records - ไม่ได้ใช้ transaction - ถ้า save ตัวแรกสำเร็จ แต่ตัวที่สอง fail จะเกิด data inconsistency **Code ปัจจุบัน (เสี่ยง):** ```typescript // swap const temp = source.orderNo; source.orderNo = dest.orderNo; dest.orderNo = temp; await Promise.all([ this.commandOperatorRepo.save(source), this.commandOperatorRepo.save(dest), ]); ``` **Recommended Fix:** ```typescript const queryRunner = AppDataSource.createQueryRunner(); try { await queryRunner.connect(); await queryRunner.startTransaction(); // swap const temp = source.orderNo; source.orderNo = dest.orderNo; dest.orderNo = temp; await Promise.all([ queryRunner.manager.save(CommandOperator, source), queryRunner.manager.save(CommandOperator, dest), ]); await queryRunner.commitTransaction(); return new HttpSuccess(); } catch (error) { await queryRunner.rollbackTransaction(); console.error('Failed to swap command operators:', error); throw new HttpError( HttpStatus.INTERNAL_SERVER_ERROR, "Failed to swap operator order" ); } finally { await queryRunner.release(); } ``` --- ### #11 - Wrong Error Status Code (BUG) **File & Location:** [Multiple Files - CommandSysController.ts:127](src/controllers/CommandSysController.ts#L127), [CommandTypeController.ts:216](src/controllers/CommandTypeController.ts#L216), etc. **Methods:** `Post` in various controllers **Problem Type:** 3. Logic Bug **Root Cause:** - Throw `HttpError(HttpStatusCode.NOT_FOUND, ...)` สำหรับ duplicate name errors - ควรใช้ `BAD_REQUEST` หรือ `CONFLICT` แทน **Code ปัจจุบัน (ผิด):** ```typescript if (checkName) { throw new HttpError(HttpStatusCode.NOT_FOUND, "ชื่อนี้มีอยู่ในระบบแล้ว"); // ❌ Wrong status code } ``` **Recommended Fix:** ```typescript if (checkName) { throw new HttpError(HttpStatusCode.CONFLICT, "ชื่อนี้มีอยู่ในระบบแล้ว"); // ✅ Correct status code } ``` --- ### #12 - Typos in Status Field (BUG) **File & Location:** [ChangePositionController.ts:79](src/controllers/ChangePositionController.ts#L79) **Method:** `CreateChangePosition` **Problem Type:** 3. Logic Bug **Root Cause:** - Status เป็น "WAITTING" (ตัว T เกิน) - ควรเป็น "WAITING" **Code ปัจจุบัน (ผิด):** ```typescript changePosition.status = "WAITTING"; // ❌ typo ``` **Recommended Fix:** ```typescript changePosition.status = "WAITING"; // ✅ correct spelling ``` **หมายเหตุ:** ปัญหาเดียวกันนี้พบใน [ChangePositionController.ts:241](src/controllers/ChangePositionController.ts#L241) --- ## สรุปคำแนะนำการแก้ไขแบบรวม ### ระดับความสำคัญ **ต้องแก้ทันที (P0 - Critical):** 1. QueryRunner transaction not released on error - อาจทำให้ connection leak **ควรแก้โดยเร็ว (P1 - High):** 2. Promise.all operations without error handling - unhandled rejections 3. QueryBuilder without validation and error handling **ควรแก้ (P2 - Medium):** 4. Null reference checks 5. Unsafe optional chain usage 6. Promise operations in edit methods **แก้เมื่อว่าง (P3 - Low):** 7. Hardcoded data 8. Missing transaction for related operations ### แนวทางการแก้ไขแบบ Global 1. **Use try-finally pattern** for all QueryRunner operations 2. **Add input validation** for all query parameters 3. **Use Promise.allSettled** หรือ wrap promises in try-catch 4. **Implement proper null checks** ก่อน accessing nested properties 5. **Use transactions** สำหรับ operations ที่ต้องการ consistency --- ## ไฟล์ที่ต้องแก้ไข 1. **src/controllers/CommandOperatorController.ts** - Transaction handling, Promise operations 2. **src/controllers/DevelopmentRequestController.ts** - Promise.all, QueryBuilder validation 3. **src/controllers/DPISController.ts** - Null reference checks 4. **src/controllers/CommandSalaryController.ts** - Input validation 5. **src/controllers/CommandTypeController.ts** - Error status codes, hardcoded data --- ## ข้อมูลเพิ่มเติม - **Controllers ที่ยังไม่ได้ตรวจสอบ:** 120 ไฟล์ - **จุดเสี่ยงที่พบซ้ำจากชุดที่ 1:** Promise.all without error handling, QueryBuilder without error handling --- **รายงานนี้ถูกสร้างโดย AI Code Review System** **สำหรับ BMA EHR Organization Project**