hrms-api-org/reports/batch-09-controllers-81-90-analysis.md

# 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