# Batch 08: Controllers 71-80 Analysis - Unhandled Exception & Crash Loop Risks

## Executive Summary
พบจุดเสี่ยงระดับ **CRITICAL** ที่อาจทำให้เกิด **Unhandled Exception** และ **Crash Loop** ในระบบ Microservices จำนวน **8 จุด** จากการตรวจสอบ 10 Controllers ในชุดที่ 8

---

## Critical Issues Found

### 1. **CRITICAL** - Unhandled External API Call in ProfileController.ts

#### **File & Location**
- **File:** `src/controllers/ProfileController.ts`
- **Methods:**
  - Line 484-499: `getSalaryProfile()` method
  - Line 977-992: Similar pattern in another method

#### **Problem Type**
1. **Unhandled Exception**
2. **Silent Error Swallowing**

#### **Root Cause**
```typescript
// Line 484-499
await Promise.all(
  await profiles.profileAvatars.slice(-7).map(async (x, i) => {
    if (x == null) {
      _ImgUrl[i] = null;
    } else {
      const url = process.env.API_URL + `/salary/file/${x?.avatar}/${x?.avatarName}`;
      try {
        const response_ = await axios.get(url, {
          headers: {
            Authorization: `${token_}`,
            "Content-Type": "application/json",
            api_key: process.env.API_KEY,
          },
        });
        _ImgUrl[i] = response_.data.downloadUrl;
      } catch {}  // ❌ SILENT ERROR - Empty catch block
    }
  })
);
```

**รายละเอียดปัญหา:**
1. **Empty catch block**: มีการใช้ `catch {}` ว่างเปล่า ทำให้ไม่ทราบว่าเกิด Error 什么
2. **Unhandled Promise rejection**: หาก axios.get throw exception ภายใน Promise.all อาจทำให้เกิด Unhandled Promise Rejection
3. **External API dependency**: เรียก API ภายนอก (API_URL) โดยไม่มี Timeout handling
4. **No retry logic**: ไม่มีการ retry เมื่อเกิด Error

**ผลกระทบ:**
- หาก External API ล่มหรือ Timeout อาจทำให้ Request ค้างอยู่นาน
- ไม่มี Logging ทำให้ยากต่อการ Debug
- อาจทำให้ Memory Leak หาก Promise ไม่ resolve

---

### 2. **CRITICAL** - Incorrect Error Handling Pattern in updateName() Function

#### **File & Location**
- **File:** `src/controllers/ProfileChangeNameController.ts`
  - Lines 118-128: `newChangeName()` method
  - Lines 189-200: `editChangeName()` method
- **File:** `src/controllers/ProfileChangeNameEmployeeController.ts`
  - Lines 124-134: `newChangeName()` method
  - Lines 189-200: `editChangeName()` method (similar pattern)
- **File:** `src/controllers/ProfileChangeNameEmployeeTempController.ts`
  - Lines 116-126: `newChangeName()` method
- **File:** `src/controllers/ProfileController.ts`
  - Lines 5473-5483: Update profile method
  - Lines 5792-5802: Update profile method

#### **Problem Type**
1. **Unhandled Exception**
2. **Type Error Risk**

#### **Root Cause**
```typescript
// Pattern found across multiple controllers
if (profile != null && profile.keycloak != null && profile.isDelete === false) {
  const result = await updateName(
    profile.keycloak,
    profile.firstName,
    profile.lastName,
    profile.prefix,
  );
  if (!result) {
    throw new Error(result.errorMessage);  // ❌ CRITICAL BUG
  }
}
```

**รายละเอียดปัญหา:**
1. **Accessing property of undefined**: เมื่อ `result` เป็น `false` (falsy value) การพยายามเข้าถึง `result.errorMessage` จะทำให้เกิด TypeError
2. **Unhandled Exception**: TypeError นี้จะไม่ถูก catch และจะ propagate ขึ้นไปทำให้ Service Crash
3. **Inconsistent return type**: ฟังก์ชัน `updateName()` ใน `src/keycloak/index.ts` ส่งค่ากลับเป็น `false`, `true`, `id`, หรือ `object with errorMessage` (ไม่ consistent)

**ตรวจสอบฟังก์ชัน updateName():**
```typescript
// src/keycloak/index.ts:525-533
if (!res) return false;
if (!res.ok) {
  return await res.json();  // Returns error object with errorMessage
}
const path = res.headers.get("Location");
const id = path?.split("/").at(-1);
return id || true;  // Returns string ID or true
```

