Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,4 @@ site
# DiracX specific
# No point in committing the pixi.lock for gubbins as it cannot work properly within the diracx repo
extensions/gubbins/pixi.lock
TIPS
Comment thread
mmascher marked this conversation as resolved.
Outdated
10 changes: 3 additions & 7 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 @@ -19,13 +18,10 @@
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):
# Fallback HTTP representation used by the app-level DiracError handler.
# Routers are expected to catch DiracError and raise HTTPException
# explicitly when endpoint-specific status codes/messages/headers are needed.
http_status_code = HTTPStatus.BAD_REQUEST # 400
http_headers: dict[str, str] | None = None
Comment thread
mmascher marked this conversation as resolved.
Outdated

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
2 changes: 1 addition & 1 deletion diracx-routers/src/diracx/routers/auth/token.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from joserfc.errors import JoseError

from diracx.core.exceptions import (
DiracHttpResponseError,
InvalidCredentialsError,
PendingAuthorizationError,
)
Expand All @@ -30,6 +29,7 @@
)
from diracx.routers.access_policies import BaseAccessPolicy
from diracx.routers.dependencies import Config
from diracx.routers.exceptions import DiracHttpResponseError

from ..dependencies import AvailableSecurityProperties
from ..fastapi_classes import DiracxRouter
Expand Down
9 changes: 9 additions & 0 deletions diracx-routers/src/diracx/routers/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from __future__ import annotations

__all__ = ["DiracHttpResponseError"]


class DiracHttpResponseError(RuntimeError):
Comment thread
mmascher marked this conversation as resolved.
Outdated
def __init__(self, status_code: int, data):
self.status_code = status_code
self.data = data
Comment thread
mmascher marked this conversation as resolved.
Outdated
5 changes: 3 additions & 2 deletions diracx-routers/src/diracx/routers/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,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 ServiceSettingsBase
from diracx.core.sources import AsyncCacheableSource
Expand All @@ -39,6 +39,7 @@
from diracx.db.sql.utils import BaseSQLDB
from diracx.routers.access_policies import BaseAccessPolicy, check_permissions

from .exceptions import DiracHttpResponseError
from .fastapi_classes import DiracFastAPI, DiracxRouter
from .otel import instrument_otel
from .utils.users import verify_dirac_access_token
Expand Down Expand Up @@ -514,7 +515,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](../../../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 that case `DiracHttpResponseError` shall be used.
Comment thread
mmascher marked this conversation as resolved.
Outdated
Loading