From f0e97963506a4cd41ca19059de874b629f15fffc Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Wed, 10 May 2023 09:38:07 -0700 Subject: [PATCH] Use the Safir ClientRequestError framework Rather than using separate exception handlers for the exceptions for unknown flocks and unknown monkeys, define those exceptions using the Safir ClientRequestError framework and use the generic exception handler from Safir. Rename those exceptions to end in Error instead of Exception, following our normal naming conventions. --- src/mobu/exceptions.py | 47 ++++++++++++++++++++++-------------- src/mobu/main.py | 47 +++++------------------------------- src/mobu/services/flock.py | 22 ++++++++++++++--- src/mobu/services/manager.py | 13 +++++++--- 4 files changed, 63 insertions(+), 66 deletions(-) diff --git a/src/mobu/exceptions.py b/src/mobu/exceptions.py index e4b1439e..8731238c 100644 --- a/src/mobu/exceptions.py +++ b/src/mobu/exceptions.py @@ -6,8 +6,11 @@ from datetime import datetime from typing import Optional, Self +from fastapi import status from httpx_ws import HTTPXWSException, WebSocketDisconnect from safir.datetime import format_datetime_for_logging +from safir.fastapi import ClientRequestError +from safir.models import ErrorLocation from safir.slack.blockkit import ( SlackBaseBlock, SlackBaseField, @@ -25,13 +28,13 @@ __all__ = [ "CachemachineError", "CodeExecutionError", - "FlockNotFoundException", + "FlockNotFoundError", "GafaelfawrWebError", "JupyterTimeoutError", "JupyterWebError", "MobuSlackException", "MobuSlackWebException", - "MonkeyNotFoundException", + "MonkeyNotFoundError", "TAPClientError", ] @@ -63,6 +66,30 @@ class GafaelfawrWebError(SlackWebException): """An API call to Gafaelfawr failed.""" +class FlockNotFoundError(ClientRequestError): + """The named flock was not found.""" + + error = "flock_not_found" + status_code = status.HTTP_404_NOT_FOUND + + def __init__(self, flock: str) -> None: + self.flock = flock + msg = f"Flock {flock} not found" + super().__init__(msg, ErrorLocation.path, ["flock"]) + + +class MonkeyNotFoundError(ClientRequestError): + """The named monkey was not found.""" + + error = "monkey_not_found" + status_code = status.HTTP_404_NOT_FOUND + + def __init__(self, monkey: str) -> None: + self.monkey = monkey + msg = f"Monkey {monkey} not found" + super().__init__(msg, ErrorLocation.path, ["monkey"]) + + class MobuSlackException(SlackException): """Represents an exception that can be reported to Slack. @@ -159,22 +186,6 @@ class MobuSlackWebException(SlackWebException, MobuSlackException): """ -class FlockNotFoundException(Exception): - """The named flock was not found.""" - - def __init__(self, flock: str) -> None: - self.flock = flock - super().__init__(f"Flock {flock} not found") - - -class MonkeyNotFoundException(Exception): - """The named monkey was not found.""" - - def __init__(self, monkey: str) -> None: - self.monkey = monkey - super().__init__(f"Monkey {monkey} not found") - - class NotebookRepositoryError(MobuSlackException): """The repository containing notebooks to run is not valid.""" diff --git a/src/mobu/main.py b/src/mobu/main.py index f40441cc..d76de3f9 100644 --- a/src/mobu/main.py +++ b/src/mobu/main.py @@ -11,22 +11,20 @@ from importlib.metadata import metadata, version import structlog -from fastapi import FastAPI, Request, status -from fastapi.responses import JSONResponse +from fastapi import FastAPI +from safir.fastapi import ClientRequestError, client_request_error_handler from safir.logging import Profile, configure_logging, configure_uvicorn_logging from safir.middleware.x_forwarded import XForwardedMiddleware -from safir.models import ErrorLocation from safir.slack.webhook import SlackRouteErrorHandler from .asyncio import schedule_periodic from .config import config from .dependencies.context import context_dependency -from .exceptions import FlockNotFoundException, MonkeyNotFoundException from .handlers.external import external_router from .handlers.internal import internal_router from .status import post_status -__all__ = ["app", "config"] +__all__ = ["app"] configure_logging( @@ -58,6 +56,9 @@ SlackRouteErrorHandler.initialize(config.alert_hook, "mobu", logger) logger.debug("Initialized Slack webhook") +# Enable the generic exception handler for client errors. +app.exception_handler(ClientRequestError)(client_request_error_handler) + @app.on_event("startup") async def startup_event() -> None: @@ -78,39 +79,3 @@ async def shutdown_event() -> None: await app.state.periodic_status except asyncio.CancelledError: pass - - -@app.exception_handler(FlockNotFoundException) -async def flock_not_found_exception_handler( - request: Request, exc: FlockNotFoundException -) -> JSONResponse: - return JSONResponse( - status_code=status.HTTP_404_NOT_FOUND, - content={ - "detail": [ - { - "loc": [ErrorLocation.path, "flock"], - "msg": f"Flock for {exc.flock} not found", - "type": "flock_not_found", - } - ] - }, - ) - - -@app.exception_handler(MonkeyNotFoundException) -async def monkey_not_found_exception_handler( - request: Request, exc: MonkeyNotFoundException -) -> JSONResponse: - return JSONResponse( - status_code=status.HTTP_404_NOT_FOUND, - content={ - "detail": [ - { - "loc": [ErrorLocation.path, "monkey"], - "msg": f"Monkey for {exc.monkey} not found", - "type": "monkey_not_found", - } - ] - }, - ) diff --git a/src/mobu/services/flock.py b/src/mobu/services/flock.py index 2e85ebc4..7f04f233 100644 --- a/src/mobu/services/flock.py +++ b/src/mobu/services/flock.py @@ -12,7 +12,7 @@ from safir.datetime import current_datetime from structlog.stdlib import BoundLogger -from ..exceptions import MonkeyNotFoundException +from ..exceptions import MonkeyNotFoundError from ..models.flock import FlockConfig, FlockData, FlockSummary from ..models.user import AuthenticatedUser, User, UserSpec from .monkey import Monkey @@ -60,10 +60,26 @@ def dump(self) -> FlockData: ) def get_monkey(self, name: str) -> Monkey: - """Retrieve a given monkey by name.""" + """Retrieve a given monkey by name. + + Parameters + ---------- + name + Name of monkey to return. + + Returns + ------- + Monkey + Requested monkey. + + Raises + ------ + MonkeyNotFoundError + Raised if no monkey was found with that name. + """ monkey = self._monkeys.get(name) if not monkey: - raise MonkeyNotFoundException(name) + raise MonkeyNotFoundError(name) return monkey def list_monkeys(self) -> list[str]: diff --git a/src/mobu/services/manager.py b/src/mobu/services/manager.py index 2ee871c2..7d722ef4 100644 --- a/src/mobu/services/manager.py +++ b/src/mobu/services/manager.py @@ -10,7 +10,7 @@ from structlog.stdlib import BoundLogger from ..config import config -from ..exceptions import FlockNotFoundException +from ..exceptions import FlockNotFoundError from ..models.flock import FlockConfig, FlockSummary from .flock import Flock @@ -98,12 +98,12 @@ def get_flock(self, name: str) -> Flock: Raises ------ - FlockNotFoundException + FlockNotFoundError Raised if no flock was found with that name. """ flock = self._flocks.get(name) if flock is None: - raise FlockNotFoundException(name) + raise FlockNotFoundError(name) return flock def list_flocks(self) -> list[str]: @@ -133,9 +133,14 @@ async def stop_flock(self, name: str) -> None: ---------- name Name of flock to stop. + + Raises + ------ + FlockNotFoundError + Raised if no flock was found with that name. """ flock = self._flocks.get(name) if flock is None: - raise FlockNotFoundException(name) + raise FlockNotFoundError(name) del self._flocks[name] await flock.stop()