From 4ca354a0c5a4e904d36fb29b8e41d32fa00dae2c Mon Sep 17 00:00:00 2001 From: Hayden <64056131+hay-kot@users.noreply.github.com> Date: Fri, 24 Feb 2023 19:12:48 -0900 Subject: [PATCH] refactor exception handler to use platform feature instead of home rolled handler --- mealie/app.py | 3 + mealie/core/exceptions.py | 17 ----- mealie/errors/__init__.py | 5 ++ mealie/errors/handlers.py | 70 +++++++++++++++++++ mealie/routes/_base/base_controllers.py | 7 -- mealie/routes/_base/mixins.py | 15 +--- .../routes/admin/admin_management_groups.py | 1 - mealie/routes/admin/admin_management_users.py | 2 +- mealie/routes/comments/__init__.py | 2 +- mealie/routes/groups/controller_cookbooks.py | 13 +--- .../groups/controller_group_notifications.py | 2 +- .../routes/groups/controller_group_reports.py | 12 +--- mealie/routes/groups/controller_labels.py | 2 +- mealie/routes/groups/controller_mealplan.py | 13 +--- .../groups/controller_shopping_lists.py | 2 +- mealie/routes/recipe/recipe_crud_routes.py | 53 ++------------ mealie/routes/recipe/timeline_events.py | 4 +- mealie/routes/unit_and_foods/foods.py | 6 +- mealie/routes/unit_and_foods/units.py | 6 +- 19 files changed, 96 insertions(+), 139 deletions(-) create mode 100644 mealie/errors/__init__.py create mode 100644 mealie/errors/handlers.py diff --git a/mealie/app.py b/mealie/app.py index f42abbd3d1a4..f3a20441e2db 100644 --- a/mealie/app.py +++ b/mealie/app.py @@ -3,6 +3,7 @@ from fastapi import FastAPI from fastapi.middleware.gzip import GZipMiddleware from fastapi.routing import APIRoute +from mealie import errors from mealie.core.config import get_app_settings from mealie.core.root_logger import get_logger from mealie.core.settings.static import APP_VERSION @@ -77,6 +78,8 @@ def api_routers(): app.include_router(media_router) app.include_router(utility_routes.router) + errors.mount_handlers(app, logger=get_logger()) + api_routers() diff --git a/mealie/core/exceptions.py b/mealie/core/exceptions.py index 39e179a8b569..c3a78b03215e 100644 --- a/mealie/core/exceptions.py +++ b/mealie/core/exceptions.py @@ -1,8 +1,3 @@ -from sqlite3 import IntegrityError - -from mealie.lang.providers import Translator - - class UnexpectedNone(Exception): """Exception raised when a value is None when it should not be.""" @@ -28,15 +23,3 @@ class NoEntryFound(Exception): """ pass - - -def mealie_registered_exceptions(t: Translator) -> dict: - """ - This function returns a dictionary of all the globally registered exceptions in the Mealie application. - """ - - return { - PermissionDenied: t.t("exceptions.permission-denied"), - NoEntryFound: t.t("exceptions.no-entry-found"), - IntegrityError: t.t("exceptions.integrity-error"), - } diff --git a/mealie/errors/__init__.py b/mealie/errors/__init__.py new file mode 100644 index 000000000000..f0ca41fed879 --- /dev/null +++ b/mealie/errors/__init__.py @@ -0,0 +1,5 @@ +from .handlers import mount_handlers + +__all__ = [ + "mount_handlers", +] diff --git a/mealie/errors/handlers.py b/mealie/errors/handlers.py new file mode 100644 index 000000000000..237c885ea114 --- /dev/null +++ b/mealie/errors/handlers.py @@ -0,0 +1,70 @@ +from logging import Logger + +import fastapi +import sqlalchemy +from fastapi.responses import JSONResponse + +from mealie.core import exceptions +from mealie.lang.providers import local_provider +from mealie.schema.response.responses import ErrorResponse + + +def json_error(content, status_code): + return JSONResponse(content={"detail": content}, status_code=status_code) + + +def mount_handlers(app: fastapi.FastAPI, logger: Logger) -> None: + """ + mount_handlers is a function that mounts the exception handlers to the FastAPI app. + It provides a common handling point for known exceptions and provides a consistent + response format. + """ + + @app.exception_handler(exceptions.PermissionDenied) + def _(req: fastapi.Request, exc: exceptions.PermissionDenied): + t = local_provider(req.headers.get("Accept-Language")) + + return json_error( + ErrorResponse.respond( + message=t.t("exceptions.permission-denied"), + exception=exc.__class__.__name__, + ), + 403, + ) + + @app.exception_handler(exceptions.NoEntryFound) + def _(req: fastapi.Request, exc: exceptions.NoEntryFound): + t = local_provider(req.headers.get("Accept-Language")) + + return json_error( + ErrorResponse.respond( + t.t("exceptions.no-entry-found"), + exception=exc.__class__.__name__, + ), + 404, + ) + + @app.exception_handler(sqlalchemy.exc.IntegrityError) + def _(req: fastapi.Request, exc: sqlalchemy.exc.IntegrityError): + t = local_provider(req.headers.get("Accept-Language")) + + return json_error( + ErrorResponse.respond( + message=t.t("exceptions.integrity-error"), + exception=exc.__class__.__name__, + ), + 400, + ) + + @app.exception_handler(Exception) + def _(req: fastapi.Request, exc: Exception): + logger.error("Unknown error") + logger.exception(exc) + + return json_error( + ErrorResponse.respond( + message="Unknown Error", + exception=exc.__class__.__name__, + ), + 500, + ) diff --git a/mealie/routes/_base/base_controllers.py b/mealie/routes/_base/base_controllers.py index 1c9a60f8021f..e7317e614342 100644 --- a/mealie/routes/_base/base_controllers.py +++ b/mealie/routes/_base/base_controllers.py @@ -7,7 +7,6 @@ from sqlalchemy.orm import Session from mealie.core.config import get_app_dirs, get_app_settings from mealie.core.dependencies.dependencies import get_admin_user, get_current_user, get_integration_id -from mealie.core.exceptions import mealie_registered_exceptions from mealie.core.root_logger import get_logger from mealie.core.settings.directories import AppDirectories from mealie.core.settings.settings import AppSettings @@ -86,12 +85,6 @@ class BaseUserController(_BaseController): # Manual Cache _checks: OperationChecks - def registered_exceptions(self, ex: type[Exception]) -> str: - registered = { - **mealie_registered_exceptions(self.translator), - } - return registered.get(ex, self.t("generic.server-error")) - @property def group_id(self) -> UUID4: return self.user.group_id diff --git a/mealie/routes/_base/mixins.py b/mealie/routes/_base/mixins.py index 715d5279fd25..718b086ffce0 100644 --- a/mealie/routes/_base/mixins.py +++ b/mealie/routes/_base/mixins.py @@ -34,33 +34,20 @@ class HttpRepo(Generic[C, R, U]): self, repo: RepositoryGeneric, logger: Logger, - exception_msgs: Callable[[type[Exception]], str] | None = None, default_message: str | None = None, ) -> None: self.repo = repo self.logger = logger - self.exception_msgs = exception_msgs if default_message: self.default_message = default_message - def get_exception_message(self, ext: Exception) -> str: - if self.exception_msgs: - return self.exception_msgs(type(ext)) - return self.default_message - def handle_exception(self, ex: Exception) -> None: # Cleanup self.logger.exception(ex) self.repo.session.rollback() - # Respond - msg = self.get_exception_message(ex) - - raise HTTPException( - status.HTTP_400_BAD_REQUEST, - detail=ErrorResponse.respond(message=msg, exception=str(ex)), - ) + raise ex def create_one(self, data: C) -> R | None: item: R | None = None diff --git a/mealie/routes/admin/admin_management_groups.py b/mealie/routes/admin/admin_management_groups.py index 7ca9d9ef7e00..2c640365722b 100644 --- a/mealie/routes/admin/admin_management_groups.py +++ b/mealie/routes/admin/admin_management_groups.py @@ -33,7 +33,6 @@ class AdminUserManagementRoutes(BaseAdminController): return HttpRepo[GroupBase, GroupInDB, GroupAdminUpdate]( self.repo, self.logger, - self.registered_exceptions, ) @router.get("", response_model=GroupPagination) diff --git a/mealie/routes/admin/admin_management_users.py b/mealie/routes/admin/admin_management_users.py index ff907c3b57a5..ef1a75deb228 100644 --- a/mealie/routes/admin/admin_management_users.py +++ b/mealie/routes/admin/admin_management_users.py @@ -26,7 +26,7 @@ class AdminUserManagementRoutes(BaseAdminController): @property def mixins(self): - return HttpRepo[UserIn, UserOut, UserOut](self.repo, self.logger, self.registered_exceptions) + return HttpRepo[UserIn, UserOut, UserOut](self.repo, self.logger) @router.get("", response_model=UserPagination) def get_all(self, q: PaginationQuery = Depends(PaginationQuery)): diff --git a/mealie/routes/comments/__init__.py b/mealie/routes/comments/__init__.py index a5bf7309b23d..3cf34d29ca77 100644 --- a/mealie/routes/comments/__init__.py +++ b/mealie/routes/comments/__init__.py @@ -30,7 +30,7 @@ class RecipeCommentRoutes(BaseUserController): @property def mixins(self) -> HttpRepo: - return HttpRepo(self.repo, self.logger, self.registered_exceptions, self.t("generic.server-error")) + return HttpRepo(self.repo, self.logger, self.t("generic.server-error")) def _check_comment_belongs_to_user(self, item_id: UUID4) -> None: comment = self.repo.get_one(item_id) diff --git a/mealie/routes/groups/controller_cookbooks.py b/mealie/routes/groups/controller_cookbooks.py index f9ce17cb471e..f13120987ba5 100644 --- a/mealie/routes/groups/controller_cookbooks.py +++ b/mealie/routes/groups/controller_cookbooks.py @@ -3,7 +3,6 @@ from functools import cached_property from fastapi import APIRouter, Depends, HTTPException from pydantic import UUID4 -from mealie.core.exceptions import mealie_registered_exceptions from mealie.routes._base import BaseCrudController, controller from mealie.routes._base.mixins import HttpRepo from mealie.routes._base.routers import MealieCrudRoute @@ -27,19 +26,9 @@ class GroupCookbookController(BaseCrudController): def repo(self): return self.repos.cookbooks.by_group(self.group_id) - def registered_exceptions(self, ex: type[Exception]) -> str: - registered = { - **mealie_registered_exceptions(self.translator), - } - return registered.get(ex, self.t("generic.server-error")) - @cached_property def mixins(self): - return HttpRepo[CreateCookBook, ReadCookBook, UpdateCookBook]( - self.repo, - self.logger, - self.registered_exceptions, - ) + return HttpRepo[CreateCookBook, ReadCookBook, UpdateCookBook](self.repo, self.logger) @router.get("", response_model=CookBookPagination) def get_all(self, q: PaginationQuery = Depends(PaginationQuery)): diff --git a/mealie/routes/groups/controller_group_notifications.py b/mealie/routes/groups/controller_group_notifications.py index 5f9430f1aad0..cd9e2a728465 100644 --- a/mealie/routes/groups/controller_group_notifications.py +++ b/mealie/routes/groups/controller_group_notifications.py @@ -49,7 +49,7 @@ class GroupEventsNotifierController(BaseUserController): @property def mixins(self) -> HttpRepo: - return HttpRepo(self.repo, self.logger, self.registered_exceptions, self.t("generic.server-error")) + return HttpRepo(self.repo, self.logger) @router.get("", response_model=GroupEventPagination) def get_all(self, q: PaginationQuery = Depends(PaginationQuery)): diff --git a/mealie/routes/groups/controller_group_reports.py b/mealie/routes/groups/controller_group_reports.py index 18755d07274e..73509b33c11a 100644 --- a/mealie/routes/groups/controller_group_reports.py +++ b/mealie/routes/groups/controller_group_reports.py @@ -3,7 +3,6 @@ from functools import cached_property from fastapi import APIRouter, HTTPException from pydantic import UUID4 -from mealie.core.exceptions import mealie_registered_exceptions from mealie.routes._base.base_controllers import BaseUserController from mealie.routes._base.controller import controller from mealie.routes._base.mixins import HttpRepo @@ -19,18 +18,9 @@ class GroupReportsController(BaseUserController): def repo(self): return self.repos.group_reports.by_group(self.user.group_id) - def registered_exceptions(self, ex: type[Exception]) -> str: - return { - **mealie_registered_exceptions(self.translator), - }.get(ex, self.t("generic.server-error")) - @cached_property def mixins(self): - return HttpRepo[ReportCreate, ReportOut, ReportCreate]( - self.repo, - self.logger, - self.registered_exceptions, - ) + return HttpRepo[ReportCreate, ReportOut, ReportCreate](self.repo, self.logger) @router.get("", response_model=list[ReportSummary]) def get_all(self, report_type: ReportCategory | None = None): diff --git a/mealie/routes/groups/controller_labels.py b/mealie/routes/groups/controller_labels.py index 2b0db90da306..58d7dfa54209 100644 --- a/mealie/routes/groups/controller_labels.py +++ b/mealie/routes/groups/controller_labels.py @@ -38,7 +38,7 @@ class MultiPurposeLabelsController(BaseUserController): @property def mixins(self) -> HttpRepo: - return HttpRepo(self.repo, self.logger, self.registered_exceptions, self.t("generic.server-error")) + return HttpRepo(self.repo, self.logger) @router.get("", response_model=MultiPurposeLabelPagination) def get_all(self, q: PaginationQuery = Depends(PaginationQuery)): diff --git a/mealie/routes/groups/controller_mealplan.py b/mealie/routes/groups/controller_mealplan.py index b9c6a8ef8c48..cde0196470de 100644 --- a/mealie/routes/groups/controller_mealplan.py +++ b/mealie/routes/groups/controller_mealplan.py @@ -3,7 +3,6 @@ from functools import cached_property from fastapi import APIRouter, Depends, HTTPException -from mealie.core.exceptions import mealie_registered_exceptions from mealie.repos.repository_meals import RepositoryMeals from mealie.routes._base import controller from mealie.routes._base.base_controllers import BaseCrudController @@ -26,19 +25,9 @@ class GroupMealplanController(BaseCrudController): def repo(self) -> RepositoryMeals: return self.repos.meals.by_group(self.group_id) - def registered_exceptions(self, ex: type[Exception]) -> str: - registered = { - **mealie_registered_exceptions(self.translator), - } - return registered.get(ex, self.t("generic.server-error")) - @cached_property def mixins(self): - return HttpRepo[CreatePlanEntry, ReadPlanEntry, UpdatePlanEntry]( - self.repo, - self.logger, - self.registered_exceptions, - ) + return HttpRepo[CreatePlanEntry, ReadPlanEntry, UpdatePlanEntry](self.repo, self.logger) @router.get("/today") def get_todays_meals(self): diff --git a/mealie/routes/groups/controller_shopping_lists.py b/mealie/routes/groups/controller_shopping_lists.py index bfe66680a40f..d127d8af6e5e 100644 --- a/mealie/routes/groups/controller_shopping_lists.py +++ b/mealie/routes/groups/controller_shopping_lists.py @@ -165,7 +165,7 @@ class ShoppingListController(BaseCrudController): @cached_property def mixins(self) -> HttpRepo[ShoppingListCreate, ShoppingListOut, ShoppingListSave]: - return HttpRepo(self.repo, self.logger, self.registered_exceptions, self.t("generic.server-error")) + return HttpRepo(self.repo, self.logger, self.t("generic.server-error")) @router.get("", response_model=ShoppingListPagination) def get_all(self, q: PaginationQuery = Depends(PaginationQuery)): diff --git a/mealie/routes/recipe/recipe_crud_routes.py b/mealie/routes/recipe/recipe_crud_routes.py index a72888563027..9f1ec506f6b2 100644 --- a/mealie/routes/recipe/recipe_crud_routes.py +++ b/mealie/routes/recipe/recipe_crud_routes.py @@ -3,7 +3,6 @@ from shutil import copyfileobj from zipfile import ZipFile import orjson -import sqlalchemy from fastapi import BackgroundTasks, Depends, File, Form, HTTPException, Query, Request, status from fastapi.datastructures import UploadFile from fastapi.responses import JSONResponse @@ -11,7 +10,6 @@ from pydantic import UUID4, BaseModel, Field from slugify import slugify from starlette.responses import FileResponse -from mealie.core import exceptions from mealie.core.dependencies import temporary_zip_path from mealie.core.dependencies.dependencies import temporary_dir, validate_recipe_token from mealie.core.security import create_recipe_slug_token @@ -145,25 +143,6 @@ router = UserAPIRouter(prefix="/recipes", tags=["Recipe: CRUD"], route_class=Mea @controller(router) class RecipeController(BaseRecipeController): - def handle_exceptions(self, ex: Exception) -> None: - thrownType = type(ex) - - if thrownType == exceptions.PermissionDenied: - self.logger.error("Permission Denied on recipe controller action") - raise HTTPException(status_code=403, detail=ErrorResponse.respond(message="Permission Denied")) - elif thrownType == exceptions.NoEntryFound: - self.logger.error("No Entry Found on recipe controller action") - raise HTTPException(status_code=404, detail=ErrorResponse.respond(message="No Entry Found")) - elif thrownType == sqlalchemy.exc.IntegrityError: - self.logger.error("SQL Integrity Error on recipe controller action") - raise HTTPException(status_code=400, detail=ErrorResponse.respond(message="Recipe already exists")) - else: - self.logger.error("Unknown Error on recipe controller action") - self.logger.exception(ex) - raise HTTPException( - status_code=500, detail=ErrorResponse.respond(message="Unknown Error", exception=ex.__class__.__name__) - ) - # ======================================================================= # URL Scraping Operations @@ -291,11 +270,7 @@ class RecipeController(BaseRecipeController): @router.post("", status_code=201, response_model=str) def create_one(self, data: CreateRecipe) -> str | None: """Takes in a JSON string and loads data into the database as a new entry""" - try: - new_recipe = self.service.create_one(data) - except Exception as e: - self.handle_exceptions(e) - return None + new_recipe = self.service.create_one(data) if new_recipe: self.publish_event( @@ -313,10 +288,7 @@ class RecipeController(BaseRecipeController): @router.post("/{slug}/duplicate", status_code=201, response_model=Recipe) def duplicate_one(self, slug: str, req: RecipeDuplicate) -> Recipe: """Duplicates a recipe with a new custom name if given""" - try: - new_recipe = self.service.duplicate_one(slug, req) - except Exception as e: - self.handle_exceptions(e) + new_recipe = self.service.duplicate_one(slug, req) if new_recipe: self.publish_event( @@ -333,10 +305,7 @@ class RecipeController(BaseRecipeController): @router.put("/{slug}") def update_one(self, slug: str, data: Recipe): """Updates a recipe by existing slug and data.""" - try: - recipe = self.service.update_one(slug, data) - except Exception as e: - self.handle_exceptions(e) + recipe = self.service.update_one(slug, data) if recipe: self.publish_event( @@ -354,10 +323,7 @@ class RecipeController(BaseRecipeController): @router.patch("/{slug}") def patch_one(self, slug: str, data: Recipe): """Updates a recipe by existing slug and data.""" - try: - recipe = self.service.patch_one(slug, data) - except Exception as e: - self.handle_exceptions(e) + recipe = self.service.patch_one(slug, data) if recipe: self.publish_event( @@ -375,11 +341,7 @@ class RecipeController(BaseRecipeController): @router.patch("/{slug}/last-made") def update_last_made(self, slug: str, data: RecipeLastMade): """Update a recipe's last made timestamp""" - - try: - recipe = self.service.update_last_made(slug, data.timestamp) - except Exception as e: - self.handle_exceptions(e) + recipe = self.service.update_last_made(slug, data.timestamp) if recipe: self.publish_event( @@ -397,10 +359,7 @@ class RecipeController(BaseRecipeController): @router.delete("/{slug}") def delete_one(self, slug: str): """Deletes a recipe by slug""" - try: - recipe = self.service.delete_one(slug) - except Exception as e: - self.handle_exceptions(e) + recipe = self.service.delete_one(slug) if recipe: self.publish_event( diff --git a/mealie/routes/recipe/timeline_events.py b/mealie/routes/recipe/timeline_events.py index 445112716c82..cb5155777eec 100644 --- a/mealie/routes/recipe/timeline_events.py +++ b/mealie/routes/recipe/timeline_events.py @@ -30,9 +30,7 @@ class RecipeTimelineEventsController(BaseCrudController): @cached_property def mixins(self): return HttpRepo[RecipeTimelineEventCreate, RecipeTimelineEventOut, RecipeTimelineEventUpdate]( - self.repo, - self.logger, - self.registered_exceptions, + self.repo, self.logger ) def get_recipe_from_slug(self, slug: str) -> Recipe: diff --git a/mealie/routes/unit_and_foods/foods.py b/mealie/routes/unit_and_foods/foods.py index 95e63a2a8f01..02bb0923e0b4 100644 --- a/mealie/routes/unit_and_foods/foods.py +++ b/mealie/routes/unit_and_foods/foods.py @@ -29,11 +29,7 @@ class IngredientFoodsController(BaseUserController): @cached_property def mixins(self): - return HttpRepo[SaveIngredientFood, IngredientFood, CreateIngredientFood]( - self.repo, - self.logger, - self.registered_exceptions, - ) + return HttpRepo[SaveIngredientFood, IngredientFood, CreateIngredientFood](self.repo, self.logger) @router.put("/merge", response_model=SuccessResponse) def merge_one(self, data: MergeFood): diff --git a/mealie/routes/unit_and_foods/units.py b/mealie/routes/unit_and_foods/units.py index 83f193a3df4a..940a9bdd8278 100644 --- a/mealie/routes/unit_and_foods/units.py +++ b/mealie/routes/unit_and_foods/units.py @@ -29,11 +29,7 @@ class IngredientUnitsController(BaseUserController): @cached_property def mixins(self): - return HttpRepo[CreateIngredientUnit, IngredientUnit, CreateIngredientUnit]( - self.repo, - self.logger, - self.registered_exceptions, - ) + return HttpRepo[CreateIngredientUnit, IngredientUnit, CreateIngredientUnit](self.repo, self.logger) @router.put("/merge", response_model=SuccessResponse) def merge_one(self, data: MergeUnit):