diff --git a/packages/hflow-server/src/hflow_server/_curation.py b/packages/hflow-server/src/hflow_server/_curation.py index 5f049c5f..96d2bab5 100644 --- a/packages/hflow-server/src/hflow_server/_curation.py +++ b/packages/hflow-server/src/hflow_server/_curation.py @@ -173,10 +173,6 @@ def _reject_non_single_select(user_sql: str) -> None: ) -def _sidecar_refusal(error: _sidecar.SidecarError) -> HTTPException: - return HTTPException(status_code=error.status_code, detail=error.detail) - - def _browsable_relations( connection: duckdb.DuckDBPyConnection, ) -> dict[str, CatalogTableKind]: @@ -310,23 +306,17 @@ def create_curation_router(settings: ServerSettings) -> APIRouter: # mutating route -- sufficient for the single-server design. sidecar_write_lock = threading.Lock() + # A SidecarError is already an HTTPException, so these three only bind the + # data root -- nothing catches and re-raises, and a refusal raised inside + # them reaches the client with the status and detail _sidecar chose. def loaded_sidecar_state() -> _sidecar.SidecarState: - try: - return _sidecar.load_sidecar_state(settings.data_root) - except _sidecar.SidecarError as error: - raise _sidecar_refusal(error) from error + return _sidecar.load_sidecar_state(settings.data_root) def stored_sidecar_state(state: _sidecar.SidecarState) -> None: - try: - _sidecar.store_sidecar_state(settings.data_root, state) - except _sidecar.SidecarError as error: - raise _sidecar_refusal(error) from error + _sidecar.store_sidecar_state(settings.data_root, state) def local_data_root_or_refuse() -> Path: - try: - return _sidecar.local_data_root(settings.data_root) - except _sidecar.SidecarError as error: - raise _sidecar_refusal(error) from error + return _sidecar.local_data_root(settings.data_root) @router.post("/curation/preview") def run_curation_preview(request: PreviewRequest) -> CurationPreviewResponse: @@ -420,14 +410,10 @@ def download_manifest(manifest_id: str) -> FileResponse: status_code=404, detail=f"no pinned manifest with id {manifest_id!r}" ) manifest_file = local_data_root_or_refuse() / entry.manifest_path - try: - # The same strict-resolve + containment check media serving uses: - # even a hand-edited registry path can only serve workspace files. - resolved_file = _media.resolve_served_file( - str(manifest_file), data_root=settings.data_root - ) - except _media.MediaResolutionError as error: - raise _media.media_refusal(error) from error + # The same strict-resolve + containment check media serving uses: even + # a hand-edited registry path can only serve workspace files. Its + # refusal is already an HTTPException, so it needs no rewrapping here. + resolved_file = _media.resolve_served_file(str(manifest_file), data_root=settings.data_root) return _media.served_file_response( resolved_file, attachment_filename=Path(entry.manifest_path).name ) diff --git a/packages/hflow-server/src/hflow_server/_media.py b/packages/hflow-server/src/hflow_server/_media.py index 39699389..4756e32d 100644 --- a/packages/hflow-server/src/hflow_server/_media.py +++ b/packages/hflow-server/src/hflow_server/_media.py @@ -68,24 +68,15 @@ } -class MediaResolutionError(Exception): - """One refusal to serve a catalog URI, carrying its HTTP mapping.""" - - def __init__(self, status_code: int, detail: str) -> None: - super().__init__(detail) - self.status_code = status_code - self.detail = detail - - -def media_refusal(error: MediaResolutionError) -> HTTPException: - """The HTTP refusal one unservable URI maps to. - - Lives beside the error it converts (as ``_connections`` and ``_runtime`` - do for theirs), so the two routes that serve catalog bytes -- episode - media and manifest downloads -- share one mapping instead of each copying - the two field reads. +class MediaResolutionError(HTTPException): + """One refusal to serve a catalog URI. + + It IS the HTTP refusal rather than something a caller converts into one: + raised from anywhere under a route, FastAPI renders it with the status and + detail given here. It stays its own type so + :func:`is_uri_servable` can catch media refusals specifically, without also + swallowing an unrelated refusal raised nearby. """ - return HTTPException(status_code=error.status_code, detail=error.detail) def _resolved_local_data_root(data_root: str) -> Path: diff --git a/packages/hflow-server/src/hflow_server/_sidecar.py b/packages/hflow-server/src/hflow_server/_sidecar.py index 19c23e2c..814a06d3 100644 --- a/packages/hflow-server/src/hflow_server/_sidecar.py +++ b/packages/hflow-server/src/hflow_server/_sidecar.py @@ -35,6 +35,8 @@ from dataclasses import dataclass from pathlib import Path +from fastapi import HTTPException + from hflow_server._contract import CheckCoverageEntry, PinnedManifestEntry, SavedQueryEntry from hflow_server._settings import local_data_root_or_none @@ -43,13 +45,13 @@ SIDECAR_FILE_NAME = "state.json" -class SidecarError(Exception): - """One refusal to read or write the sidecar, carrying its HTTP mapping.""" +class SidecarError(HTTPException): + """One refusal to read or write the sidecar. - def __init__(self, status_code: int, detail: str) -> None: - super().__init__(detail) - self.status_code = status_code - self.detail = detail + It IS the HTTP refusal rather than something the router converts into one: + raised from anywhere under a route, FastAPI renders it with the status and + detail given here. + """ @dataclass(frozen=True) diff --git a/packages/hflow-server/src/hflow_server/server.py b/packages/hflow-server/src/hflow_server/server.py index 430e735f..426f1019 100644 --- a/packages/hflow-server/src/hflow_server/server.py +++ b/packages/hflow-server/src/hflow_server/server.py @@ -374,11 +374,10 @@ def _catalog_marker_readable(workspace: Workspace) -> bool: def _served_file_response_or_refuse(uri: str, data_root: str) -> FileResponse: - try: - resolved_file = _media.resolve_served_file(uri, data_root=data_root) - except _media.MediaResolutionError as error: - raise _media.media_refusal(error) from error - return _media.served_file_response(resolved_file) + """Bytes for one catalog URI. An unservable URI raises its own refusal + (a ``MediaResolutionError`` IS an ``HTTPException``), so there is nothing + to catch and convert here.""" + return _media.served_file_response(_media.resolve_served_file(uri, data_root=data_root)) def _assets_directory(settings: ServerSettings) -> Path | None: