From f94755f8d13e576af5fafa9221a3ca5c372a96f7 Mon Sep 17 00:00:00 2001 From: Abram Date: Thu, 29 Aug 2024 12:30:15 +0100 Subject: [PATCH 1/3] fix (backend): raise NoResultFound to ensure proper error handling when a base is missing --- agenta-backend/agenta_backend/services/db_manager.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/agenta-backend/agenta_backend/services/db_manager.py b/agenta-backend/agenta_backend/services/db_manager.py index 699406a324..63d184c53b 100644 --- a/agenta-backend/agenta_backend/services/db_manager.py +++ b/agenta-backend/agenta_backend/services/db_manager.py @@ -325,6 +325,8 @@ async def fetch_base_by_id(base_id: str) -> Optional[VariantBaseDB]: .filter_by(id=uuid.UUID(base_uuid)) ) base = result.scalars().first() + if base is None: + raise NoResultFound(f"Base with id {base_id} not found") return base From 52fab02aeb76b3edf12a1fe624ed4440b49ae22e Mon Sep 17 00:00:00 2001 From: Abram Date: Tue, 10 Sep 2024 10:55:09 +0100 Subject: [PATCH 2/3] refactor (backend): ensure errors are caught and handled gracefully in /container_url/ endpoint --- .../routers/container_router.py | 65 ++++++++++--------- 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/agenta-backend/agenta_backend/routers/container_router.py b/agenta-backend/agenta_backend/routers/container_router.py index 4dfea8a01d..f2bdd7290c 100644 --- a/agenta-backend/agenta_backend/routers/container_router.py +++ b/agenta-backend/agenta_backend/routers/container_router.py @@ -159,39 +159,39 @@ async def construct_app_container_url( Raises: HTTPException: If the base or variant cannot be found or the user does not have access. """ - # assert that one of base_id or variant_id is provided - assert base_id or variant_id, "Please provide either base_id or variant_id" - - if base_id: - object_db = await db_manager.fetch_base_by_id(base_id) - elif variant_id and variant_id != "None": - # NOTE: Backward Compatibility - # --------------------------- - # When a user creates a human evaluation with a variant and later deletes the variant, - # the human evaluation page becomes inaccessible due to the backend raising a - # "'NoneType' object has no attribute 'variant_id'" error. To suppress this error, - # we will return the string "None" as the ID of the variant. - # This change ensures that users can still view their evaluations; however, - # they will no longer be able to access a deployment URL for the deleted variant. - # Therefore, we ensure that variant_id is not "None". - object_db = await db_manager.fetch_app_variant_by_id(variant_id) - else: - # NOTE: required for backward compatibility - object_db = None - - # Check app access - if isCloudEE() and object_db is not None: - has_permission = await check_action_access( - user_uid=request.state.user_id, - object=object_db, - permission=Permission.VIEW_APPLICATION, - ) - if not has_permission: - error_msg = f"You do not have permission to perform this action. Please contact your organization admin." - logger.error(error_msg) - raise HTTPException(status_code=403, detail=error_msg) try: + # assert that one of base_id or variant_id is provided + assert base_id or variant_id, "Please provide either base_id or variant_id" + + if base_id: + object_db = await db_manager.fetch_base_by_id(base_id) + elif variant_id and variant_id != "None": + # NOTE: Backward Compatibility + # --------------------------- + # When a user creates a human evaluation with a variant and later deletes the variant, + # the human evaluation page becomes inaccessible due to the backend raising a + # "'NoneType' object has no attribute 'variant_id'" error. To suppress this error, + # we will return the string "None" as the ID of the variant. + # This change ensures that users can still view their evaluations; however, + # they will no longer be able to access a deployment URL for the deleted variant. + # Therefore, we ensure that variant_id is not "None". + object_db = await db_manager.fetch_app_variant_by_id(variant_id) + else: + # NOTE: required for backward compatibility + object_db = None + + # Check app access + if isCloudEE() and object_db is not None: + has_permission = await check_action_access( + user_uid=request.state.user_id, + object=object_db, + permission=Permission.VIEW_APPLICATION, + ) + if not has_permission: + error_msg = f"You do not have permission to perform this action. Please contact your organization admin." + logger.error(error_msg) + raise HTTPException(status_code=403, detail=error_msg) if getattr(object_db, "deployment_id", None): # this is a base deployment = await db_manager.get_deployment_by_id( str(object_db.deployment_id) # type: ignore @@ -207,4 +207,5 @@ async def construct_app_container_url( ) return URI(uri=deployment.uri) except Exception as e: - return JSONResponse({"message": str(e)}, status_code=500) + status_code = e.status_code if hasattr(e, "status_code") else 500 + return JSONResponse({"detail": str(e)}, status_code=status_code) From 6c53de52aa8adc4a829070d25af6d7484820c7f0 Mon Sep 17 00:00:00 2001 From: Abraham 'Abram' Israel Date: Mon, 21 Oct 2024 10:56:21 +0100 Subject: [PATCH 3/3] chore (backend): fix broken try-except block in container_router --- .../routers/container_router.py | 89 ++++++++++--------- 1 file changed, 45 insertions(+), 44 deletions(-) diff --git a/agenta-backend/agenta_backend/routers/container_router.py b/agenta-backend/agenta_backend/routers/container_router.py index 63d3695964..199ca3fd3c 100644 --- a/agenta-backend/agenta_backend/routers/container_router.py +++ b/agenta-backend/agenta_backend/routers/container_router.py @@ -159,52 +159,53 @@ async def construct_app_container_url( HTTPException: If the base or variant cannot be found or the user does not have access. """ - # assert that one of base_id or variant_id is provided - assert base_id or variant_id, "Please provide either base_id or variant_id" - - if base_id: - object_db = await db_manager.fetch_base_by_id(base_id) - elif variant_id and variant_id != "None": - # NOTE: Backward Compatibility - # --------------------------- - # When a user creates a human evaluation with a variant and later deletes the variant, - # the human evaluation page becomes inaccessible due to the backend raising a - # "'NoneType' object has no attribute 'variant_id'" error. To suppress this error, - # we will return the string "None" as the ID of the variant. - # This change ensures that users can still view their evaluations; however, - # they will no longer be able to access a deployment URL for the deleted variant. - # Therefore, we ensure that variant_id is not "None". - object_db = await db_manager.fetch_app_variant_by_id(variant_id) - else: - # NOTE: required for backward compatibility - object_db = None - - # Check app access - if isCloudEE() and object_db is not None: - has_permission = await check_action_access( - user_uid=request.state.user_id, - project_id=str(object_db.project_id), - permission=Permission.VIEW_APPLICATION, - ) - if not has_permission: - error_msg = f"You do not have permission to perform this action. Please contact your organization admin." - logger.error(error_msg) - raise HTTPException(status_code=403, detail=error_msg) - - if getattr(object_db, "deployment_id", None): # this is a base - deployment = await db_manager.get_deployment_by_id( - str(object_db.deployment_id) # type: ignore - ) - elif getattr(object_db, "base_id", None): # this is a variant - deployment = await db_manager.get_deployment_by_id( - str(object_db.base.deployment_id) # type: ignore - ) + try: + # assert that one of base_id or variant_id is provided + assert base_id or variant_id, "Please provide either base_id or variant_id" + + if base_id: + object_db = await db_manager.fetch_base_by_id(base_id) + elif variant_id and variant_id != "None": + # NOTE: Backward Compatibility + # --------------------------- + # When a user creates a human evaluation with a variant and later deletes the variant, + # the human evaluation page becomes inaccessible due to the backend raising a + # "'NoneType' object has no attribute 'variant_id'" error. To suppress this error, + # we will return the string "None" as the ID of the variant. + # This change ensures that users can still view their evaluations; however, + # they will no longer be able to access a deployment URL for the deleted variant. + # Therefore, we ensure that variant_id is not "None". + object_db = await db_manager.fetch_app_variant_by_id(variant_id) else: - raise HTTPException( - status_code=400, - detail="Deployment not found", + # NOTE: required for backward compatibility + object_db = None + + # Check app access + if isCloudEE() and object_db is not None: + has_permission = await check_action_access( + user_uid=request.state.user_id, + project_id=str(object_db.project_id), + permission=Permission.VIEW_APPLICATION, ) - return URI(uri=deployment.uri) + if not has_permission: + error_msg = f"You do not have permission to perform this action. Please contact your organization admin." + logger.error(error_msg) + raise HTTPException(status_code=403, detail=error_msg) + + if getattr(object_db, "deployment_id", None): # this is a base + deployment = await db_manager.get_deployment_by_id( + str(object_db.deployment_id) # type: ignore + ) + elif getattr(object_db, "base_id", None): # this is a variant + deployment = await db_manager.get_deployment_by_id( + str(object_db.base.deployment_id) # type: ignore + ) + else: + raise HTTPException( + status_code=400, + detail="Deployment not found", + ) + return URI(uri=deployment.uri) except Exception as e: status_code = e.status_code if hasattr(e, "status_code") else 500 return JSONResponse({"detail": str(e)}, status_code=status_code)