From af9e1f395b00dbd437b1a77f0c9f8ffb230da085 Mon Sep 17 00:00:00 2001 From: Piv94165 <106757110+Piv94165@users.noreply.github.com> Date: Fri, 10 Nov 2023 15:09:49 +0100 Subject: [PATCH] refactor(backend): better error handling in backend (#277) Add specific messages if Github settings were not added and change some status codes in backend ### What Some errors raised by backend have a 500 status code intead of having 4XX error. For example, if a developer forgets to add a Personal Access Token and wants to import a project to edit, the server throws a 500 error instead of 400 error. ### Screenshot ![image](https://github.com/openfoodfacts/taxonomy-editor/assets/106757110/e0c24f5a-9de7-47b1-a31b-20741729df8b) ### Fixes bug(s) ### Part of - backend/editor --------- Co-authored-by: alice.juan --- backend/editor/api.py | 23 ++++++++++---- backend/editor/github_functions.py | 51 +++++++++++++++++++++--------- 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/backend/editor/api.py b/backend/editor/api.py index ae236e24..ad1c6f92 100644 --- a/backend/editor/api.py +++ b/backend/editor/api.py @@ -24,6 +24,7 @@ status, ) from fastapi.encoders import jsonable_encoder +from fastapi.exception_handlers import http_exception_handler from fastapi.exceptions import RequestValidationError from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import FileResponse, JSONResponse @@ -110,8 +111,8 @@ def file_cleanup(filepath): """ try: os.remove(filepath) - except Exception as e: - raise HTTPException(status_code=500, detail="Taxonomy file not found for deletion") from e + except FileNotFoundError: + log.warn(f"Taxonomy file {filepath} not found for deletion") class StatusFilter(str, Enum): @@ -139,6 +140,16 @@ async def validation_exception_handler(request: Request, exc: RequestValidationE ) +@app.exception_handler(HTTPException) +async def log_http_exception(request: Request, exc: HTTPException): + """ + Custom exception handler to log FastAPI exceptions. + """ + # Log the detail message + log.info(f" ERROR: {exc.detail}") + return await http_exception_handler(request, exc) + + # Get methods @@ -351,10 +362,10 @@ async def export_to_github( return url except GithubBranchExistsError: - raise HTTPException(status_code=500, detail="The Github branch already exists!") + raise HTTPException(status_code=409, detail="The Github branch already exists!") except GithubUploadError: - raise HTTPException(status_code=500, detail="Github upload error!") + raise HTTPException(status_code=400, detail="Github upload error!") # Post methods @@ -370,7 +381,7 @@ async def import_from_github(request: Request, branch: str, taxonomy_name: str): taxonomy = TaxonomyGraph(branch, taxonomy_name) if not taxonomy.is_valid_branch_name(): - raise HTTPException(status_code=400, detail="branch_name: Enter a valid branch name!") + raise HTTPException(status_code=422, detail="branch_name: Enter a valid branch name!") if await taxonomy.does_project_exist(): raise HTTPException(status_code=409, detail="Project already exists!") if not await taxonomy.is_branch_unique(): @@ -390,7 +401,7 @@ async def upload_taxonomy( # use the file name as the taxonomy name taxonomy = TaxonomyGraph(branch, taxonomy_name) if not taxonomy.is_valid_branch_name(): - raise HTTPException(status_code=400, detail="branch_name: Enter a valid branch name!") + raise HTTPException(status_code=422, detail="branch_name: Enter a valid branch name!") if await taxonomy.does_project_exist(): raise HTTPException(status_code=409, detail="Project already exists!") if not await taxonomy.is_branch_unique(): diff --git a/backend/editor/github_functions.py b/backend/editor/github_functions.py index 59bfcff9..4c0e683e 100644 --- a/backend/editor/github_functions.py +++ b/backend/editor/github_functions.py @@ -3,7 +3,8 @@ """ from textwrap import dedent -from github import Github +from fastapi import HTTPException +from github import Github, GithubException from . import settings @@ -21,8 +22,19 @@ def init_driver_and_repo(self): """ Initalize connection to Github with an access token """ - github_driver = Github(settings.access_token) - repo = github_driver.get_repo(settings.repo_uri) + access_token = settings.access_token + if not access_token: + raise HTTPException( + status_code=400, + detail="Access token is not set. Please add your access token in .env", + ) + repo_uri = settings.repo_uri + if not repo_uri: + raise HTTPException( + status_code=400, detail="repo_uri is not set. Please add your access token in .env" + ) + github_driver = Github(access_token) + repo = github_driver.get_repo(repo_uri) return repo def list_all_branches(self): @@ -47,19 +59,28 @@ def update_file(self, filename): # Find taxonomy text file to be updated github_filepath = f"taxonomies/{self.taxonomy_name}.txt" commit_message = f"Update {self.taxonomy_name}.txt" + try: + current_file = self.repo.get_contents(github_filepath) + with open(filename, "r") as f: + new_file_contents = f.read() - current_file = self.repo.get_contents(github_filepath) - with open(filename, "r") as f: - new_file_contents = f.read() - - # Update the file - self.repo.update_file( - github_filepath, - commit_message, - new_file_contents, - current_file.sha, - branch=self.branch_name, - ) + # Update the file + self.repo.update_file( + github_filepath, + commit_message, + new_file_contents, + current_file.sha, + branch=self.branch_name, + ) + except GithubException as e: + # Handle GitHub API-related exceptions + raise Exception(f"GitHub API error: {e}") from e + except FileNotFoundError as e: + # Handle file not found error (e.g., when 'filename' does not exist) + raise Exception(f"File not found: {filename}") from e + except Exception as e: + # Handle any other unexpected exceptions + raise Exception(f"An error occurred: {e}") from e def create_pr(self, description): """