**ผลกระทบ:**
- **CRASH LOOP**: เมื่อ Keycloak API คืนค่า error จะเกิด TypeError และทำให้ Process Crash
- ข้อมูลใน Database ถูกบันทึกแล้ว แต่ Keycloak ไม่ได้ถูก update (Data Inconsistency)

---

### 3. **HIGH** - Missing Error Handling in Promise.all() Operations

#### **File & Location**
- **File:** `src/controllers/ProfileCertificateEmployeeTempController.ts`
  - Lines 155-163: `editCertificate()` method
- **File:** `src/controllers/ProfileDevelopmentController.ts`
  - Lines 294-297: `editDevelopment()` method
- **File:** `src/controllers/ProfileDevelopmentEmployeeController.ts`
  - Lines 237-240: `editDevelopment()` method

#### **Problem Type**
1. **Missing Error Handle**
2. **Data Consistency Risk**

#### **Root Cause**
```typescript
// Example from ProfileCertificateEmployeeTempController.ts:155-163
await Promise.all([
  this.certificateRepo.save(record, { data: req }),
  setLogDataDiff(req, { before, after: record }),
  this.certificateHistoryRepository.save(history, { data: req }),
]);
```

**รายละเอียดปัญหา:**
1. **Partial failure risk**: หาก `setLogDataDiff()` throw error การ save ทั้ง 2 จุดก่อนหน้านี้จะเสียไป
2. **No transaction**: ไม่มีการใช้ Transaction ในการ save ข้อมูลหลายตาราง
3. **Orphaned data**: อาจเกิดข้อมูลปนกันระหว่าง production และ history

---

### 4. **MEDIUM** - StructuredClone Potential Memory Issue

#### **File & Location**
- **Multiple Controllers**: ใช้ `structuredClone()` กับ object ขนาดใหญ่
- **Example:** `ProfileChangeNameController.ts:137`, `ProfileDevelopmentController.ts:349`

#### **Problem Type**
1. **Memory Issue**
2. **Performance Risk**

#### **Root Cause**
```typescript
const before = structuredClone(record); // record อาจมีขนาดใหญ่
```

**รายละเอียดปัญหา:**
- `structuredClone()` ใช้เวลาและ memory มากกับ object ขนาดใหญ่
- อาจทำให้เกิด Memory Heap Overflow ใน Production

---

## Recommended Fixes

### Fix 1: ProfileController.ts - External API Call with Proper Error Handling

**Before:**
```typescript
try {
  const response_ = await axios.get(url, {
    headers: {
      Authorization: `${token_}`,
      "Content-Type": "application/json",
      api_key: process.env.API_KEY,
    },
  });
  _ImgUrl[i] = response_.data.downloadUrl;
} catch {}  // ❌ Empty catch
```

**After:**
```typescript
try {
  const response_ = await axios.get(url, {
    headers: {
      Authorization: `${token_}`,
      "Content-Type": "application/json",
      api_key: process.env.API_KEY,
    },
    timeout: 5000,  // Add timeout
  });
  _ImgUrl[i] = response_.data.downloadUrl;
} catch (error) {
  console.error(`Failed to fetch avatar ${x?.avatar}:`, error.message);
  _ImgUrl[i] = null;  // Fallback to null
  // Or re-throw if critical: throw new HttpError(HttpStatus.SERVICE_UNAVAILABLE, "Avatar service unavailable");
}
```

---

### Fix 2: Incorrect Error Handling Pattern - ALL Controllers

**Before:**
```typescript
const result = await updateName(
  profile.keycloak,
  profile.firstName,
  profile.lastName,
  profile.prefix,
);
if (!result) {
  throw new Error(result.errorMessage);  // ❌ TypeError when result is false
}
```

**After:**
```typescript
const result = await updateName(
  profile.keycloak,
  profile.firstName,
  profile.lastName,
  profile.prefix,
);

// Check result type properly
if (result === false || (result && result.errorMessage)) {
  const errorMessage = result?.errorMessage || 'Failed to update name in Keycloak';
  console.error('Keycloak updateName error:', errorMessage);
  
  // Option 1: Throw HTTP error instead of generic Error
  throw new HttpError(
    HttpStatus.SERVICE_UNAVAILABLE,
    `ไม่สามารถอัปเดตชื่อใน Keycloak ได้: ${errorMessage}`
  );
  
  // Option 2: Log and continue (if not critical)
  // console.warn(`Keycloak update failed for user ${profile.keycloak}: ${errorMessage}`);
  // Don't throw - just log the error
}
```

**OR** Fix the keycloak function to return consistent type:

