Consider custom exceptions in OpenAPI documentation

[MoritzWeber0]

Custom exception handlers should also appear as possible responses in the auto-generated OpenAPI specification.

[romeonicholas]

Rough idea, but what do you think of this @MoritzWeber0?

• basic error pydantic models
• a function to create error responses
• generating responses just above each route so they can then be used in the responses list (this is what makes them appear in OpenAPI)

The things I'm still trying to figure out:

• This lets every error have its own example in the documentation, though then we're documenting in the route instead of in the pydantic model
• Do we have any routes that have multiple ways to reach the same error status code? This might not allow us to document multiple reasons for a specific error code without resorting to something silly like a long string with a separator, but I haven't tested it/checked
• We could pass both technical and user descriptions this way, but not sure if that's necessary or helpful

I just tested this out on the user routes for users/id:

class ErrorDetail(pydantic.BaseModel):
    err_code: str
    reason: str

class ErrorResponse(pydantic.BaseModel):
    status_code: int
    detail: ErrorDetail

def create_error_response_and_exception(
    status_code: int, err_code: str, reason: str
) -> dict:
    return {
        "response": {
            "model": ErrorResponse,
            "description": "Maybe a technical description as opposed to the user one?",
            "content": {
                "application/json": {
                    "example": ErrorResponse(
                        status_code=status_code,
                        detail=ErrorDetail(
                            err_code=err_code,
                            reason=reason,
                        ),
                    ).model_dump()
                }
            },
        },
        "exception": {
            "status_code": status_code,
            "detail": {
                "err_code": err_code,
                "reason": reason,
            },
        },
    }

NO_PROJECTS_IN_COMMON = create_error_response_and_exception(
    status_code=status.HTTP_403_FORBIDDEN,
    err_code="NO_PROJECTS_IN_COMMON",
    reason="You need at least one project in common to access the user profile of another user.",
)

@router.get(
    "/{user_id}",
    response_model=models.User,
    responses={status.HTTP_403_FORBIDDEN: NO_PROJECTS_IN_COMMON["response"]},
)
def get_user(
    own_user: models.DatabaseUser = fastapi.Depends(injectables.get_own_user),
    user: models.DatabaseUser = fastapi.Depends(injectables.get_existing_user),
    db: orm.Session = fastapi.Depends(database.get_db),
) -> models.DatabaseUser:
    if (
        user.id == own_user.id
        or len(projects_crud.get_common_projects_for_users(db, own_user, user))
        > 0
        or auth_injectables.RoleVerification(
            required_role=models.Role.ADMIN, verify=False
        )(own_user.name, db)
    ):
        return user
    else:
        raise fastapi.HTTPException(**NO_PROJECTS_IN_COMMON["exception"])

[romeonicholas]

Ugh, just realized raising the exception that way would break the existing error interceptor. So it seems like it would require even more duplication, because you'd need to repeat everything to generate the responses and to raise the exception properly? Seems like there must be a way to avoid that.

Edited: changed code to fix the skipped error interceptor and to move the duplication into the exception building function instead of the route, maybe that's better? Still kind of weird.

[MoritzWeber0]

Rough idea, but what do you think of this @MoritzWeber0?

• basic error pydantic models

We don't use pydantic models as of now and I want to avoid that we have to define exceptions twice. Have look at the exception files, e.g. capellacollab/users/exceptions.py. We already define the exceptions here, it would be great if we could use them.

• generating responses just above each route so they can then be used in the responses list (this is what makes them appear in OpenAPI)

It would be nice if we could easily register all exceptions a API route can throw, e.g. via an annotation. EDIT: Passing via the responses field like you've done it should be good. Better would be if it's completely auto-determined by scanning the AST of the function to detect which exceptions a function can raise: https://stackoverflow.com/questions/32560116/how-to-list-all-exceptions-a-function-could-raise-in-python-3

I just tested this out on the user routes for users/id:

Looks good.

class ErrorDetail(pydantic.BaseModel):
    err_code: str
    reason: str

class ErrorResponse(pydantic.BaseModel):
    status_code: int
    detail: ErrorDetail

def create_error_response_and_exception(
    status_code: int, err_code: str, reason: str
) -> dict:
    return {
        "response": {
            "model": ErrorResponse,
            "description": "Maybe a technical description as opposed to the user one?",
            "content": {
                "application/json": {
                    "example": ErrorResponse(
                        status_code=status_code,
                        detail=ErrorDetail(
                            err_code=err_code,
                            reason=reason,
                        ),
                    ).model_dump()
                }
            },
        },
        "exception": {
            "status_code": status_code,
            "detail": {
                "err_code": err_code,
                "reason": reason,
            },
        },
    }

NO_PROJECTS_IN_COMMON = create_error_response_and_exception(
    status_code=status.HTTP_403_FORBIDDEN,
    err_code="NO_PROJECTS_IN_COMMON",
    reason="You need at least one project in common to access the user profile of another user.",
)

@router.get(
    "/{user_id}",
    response_model=models.User,
    responses={status.HTTP_403_FORBIDDEN: NO_PROJECTS_IN_COMMON["response"]},
)
def get_user(
    own_user: models.DatabaseUser = fastapi.Depends(injectables.get_own_user),
    user: models.DatabaseUser = fastapi.Depends(injectables.get_existing_user),
    db: orm.Session = fastapi.Depends(database.get_db),
) -> models.DatabaseUser:
    if (
        user.id == own_user.id
        or len(projects_crud.get_common_projects_for_users(db, own_user, user))
        > 0
        or auth_injectables.RoleVerification(
            required_role=models.Role.ADMIN, verify=False
        )(own_user.name, db)
    ):
        return user
    else:
        raise fastapi.HTTPException(**NO_PROJECTS_IN_COMMON["exception"])

I'd keep the current way that we define our custom exceptions and raise them. There are still some places where we raise fastapi.HTTPException, but those should be replaces with the new format. But what we could do is to restructure the exceptions a bit.

Let's look at the current structure (example capellacollab/users/exceptions.py:

@dataclasses.dataclass
class UserNotFoundError(Exception):
    username: str | None = None
    user_id: int | None = None

async def user_not_found_exception_handler(
    request: fastapi.Request, exc: UserNotFoundError
) -> fastapi.Response:
    return await exception_handlers.http_exception_handler(
        request,
        fastapi.HTTPException(
            status_code=status.HTTP_404_NOT_FOUND,
            detail={
                "title": "User not found",
                "reason": f"The user '{exc.username or exc.user_id}' doesn't exist.",
            },
        ),
    )

def register_exceptions(app: fastapi.FastAPI):
    app.add_exception_handler(
        UserNotFoundError, user_not_found_exception_handler  # type: ignore[arg-type]
    )

As you already can see, this is a lot of boilerplate. We could also generate the error message in the Exception directly:

@dataclasses.dataclass
class UserNotFoundError(Exception):
   def __init__(self, username: str | None = None, user_id: int | None = None):
      self.status_code=status.HTTP_404_NOT_FOUND
      self.title = "User not found"
      self.reason = f"The user '{username or user_id}' doesn't exist."

Then, we could remove the user_not_found_exception_handler function. In addition, we can have a central registration of exceptions, for example a set:

registered_exceptions = {UserNotFoundError}

The main function could pick it up and register all registered exceptions properly. In addition, we can add create_error_responses([UserNotFoundError]) to the route and generate the docs from the Exception directly. What do you think about it?