Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions diracx-core/src/diracx/core/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
__all__ = [
"AuthorizationError",
"DiracError",
"DiracHttpResponseError",
"IAMClientError",
"IAMServerError",
"InvalidCredentialsError",
Expand All @@ -16,19 +15,8 @@
"TokenNotFoundError",
]

from http import HTTPStatus


class DiracHttpResponseError(RuntimeError):
def __init__(self, status_code: int, data):
self.status_code = status_code
self.data = data


class DiracError(RuntimeError):
http_status_code = HTTPStatus.BAD_REQUEST # 400
http_headers: dict[str, str] | None = None

def __init__(self, detail: str = "Unknown"):
self.detail = detail

Expand Down
12 changes: 6 additions & 6 deletions diracx-routers/src/diracx/routers/auth/device_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ async def initiate_device_flow(
except ValueError as e:
raise HTTPException(
status_code=HTTPStatus.BAD_REQUEST,
detail=e.args[0],
detail=str(e),
) from e

return device_flow_response
Expand Down Expand Up @@ -116,19 +116,19 @@ async def do_device_flow(
logger.warning("Invalid or expired user_code: %s", e)
raise HTTPException(
status_code=HTTPStatus.BAD_REQUEST,
detail=e.args[0],
detail=str(e),
) from e
except ValueError as e:
logger.warning("Invalid scope during device flow: %s", e)
raise HTTPException(
status_code=HTTPStatus.BAD_REQUEST,
detail=e.args[0],
detail=str(e),
) from e
except IAMServerError as e:
logger.warning("IAM server error during device flow: %s", e)
raise HTTPException(
status_code=HTTPStatus.BAD_GATEWAY,
detail=e.args[0],
detail=str(e),
) from e
return RedirectResponse(authorization_flow_url)

Expand Down Expand Up @@ -163,13 +163,13 @@ async def finish_device_flow(
logger.warning("IAM server error during device flow completion: %s", e)
raise HTTPException(
status_code=HTTPStatus.BAD_GATEWAY,
detail=e.args[0],
detail=str(e),
) from e
except IAMClientError as e:
logger.warning("IAM client error during device flow completion: %s", e)
raise HTTPException(
status_code=HTTPStatus.UNAUTHORIZED,
detail=e.args[0],
detail=str(e),
) from e

return RedirectResponse(f"{request_url}/finished")
Expand Down
10 changes: 5 additions & 5 deletions diracx-routers/src/diracx/routers/auth/token.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
from typing import Annotated, Literal

from fastapi import Depends, Form, Header, HTTPException
from fastapi.responses import JSONResponse
from joserfc.errors import JoseError

from diracx.core.exceptions import (
DiracHttpResponseError,
InvalidCredentialsError,
PendingAuthorizationError,
)
Expand Down Expand Up @@ -138,11 +138,11 @@ async def get_oidc_token(
code_verifier=code_verifier,
refresh_token=refresh_token,
)
except PendingAuthorizationError as e:
raise DiracHttpResponseError(
except PendingAuthorizationError:
return JSONResponse(
status_code=HTTPStatus.BAD_REQUEST,
data={"error": "authorization_pending"},
) from e
content={"error": "authorization_pending"},
)
except ValueError as e:
raise HTTPException(
status_code=HTTPStatus.BAD_REQUEST,
Expand Down
17 changes: 6 additions & 11 deletions diracx-routers/src/diracx/routers/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
from uvicorn.logging import AccessFormatter, DefaultFormatter

from diracx.core.config import ConfigSource
from diracx.core.exceptions import DiracError, DiracHttpResponseError, NotReadyError
from diracx.core.exceptions import DiracError, NotReadyError
from diracx.core.extensions import DiracEntryPoint, select_from_extension
from diracx.core.settings import FactorySettings, ServiceSettingsBase
from diracx.core.sources import AsyncCacheableSource
Expand Down Expand Up @@ -327,9 +327,6 @@ def create_app_inner(
# with a subclass of Exception (https://mypy.readthedocs.io/en/latest/generics.html#variance-of-generic-types)
handler_signature = Callable[[Request, Exception], Response | Awaitable[Response]]
app.add_exception_handler(DiracError, cast(handler_signature, dirac_error_handler))
app.add_exception_handler(
DiracHttpResponseError, cast(handler_signature, http_response_handler)
)
app.add_exception_handler(
DBUnavailableError, cast(handler_signature, route_unavailable_error_hander)
)
Expand Down Expand Up @@ -427,17 +424,15 @@ def create_app() -> DiracFastAPI:


def dirac_error_handler(request: Request, exc: DiracError) -> Response:
status_code = getattr(exc, "http_status_code", HTTPStatus.BAD_REQUEST)
headers = getattr(exc, "http_headers", None)
return JSONResponse(
status_code=exc.http_status_code,
status_code=status_code,
content={"detail": exc.detail},
headers=exc.http_headers,
headers=headers,
)


def http_response_handler(request: Request, exc: DiracHttpResponseError) -> Response:
return JSONResponse(status_code=exc.status_code, content=exc.data)


def route_unavailable_error_hander(request: Request, exc: DBUnavailableError):
logger.warning(
"503 Service Unavailable: %s (path=%s)",
Expand Down Expand Up @@ -505,7 +500,7 @@ async def is_db_unavailable(db: BaseSQLDB | BaseOSDB) -> str:
_db_alive_cache[db] = ""

except DBUnavailableError as e:
_db_alive_cache[db] = e.args[0]
_db_alive_cache[db] = str(e)

return _db_alive_cache[db]

Expand Down
7 changes: 7 additions & 0 deletions docs/dev/reference/coding-conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,10 @@ To make sure that each part of DiracX is doing only what it is supposed to do, y
- `diracx-routers` should deal with user interactions through HTTPs. It is expected to deal with permissions and should call `diracx-logic`. Results returned should be translated into HTTP responses.
- `diracx-logic` should embed Dirac specificities. It should encapsulate the logic of the services and should call `diracx-db` to interact with databases.
- `diracx-db` should contain atomic methods (complex logic is expected to be located in `diracx-db`).

### Error handling boundaries

- `diracx-db` and `diracx-logic` should raise domain exceptions (typically `DiracError` subclasses), and should not depend on `fastapi`.
- `diracx-routers` is the HTTP boundary and should translate known backend/domain exceptions into `HTTPException` with the desired status code, message, and headers.
- `DiracError` handling registered at app level (see diracx-routers/src/diracx/routers/factory.py, especially `app.add_exception_handler(DiracError, cast(handler_signature, dirac_error_handler))`) is a fallback safety net for uncaught domain exceptions, not a replacement for explicit mapping when the router needs specific HTTP behavior.
- Protocol-specific responses can be handled directly in routers when `HTTPException` shape is not suitable (for example OAuth2-style payloads in tokens.py), e.g. by returning a `JSONResponse` directly.
Loading