```typescript
// src/keycloak/index.ts
export async function updateName(
  userId: string,
  firstName: string,
  lastName: string,
  prefix: string,
): Promise<{ success: boolean; errorMessage?: string }> {
  try {
    const existingUser = await getUser(userId);
    if (!existingUser) {
      return { success: false, errorMessage: `User ${userId} not found` };
    }

    const updatedUser = {
      ...existingUser,
      firstName,
      lastName,
      attributes: {
        ...(existingUser.attributes || {}),
        prefix,
      },
    };

    const res = await fetch(`${KC_URL}/admin/realms/${KC_REALMS}/users/${userId}`, {
      headers: {
        "authorization": `Bearer ${await getToken()}`,
        "content-type": `application/json`,
      },
      method: "PUT",
      body: JSON.stringify(updatedUser),
    });

    if (!res.ok) {
      const errorData = await res.json();
      return { success: false, errorMessage: errorData.message || 'Update failed' };
    }

    return { success: true };
  } catch (error) {
    return { success: false, errorMessage: error.message };
  }
}
```

---

### Fix 3: Add Transaction Support for Multi-Table Operations

**Before:**
```typescript
await Promise.all([
  this.certificateRepo.save(record, { data: req }),
  setLogDataDiff(req, { before, after: record }),
  this.certificateHistoryRepository.save(history, { data: req }),
]);
```

**After:**
```typescript
try {
  await AppDataSource.transaction(async (transactionalEntityManager) => {
    await transactionalEntityManager.save(ProfileCertificate, record);
    await transactionalEntityManager.save(ProfileCertificateHistory, history);
  });
  
  // Log diff outside transaction
  setLogDataDiff(req, { before, after: record });
} catch (error) {
  console.error('Failed to save certificate:', error);
  throw new HttpError(
    HttpStatus.INTERNAL_SERVER_ERROR,
    'ไม่สามารถบันทึกข้อมูลได้ กรุณาลองใหม่'
  );
}
```

---

### Fix 4: Add Global Error Handler for Unhandled Exceptions

**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]', err);

  // Don't leak error details in production
  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 result.errorMessage pattern
  if (err instanceof TypeError && err.message.includes("errorMessage")) {
    return res.status(500).json({
      error: 'External service error',
      ...(isDevelopment && { details: err.message })
    });
  }

  // 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);
    // Don't crash the process
    // Log to monitoring service instead
  });

  process.on('uncaughtException', (error) => {
    console.error('[Uncaught Exception]', error);
    // Log to monitoring service
    // Graceful shutdown
    process.exit(1);
  });
}
```

---

## Summary Statistics

| Issue Type | Count | Severity |
|------------|-------|----------|
| Unhandled External API Call | 2 | CRITICAL |
| Incorrect Error Handling (TypeError Risk) | 8 | CRITICAL |
| Missing Transaction Support | 6 | HIGH |
| Silent Error Swallowing | 2 | MEDIUM |
| Memory/Performance Risk | Multiple | MEDIUM |

---

## Files Requiring Immediate Attention

1. ✅ `src/controllers/ProfileController.ts` - CRITICAL (Line 484, 5473, 5792)
2. ✅ `src/controllers/ProfileChangeNameController.ts` - CRITICAL (Line 118, 189)
3. ✅ `src/controllers/ProfileChangeNameEmployeeController.ts` - CRITICAL (Line 124, 189)
4. ✅ `src/controllers/ProfileChangeNameEmployeeTempController.ts` - CRITICAL (Line 116)
5. ✅ `src/keycloak/index.ts` - CRITICAL (Need to fix return type consistency)

---

## Priority Recommendations

### P0 (Immediate Action Required)
1. Fix the `result.errorMessage` TypeError pattern across all controllers
2. Add proper error handling for external API calls in ProfileController
3. Implement global error handler for unhandled exceptions

### P1 (This Sprint)
4. Add transaction support for multi-table operations
5. Implement retry logic for external API calls
6. Add proper logging and monitoring

### P2 (Next Sprint)
7. Review memory usage with structuredClone()
8. Add circuit breaker pattern for external services
9. Implement comprehensive error tracking

---

## Testing Recommendations

1. **Unit Tests**: Test error scenarios for Keycloak integration
2. **Integration Tests**: Test external API failure scenarios
3. **Load Tests**: Test memory usage with large profile data
4. **Chaos Testing**: Test behavior when external services are down

---

**Report Generated:** 2026-05-08
**Batch:** 08 (Controllers 71-80)
**Total Files Analyzed:** 10
**Critical Issues Found:** 8