diff --git a/docs/dev/case-v1p1-conformance-backlog.md b/docs/dev/case-v1p1-conformance-backlog.md index a806f4e..91b27d0 100644 --- a/docs/dev/case-v1p1-conformance-backlog.md +++ b/docs/dev/case-v1p1-conformance-backlog.md @@ -16,6 +16,7 @@ compeito の現在のゴールは **OpenCASE / OpenSALT との実用的な相互 - Service Discovery `GET /ims/case/v1p1/discovery/imscasev1p1_openapi3_v1p0.json`（実装済・テストあり） - エラー封筒 `imsx_StatusInfo`（codeMajor / severity / codeMinor.codeMinorField[].{Name,Value}）は適合 - `GET /CFItemAssociations/{id}` の既定を全件返却に（公式契約にページネーション定義なし。既定 limit=100 のサイレント切り詰めを廃止、`limit`/`offset` は明示指定時のみの拡張に。2026-06 適合性監査 N1、PR #220） +- **未定義サブパスの 404 / 未捕捉の 500 を imsx_StatusInfo 形式で返す**（旧 C14 / C15）。`main.py` に `StarletteHTTPException` ハンドラ（CASE API パスの 404 → `unknownobject`）とグローバル `Exception` ハンドラ（CASE API パスの 500 → `internal_server_error`）を追加。CASE API 以外は既定挙動を維持。 ## certification 着手項目（未対応 / 意図的差異） @@ -34,8 +35,8 @@ compeito の現在のゴールは **OpenCASE / OpenSALT との実用的な相互 | C11 | **エラー封筒の `imsx_codeMinorFieldName`** | 常に既定の `"sourcedId"`。invalid_sort_field / invalid_selection_field 系では `sort` / `fields` / `limit` 等の実フィールド名が意味的に正しい | P3 | `imsx_error_response` に fieldName 引数を追加し、各呼び出し箇所で該当フィールド名を渡す | | C12 | **`ext:` associationType の文字種** | import 受理が `startswith("ext:")` のみで、公式パターン `(ext:)[a-zA-Z0-9.\-_]+` の文字種を検証しない（`ext:日本語` 等も通る） | P3 | 正規表現で検証し、不一致は invalid associationType として skip + warning | | C13 | **スキーマ層の出力時検証なし** | Pydantic スキーマで identifier の UUID パターン・associationType / targetType の enum を検証していない（import 側で防いでいるため実害は低い） | P3 | strict 出力モード導入時に field_validator で同梱 | -| C14 | **未定義サブパスの 404 が imsx 形式でない** | `/{tenant}/ims/case/v1p1/...` 配下の未定義サブパスは FastAPI/Starlette 既定の 404（`{"detail":"Not Found"}`）を返し、imsx_StatusInfo 形式になっていない（既知リソース種別で ID 不在の 404 `unknownobject` は実装済み） | P2 | CASE API パス配下の catch-all ルートまたは `StarletteHTTPException` ハンドラを `main.py` に追加し、imsx 404 に変換 | -| C15 | **500 が imsx 形式でない** | 未捕捉例外は Starlette 既定のプレーン 500 を返し、`internal_server_error` の imsx_StatusInfo 形式になっていない | P2 | グローバル `Exception` ハンドラを `main.py` に追加し、CASE API パスの 500 を imsx 形式に変換 | + +> C14（未定義サブパスの 404 imsx 化）と C15（500 imsx 化）は対応済み。上記「すでに対応済み」を参照。 ## デプロイ上の制約（参考） diff --git a/docs/spec/api-examples.md b/docs/spec/api-examples.md index a50b93a..46dd25f 100644 --- a/docs/spec/api-examples.md +++ b/docs/spec/api-examples.md @@ -747,9 +747,8 @@ The response includes an `Allow: GET` header. ### 500 Internal Server Error -> ⚠️ **Target shape, not yet implemented.** Uncaught errors currently return -> Starlette's default plain 500, not the imsx shape below. Tracked as -> [conformance backlog](../dev/case-v1p1-conformance-backlog.md) C15. +Uncaught errors on the CASE API return this imsx shape (a global exception +handler). Off the CASE API the default plain 500 is kept. ```json { diff --git a/docs/spec/api-spec.md b/docs/spec/api-spec.md index c126e71..610aa0f 100644 --- a/docs/spec/api-spec.md +++ b/docs/spec/api-spec.md @@ -254,10 +254,9 @@ The CASE v1.1 `imsx_StatusInfo` shape. Fields are at the root (no wrapper): - HTTP status mapping: 400 → `failure/error`; 404 → `failure/error` + `unknownobject`; 405 → `failure/error` + `invalid_selection_field`; 429 → `failure/error` + `server_busy`; 500 → `failure/error` + `internal_server_error`. - **429 (Server Busy):** defined for every endpoint in the CASE v1.1 OpenAPI. We do not implement rate limiting in Phase 1 explicitly, but API Gateway / Lambda throttling may yield 429. In that case we return the `server_busy` imsx_StatusInfo shape. - We do not use FastAPI's default 422 Validation Error; a custom exception handler converts `RequestValidationError` into a **400** `invalid_selection_field` imsx_StatusInfo response. -- ⚠️ **Not yet implemented** (see [conformance backlog](../dev/case-v1p1-conformance-backlog.md) C14 / C15) — two cases in the status mapping above are not yet wired: - - **404 for undefined sub-paths:** requests to undefined sub-paths under `/{tenant}/ims/case/v1p1/...` currently fall through to FastAPI/Starlette's default 404 (`{"detail": "Not Found"}`), **not** the imsx_StatusInfo shape. A catch-all route or a `StarletteHTTPException` handler for the CASE API path is needed to translate it (C14). - - **500 (internal_server_error):** uncaught server errors currently return Starlette's default plain 500, **not** the `internal_server_error` imsx shape. A global exception handler is needed (C15). - - Error shapes that **are** implemented: 400 (`invalid_uuid` / `invalid_selection_field`), 404 `unknownobject` for a known resource type whose ID does not exist, and 405. +- **404 for undefined sub-paths:** requests to undefined sub-paths under `/{tenant}/ims/case/v1p1/...` return a **404** `unknownobject` in the imsx_StatusInfo shape (a `StarletteHTTPException` handler translates Starlette's default 404 for CASE API paths). Off the CASE API the default 404 (`{"detail": "Not Found"}`) is kept. +- **500 (internal_server_error):** uncaught errors on the CASE API return a **500** `internal_server_error` in the imsx_StatusInfo shape (a global `Exception` handler). Off the CASE API the default plain 500 is kept. In both cases the original exception is still re-raised by Starlette's `ServerErrorMiddleware` for logging. +- All other documented error shapes are implemented: 400 (`invalid_uuid` / `invalid_selection_field`), 404 `unknownobject` for a known resource type whose ID does not exist, and 405. ## Unsupported HTTP methods @@ -595,10 +594,9 @@ CASE v1.1 の imsx_StatusInfo 形式。ルートレベルに直接フィール - HTTPステータスコード対応: 400→`failure/error`, 404→`failure/error`+`unknownobject`, 405→`failure/error`+`invalid_selection_field`, 429→`failure/error`+`server_busy`, 500→`failure/error`+`internal_server_error` - **429 (Server Busy):** CASE v1.1 OpenAPI で全エンドポイントに定義されている。Phase 1 では明示的なレート制限を実装しないが、API Gateway / Lambda のスロットリングにより 429 が返される可能性がある。その場合は imsx_StatusInfo 形式で `server_busy` を返す - FastAPI のデフォルト 422 Validation Error レスポンスは使用せず、imsx_StatusInfo 形式の **400**（`invalid_selection_field`）に変換する。カスタム例外ハンドラで RequestValidationError をキャッチし、imsx_StatusInfo 形式で返す -- ⚠️ **未実装**（[conformance backlog](../dev/case-v1p1-conformance-backlog.md) C14 / C15 参照）— 上記マッピングのうち以下2件は未配線: - - **未定義サブパスの 404:** `/{tenant}/ims/case/v1p1/...` 配下の未定義サブパスは現状 FastAPI/Starlette のデフォルト 404（`{"detail": "Not Found"}`）にフォールスルーし、imsx_StatusInfo 形式**ではない**。CASE API パス配下の catch-all ルートまたは `StarletteHTTPException` ハンドラで変換する必要がある（C14）。 - - **500（internal_server_error）:** 未捕捉のサーバーエラーは現状 Starlette のデフォルトのプレーン 500 を返し、`internal_server_error` の imsx 形式**ではない**。グローバル例外ハンドラが必要（C15）。 - - **実装済み**のエラー形式: 400（`invalid_uuid` / `invalid_selection_field`）、既知リソース種別で ID 不在時の 404 `unknownobject`、405。 +- **未定義サブパスの 404:** `/{tenant}/ims/case/v1p1/...` 配下の未定義サブパスには **404** `unknownobject` を imsx_StatusInfo 形式で返す（CASE API パスの 404 を `StarletteHTTPException` ハンドラで変換）。CASE API 以外では既定の 404（`{"detail": "Not Found"}`）を維持。 +- **500（internal_server_error）:** CASE API での未捕捉エラーには **500** `internal_server_error` を imsx_StatusInfo 形式で返す（グローバル `Exception` ハンドラ）。CASE API 以外では既定のプレーン 500 を維持。いずれの場合も Starlette の `ServerErrorMiddleware` が元例外を再 raise するためログには残る。 +- 上記以外の文書化済みエラー形式はすべて実装済み: 400（`invalid_uuid` / `invalid_selection_field`）、既知リソース種別で ID 不在時の 404 `unknownobject`、405。 ## 非対応HTTPメソッド diff --git a/src/main.py b/src/main.py index fcae6d6..19dca00 100644 --- a/src/main.py +++ b/src/main.py @@ -1,14 +1,19 @@ from pathlib import Path from fastapi import FastAPI, Request +from fastapi.exception_handlers import http_exception_handler as default_http_exception_handler from fastapi.exceptions import RequestValidationError -from fastapi.responses import JSONResponse, RedirectResponse +from fastapi.responses import JSONResponse, PlainTextResponse, RedirectResponse from fastapi.staticfiles import StaticFiles +from starlette.exceptions import HTTPException as StarletteHTTPException from src.errors import InvalidUUIDError, ResourceNotFoundError, imsx_error_response from src.routers.case_api import router as case_api_router from src.routers.web import router as web_router +# Marker identifying CASE API requests (kept in sync with the middleware below). +_CASE_API_MARKER = "/ims/case/v1p1/" + app = FastAPI( title="COMPEITO", description="1EdTech CASE v1.1 compatible web service", @@ -46,7 +51,7 @@ async def redirect_v1p0(request: Request, call_next): @app.middleware("http") async def method_not_allowed(request: Request, call_next): - if "/ims/case/v1p1/" in request.url.path and request.method != "GET": + if _CASE_API_MARKER in request.url.path and request.method != "GET": response = imsx_error_response(405, "Method not allowed", "invalid_selection_field") response.headers["Allow"] = "GET" return response @@ -73,6 +78,26 @@ async def resource_not_found_handler(request: Request, exc: ResourceNotFoundErro return imsx_error_response(404, exc.message, "unknownobject") +@app.exception_handler(StarletteHTTPException) +async def http_exception_handler(request: Request, exc: StarletteHTTPException): + # Undefined sub-paths under the CASE API get an imsx 404 (unknownobject); + # everything else keeps FastAPI's default handling (e.g. the Web UI). + if exc.status_code == 404 and _CASE_API_MARKER in request.url.path: + return imsx_error_response(404, exc.detail or "Not found", "unknownobject") + return await default_http_exception_handler(request, exc) + + +@app.exception_handler(Exception) +async def unhandled_exception_handler(request: Request, exc: Exception) -> JSONResponse | PlainTextResponse: + # Uncaught errors on the CASE API return an imsx 500; off the CASE API we + # mirror Starlette's default plain 500. In both cases Starlette's + # ServerErrorMiddleware re-raises the original exception afterwards, so the + # traceback is still logged by the server. + if _CASE_API_MARKER in request.url.path: + return imsx_error_response(500, "Internal server error", "internal_server_error") + return PlainTextResponse("Internal Server Error", status_code=500) + + # --------------------------------------------------------------------------- # Health endpoint # --------------------------------------------------------------------------- diff --git a/tests/integration/test_error_envelope.py b/tests/integration/test_error_envelope.py new file mode 100644 index 0000000..10d1f28 --- /dev/null +++ b/tests/integration/test_error_envelope.py @@ -0,0 +1,76 @@ +"""Error-envelope completeness for the CASE API (conformance backlog C14 / C15). + +- Undefined sub-paths under /{tenant}/ims/case/v1p1/ return an imsx 404. +- Uncaught errors on the CASE API return an imsx 500. +- Non-CASE paths keep FastAPI/Starlette's default error handling. +""" + +import uuid + +import pytest +from httpx import ASGITransport, AsyncClient +from sqlalchemy.ext.asyncio import AsyncSession + +from src.database import get_session +from src.main import app +from src.models.tenant import Tenant + +pytestmark = pytest.mark.asyncio + + +class TestCaseApi404Envelope: + async def test_undefined_case_subpath_returns_imsx_404(self, client: AsyncClient) -> None: + # No route matches "Bogus" → Starlette 404 → converted to imsx (C14). + resp = await client.get(f"/{uuid.uuid4()}/ims/case/v1p1/Bogus") + assert resp.status_code == 404 + body = resp.json() + assert body["imsx_codeMajor"] == "failure" + assert body["imsx_severity"] == "error" + assert "unknownobject" in str(body["imsx_codeMinor"]) + + async def test_non_case_404_keeps_default(self, client: AsyncClient) -> None: + # A missing static file 404s off the CASE API → default shape, not imsx. + resp = await client.get("/static/does-not-exist.xyz") + assert resp.status_code == 404 + assert "imsx_codeMajor" not in resp.json() + + async def test_non_case_405_keeps_default(self, client: AsyncClient) -> None: + # 405 off the CASE API is delegated to the default handler unchanged. + resp = await client.post("/health") + assert resp.status_code == 405 + assert "imsx_codeMajor" not in resp.json() + + +class TestCaseApi500Envelope: + async def test_500_on_case_api_returns_imsx( + self, + db_session: AsyncSession, + tenant: Tenant, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + from src.services import case_query_service + + def boom(*args, **kwargs): + raise RuntimeError("boom") + + # Force an uncaught error after tenant resolution succeeds. + monkeypatch.setattr(case_query_service, "count_cf_documents", boom) + + async def _override_get_session(): + yield db_session + + app.dependency_overrides[get_session] = _override_get_session + # raise_app_exceptions=False so the handled 500 response is observable + # (ServerErrorMiddleware re-raises the original for logging otherwise). + transport = ASGITransport(app=app, raise_app_exceptions=False) + try: + async with AsyncClient(transport=transport, base_url="http://test") as ac: + resp = await ac.get(f"/{tenant.id}/ims/case/v1p1/CFDocuments") + finally: + app.dependency_overrides.clear() + + assert resp.status_code == 500 + body = resp.json() + assert body["imsx_codeMajor"] == "failure" + assert body["imsx_severity"] == "error" + assert "internal_server_error" in str(body["imsx_codeMinor"])