diff --git a/openspec/changes/fix-booking-exception-handling/.openspec.yaml b/openspec/changes/fix-booking-exception-handling/.openspec.yaml new file mode 100644 index 0000000..5c881ba --- /dev/null +++ b/openspec/changes/fix-booking-exception-handling/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-18 diff --git a/openspec/changes/fix-booking-exception-handling/design.md b/openspec/changes/fix-booking-exception-handling/design.md new file mode 100644 index 0000000..36f51d7 --- /dev/null +++ b/openspec/changes/fix-booking-exception-handling/design.md @@ -0,0 +1,65 @@ +# Design + +## Context + +背景見 proposal.md「Why」。現況: + +- `MemberBookingController::store` 的 `DB::transaction` 閉包拋 `\RuntimeException('名額不足,請重新選擇')`、`\RuntimeException('您已預約此時段')`;`ProviderBookingController::confirm` 拋 `\RuntimeException('名額不足,無法確認此預約')`。兩處都以 `catch (\RuntimeException $e)` 回 422 + `$e->getMessage()` +- `bootstrap/app.php` 只有 `Sentry\Laravel\Integration::handles($exceptions)`,沒有自訂 render;未被攔截的例外由 Laravel 預設處理器回應,`APP_DEBUG=false` 時 JSON body 為 `{"message":"Server Error"}` +- 全專案 grep `RuntimeException` 只有這兩個 Controller;其他 `catch (\Throwable)` 都只包通知派送(記 log 後繼續),不在本次範圍 +- `phpunit.xml` 沒有設定 `APP_DEBUG`,測試會沿用 `.env` 的值(本機為 true),因此驗證「不洩漏細節」的測試必須自行設定 `app.debug = false` + +## Goals / Non-Goals + +**Goals** +- 只有刻意拋出的業務例外會變成 422;其他例外一律走 Laravel 例外處理器 + +**Non-Goals** +- 不統一全站 500 的 JSON 格式(不在 `withExceptions` 加自訂 render);500 body 沒有 `status` 欄位,與其他端點未攔截的 500 一致 +- 不處理 M8(鎖之前讀取狀態的併發問題)與 M2(抽出 `BookingService`);本次只改例外型別與 catch 範圍 +- 不調整前端;資料庫錯誤時前端會顯示 `Server Error` +- 不拆分 `booking-lifecycle` 中超過 500 字元的「預約狀態機」requirement(不在本次修改範圍,需要時另開 change) + +## Decisions + +### D1:新增窄型業務例外 `App\Exceptions\BookingRuleViolationException` +- 放在新目錄 `app/Exceptions/`(目前不存在,Laravel 11 慣例位置) +- 繼承 `\RuntimeException`。安全性來自 Controller 只 catch 這個具體類別,而不是父類別的選擇;繼承 `RuntimeException` 讓語意(執行期業務規則失敗)不變 +- 訊息只由程式內字串常值傳入,不包含使用者輸入或其他例外的訊息,因此可以安全回給使用者 +- **替代方案**: + - 在 catch 內判斷 `instanceof QueryException` 後重新拋出:仍會漏接其他非業務的 `RuntimeException`(例如 `PDOException` 以外的驅動錯誤),是黑名單做法,不採用 + - 交易閉包回傳錯誤碼而非拋例外:拋例外才能觸發 `DB::transaction` 的 rollback,改回傳值需要手動處理 rollback,不採用 + +### D2:非業務例外不在 Controller 攔截 +- 依使用者決定:交給 Laravel 例外處理器。好處是 Sentry 透過既有 `Integration::handles` 自動收到,不需在 Controller 呼叫 `report()` +- **替代方案**:Controller 內 catch `\Throwable` + `report()` + 專案格式 500,或在 `withExceptions` 全域統一 JSON 500 格式。前者造成兩個端點格式與其他端點不一致;後者影響全站所有端點,範圍超出 M7。兩者皆不採用 + +### D3:測試方式 +- **觸發資料庫例外**:在測試內註冊 Eloquent model 事件,於交易內拋出 `Illuminate\Database\QueryException`,其 SQL 字串含資料表與欄位名稱。這樣能真的經過 Controller 的 catch 路徑,不需 mock DB facade + - 例外必須發生在交易內**已有一筆寫入之後**,否則「資料沒有變動」在沒有 rollback 時也會成立,無法證明 rollback:建立預約用 `Booking::created`(INSERT 之後);確認預約用 `CourseSchedule::updating`(預約狀態已更新、`increment` 時觸發;Laravel 11 的 `Model::increment` 會觸發 `updating` 事件) +- **斷言 500 body**:以 `assertExactJson(['message' => 'Server Error'])` 鎖住完整 body,避免空 body 也讓「不含 SQL」的斷言通過 +- **觸發 store 交易內的名額不足**:此分支只在併發時發生(交易前已有相同檢查)。測試以 `CourseSchedule::retrieved` 事件,在交易內第二次讀取時段時把 `current_participants` 設為滿額,模擬另一筆預約搶先佔位 +- **斷言回報**:以 `Exceptions::fake()` 搭配 `Exceptions::assertReported(QueryException::class)`;fake 仍委派原處理器 render,因此可同時斷言 500 回應。Sentry 本身是否收到屬於整合設定,CI 無法驗證,以「例外有被回報」作為可測量的替代 +- **斷言不洩漏**:設定 `config(['app.debug' => false])` 後,斷言 body 不含 `insert into`/`update`、`bookings` 與測試用 SQL 中的欄位名稱 +- **業務失敗的 422**:以 `assertExactJson` 斷言完整 body,確保訊息文字沒有被改動;並在 `Exceptions::fake()` 下斷言 `Exceptions::assertNotReported(BookingRuleViolationException::class)`,業務失敗不應出現在 Sentry +- **變異驗證**:暫時把 catch 改回 `\RuntimeException`,確認 500 測試會失敗。這是本機手動步驟、不提交,放在 tasks 的手動驗證 group,不算 Scenario 覆蓋 + +## 安全確認 + +- **輸入驗證**:沿用既有 `$request->validate()`,不變;沒有拼接 SQL 或 shell +- **資料庫存取**:沿用 Eloquent/Query Builder 與 `lockForUpdate`,不變;不新增查詢,無 N+1 變化 +- **權限**:路由不變。`POST /api/member/bookings` 掛 `auth:sanctum` + `member` middleware;`PUT /api/provider/bookings/{id}/confirm` 掛 `auth:sanctum` + `provider` middleware,Controller 內 `authorizeProvider()` 檢查預約屬於呼叫者的課程。本次不改任何權限邏輯 +- **資訊洩漏**:本次的主要目的;回給使用者的訊息只剩 D1 中的程式內常值,以及 Laravel 的通用 500 訊息 +- **正式環境前提**:500 body 不洩漏細節依賴 `APP_DEBUG=false`;2026-10-18 已在 VPS 確認(H2) + +## Risks / Trade-offs + +- [資料庫錯誤的 HTTP 狀態由 422 改為 500,前端與監控會看到更多 5xx] → 這是正確分類;目前前端都以 `e.response?.data?.message` 顯示,不會壞掉,只是文字變成 `Server Error` +- [之後有人在交易閉包內又拋一般 `RuntimeException` 表示業務失敗,會變成 500] → 測試以 `assertExactJson` 鎖住三個業務訊息的 422;新增業務規則時必須使用 `BookingRuleViolationException` +- [死結、逾時這類可重試的暫時性錯誤,使用者只看到 `Server Error`,沒有「請稍後重試」的提示] → 本次接受;若之後要區分可重試錯誤,屬於全站錯誤格式的另一個 change +- [`APP_DEBUG=true` 的環境(本機、若有預備環境)500 body 仍會含例外訊息與 SQL] → 這是 Laravel debug 模式的設計行為,只有正式環境(已確認 `APP_DEBUG=false`)保證不洩漏;任何對外開放的環境都必須設為 false +- [SQLite 測試無法重現真實的死結或逾時] → 測試只需要證明「任何 `QueryException` 都會走 500」,例外類型本身與資料庫驅動無關,SQLite 足以驗證分類邏輯 + +## Migration Plan + +純程式碼變更,無 migration、無設定變更。部署後若需回滾,revert 這個 PR 即可。 diff --git a/openspec/changes/fix-booking-exception-handling/proposal.md b/openspec/changes/fix-booking-exception-handling/proposal.md new file mode 100644 index 0000000..09c22a2 --- /dev/null +++ b/openspec/changes/fix-booking-exception-handling/proposal.md @@ -0,0 +1,29 @@ +# Proposal + +## Why + +`MemberBookingController::store` 與 `ProviderBookingController::confirm` 用 `catch (\RuntimeException $e)` 把交易內的業務失敗轉成 422,並直接回傳 `$e->getMessage()`。`Illuminate\Database\QueryException` 經由 `PDOException` 也是 `RuntimeException` 的子類別,因此交易內的資料庫錯誤(死結、逾時、約束違反、連線中斷)會被同一個 catch 接住:SQL 與資料表結構被當成 422 訊息回給使用者(資訊洩漏),且不經過 Laravel 例外處理器、不會送到 Sentry,營運端看不到伺服器故障(待辦 M7,`docs/analysis/2026-10-14-project-review-todo.md`)。 + +## What Changes + +- 新增專用的業務例外 `BookingRuleViolationException`;兩個預約交易閉包內的業務規則失敗(名額不足、重複預約)改拋此例外 +- 兩個 Controller 只 catch `BookingRuleViolationException` 並回 422 與其訊息;其他例外(含 `QueryException`)不再被攔截,交給 Laravel 例外處理器產生 500 並回報(既有 `Sentry\Laravel\Integration::handles` 會送到 Sentry) +- 業務失敗的 422 狀態碼與訊息文字維持不變 +- 資料庫錯誤的回應由「422 + 原始例外訊息」改為「500 + Laravel 預設通用訊息」(`APP_DEBUG=false` 時 body 為 `{"message":"Server Error"}`) + +## Capabilities + +### New Capabilities +(無) + +### Modified Capabilities +- `booking-lifecycle`:新增「預約交易錯誤分類」requirement,明定業務規則失敗回 422 與既有訊息、非業務例外回 500 且不洩漏資料庫細節並被回報 + +## Impact + +- **程式碼**:新增 `app/Exceptions/BookingRuleViolationException.php`;修改 `app/Http/Controllers/API/MemberBookingController.php`(`store`)、`app/Http/Controllers/API/ProviderBookingController.php`(`confirm`) +- **API 回應格式**:業務失敗(422)不變。資料庫錯誤的回應由 422 改為 500,body 為 Laravel 預設格式 `{"message":"Server Error"}`,**不含**專案慣例的 `status` 欄位——這是刻意選擇(交給 Laravel 處理、不另寫轉換),與其他端點未被攔截的 500 一致 +- **前端**:`CourseDetailView.vue`、`coach/BookingManagerView.vue` 以 `e.response?.data?.message` 顯示錯誤;資料庫錯誤時會顯示 `Server Error`(原本顯示 SQL 訊息),不需修改前端 +- **監控**:預約交易內的資料庫錯誤開始出現在 Sentry +- **測試**:新增 Feature 測試;既有 `BookingOversellTest`、`BookingLifecycleTest` 的 422 行為不變 +- **相依、migration、路由、Swagger**:無變更(`app/Docs/` 目前沒有記錄這兩個端點的 500,維持不新增) diff --git a/openspec/changes/fix-booking-exception-handling/specs/booking-lifecycle/spec.md b/openspec/changes/fix-booking-exception-handling/specs/booking-lifecycle/spec.md new file mode 100644 index 0000000..d5ef09a --- /dev/null +++ b/openspec/changes/fix-booking-exception-handling/specs/booking-lifecycle/spec.md @@ -0,0 +1,24 @@ +## ADDED Requirements + +### Requirement: 預約交易錯誤分類 +建立預約(`POST /api/member/bookings`)與確認預約(`PUT /api/provider/bookings/{id}/confirm`)的交易中,系統 SHALL 區分業務規則失敗與伺服器端故障:業務規則失敗(名額不足、重複預約)回傳 422 與既定訊息;其他任何例外(含資料庫錯誤)MUST NOT 以 422 回傳,MUST 回傳 500,回應內容不得包含 SQL、資料表或欄位名稱,且該例外 MUST 交由例外回報機制記錄(正式環境會送至 Sentry)。兩種情況下交易都 MUST rollback,不留下部分寫入。 + +#### Scenario: 重複預約仍回 422 與既定訊息 +- **WHEN** 已登入 member 對同一時段已有 `pending` 預約,再次送出 `POST /api/member/bookings`(`schedule_id` 相同、`participants` 1) +- **THEN** 系統回傳 422,body 為 `{"status": false, "message": "您已預約此時段"}`,該 member 在此時段的預約筆數不變 + +#### Scenario: 交易內名額不足仍回 422 與既定訊息 +- **WHEN** 已登入 member 送出 `POST /api/member/bookings` 時通過交易前的名額檢查,但交易內鎖定時段後重新計算的剩餘名額小於 `participants` +- **THEN** 系統回傳 422,body 為 `{"status": false, "message": "名額不足,請重新選擇"}`,不建立預約 + +#### Scenario: 確認預約時名額不足仍回 422 與既定訊息 +- **WHEN** 已登入 provider 對自己課程的 `pending` 預約送出 `PUT /api/provider/bookings/{id}/confirm`,而該預約人數大於時段剩餘名額 +- **THEN** 系統回傳 422,body 為 `{"status": false, "message": "名額不足,無法確認此預約"}`,預約維持 `pending`,`current_participants` 不變 + +#### Scenario: 建立預約時資料庫錯誤回 500 且不洩漏細節 +- **WHEN** `APP_DEBUG` 為 false,已登入 member 送出合法的 `POST /api/member/bookings`,而交易內寫入預約之後發生資料庫例外 +- **THEN** 系統回傳 500,body 為通用錯誤訊息且不含 SQL 語句、`bookings` 等資料表名稱與欄位名稱,資料庫例外被回報,`bookings` 筆數不變 + +#### Scenario: 確認預約時資料庫錯誤回 500 且不洩漏細節 +- **WHEN** `APP_DEBUG` 為 false,已登入 provider 對自己課程的 `pending` 預約送出 `PUT /api/provider/bookings/{id}/confirm`(名額足夠),而交易內預約狀態已更新、更新時段名額時發生資料庫例外 +- **THEN** 系統回傳 500,body 為通用錯誤訊息且不含 SQL 語句、資料表名稱與欄位名稱,資料庫例外被回報,預約維持 `pending`,`current_participants` 不變 diff --git a/openspec/changes/fix-booking-exception-handling/tasks.md b/openspec/changes/fix-booking-exception-handling/tasks.md new file mode 100644 index 0000000..dbf841c --- /dev/null +++ b/openspec/changes/fix-booking-exception-handling/tasks.md @@ -0,0 +1,24 @@ +# Tasks + +測試集中在新檔 `tests/Feature/BookingExceptionHandlingTest.php`(建立 provider/時段的輔助方法參考 `tests/Feature/BookingOversellTest.php`)。每個測試的 docblock 要寫明「為什麼」:資料庫錯誤曾以 422 把 SQL 回給使用者,且不會送到 Sentry。觸發與斷言方式見 design.md D3。 + +## 1. 業務例外與 Controller + +- [ ] 1.1 [後端] 新增 `app/Exceptions/BookingRuleViolationException.php`(`final class`,繼承 `\RuntimeException`,不加額外方法);驗證:2.x 測試通過 +- [ ] 1.2 [後端] `app/Http/Controllers/API/MemberBookingController.php` 的 `store`:交易閉包內兩處 `throw new \RuntimeException(...)` 改為 `throw new BookingRuleViolationException(...)`,訊息文字不變;`catch (\RuntimeException $e)` 改為 `catch (BookingRuleViolationException $e)` +- [ ] 1.3 [後端] `app/Http/Controllers/API/ProviderBookingController.php` 的 `confirm`:同 1.2,處理 `名額不足,無法確認此預約` +- [ ] 1.4 [後端] 確認 `grep -rn "RuntimeException" app/` 只剩 `BookingRuleViolationException` 本身的繼承宣告 + +## 2. 測試 + +- [ ] 2.1 [整合測試] `test_duplicate_booking_returns_422_with_rule_message`:member 已有該時段 `pending` 預約,再次 `POST /api/member/bookings`,回 422,`assertExactJson(['status' => false, 'message' => '您已預約此時段'])`,該 member 在此時段的預約仍為 1 筆;測試開頭 `Exceptions::fake()`,最後 `Exceptions::assertNotReported(BookingRuleViolationException::class)`(對應 Scenario「重複預約仍回 422 與既定訊息」) +- [ ] 2.2 [整合測試] `test_capacity_lost_inside_transaction_returns_422_with_rule_message`:以 `CourseSchedule::retrieved` 事件在交易內把 `current_participants` 設為滿額(事件只能改動交易內 `lockForUpdate()->find()` 那次讀取;交易前的第一次讀取若被改動,會被前置檢查以「人數超過剩餘名額」攔下,測不到交易內路徑),`POST /api/member/bookings` 回 422,`assertExactJson(['status' => false, 'message' => '名額不足,請重新選擇'])`,`bookings` 筆數為 0;同 2.1 以 `Exceptions::fake()` 斷言未回報(對應 Scenario「交易內名額不足仍回 422 與既定訊息」) +- [ ] 2.3 [整合測試] `test_confirm_without_capacity_returns_422_with_rule_message`:provider 確認人數大於剩餘名額的 `pending` 預約,回 422,`assertExactJson(['status' => false, 'message' => '名額不足,無法確認此預約'])`,預約仍為 `pending`、`current_participants` 不變;同 2.1 以 `Exceptions::fake()` 斷言未回報(對應 Scenario「確認預約時名額不足仍回 422 與既定訊息」) +- [ ] 2.4 [整合測試] `test_database_error_on_create_returns_500_without_sql`:`config(['app.debug' => false])`、`Exceptions::fake()`,`Booking::created`(INSERT 已執行之後)拋出 SQL 含 `insert into "bookings"` 與欄位名稱的 `QueryException`;合法 `POST /api/member/bookings` 回 500,`assertExactJson(['message' => 'Server Error'])`,body 不含 `insert into`、`bookings`、`member_id`,`Exceptions::assertReported(QueryException::class)`,`bookings` 筆數為 0(INSERT 已發生,筆數為 0 才能證明 rollback)(對應 Scenario「建立預約時資料庫錯誤回 500 且不洩漏細節」) +- [ ] 2.5 [整合測試] `test_database_error_on_confirm_returns_500_without_sql`:同 2.4 的設定,`CourseSchedule::updating`(在交易內預約狀態已更新、`increment` 執行時觸發)拋出 SQL 含 `update "course_schedules"` 的 `QueryException`;provider 確認名額足夠的 `pending` 預約,回 500,`assertExactJson(['message' => 'Server Error'])`,body 不含 `update`、`course_schedules`、`current_participants`,`Exceptions::assertReported(QueryException::class)`,預約仍為 `pending`(狀態更新已執行,仍為 `pending` 才能證明 rollback)、`current_participants` 不變(對應 Scenario「確認預約時資料庫錯誤回 500 且不洩漏細節」) +- [ ] 2.6 [整合測試] 執行 `php artisan test --filter='BookingLifecycleTest|BookingOversellTest'`,確認既有 422 行為不變;完整 `php artisan test` 需在有 GD 的環境執行(見 `cfdive-test-gd` image 的做法) + +## 3. 手動驗證 + +- [ ] 3.1 [手動驗證] 變異驗證(只在本機執行,不提交):暫時把 1.2、1.3 的 catch 改回 `\RuntimeException`,確認 2.4、2.5 失敗後再還原,證明測試抓得到原本的問題 +- [ ] 3.2 [手動驗證] 部署後在正式環境建立一筆預約、確認一筆預約,確認業務流程正常(201/200);Sentry 是否收到預約端點的 500 只能等實際錯誤發生,若期間沒有錯誤則記錄為「未觀察到」,不視為失敗(自動化驗證以 2.4、2.5 為準)