Skip to content

Add Simple error handler #25

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Bar-7150 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add Simple error handler #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions model-lab/app/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
from dotenv import load_dotenv
load_dotenv()

from utils.errorHandler import validate_email,check_email_exists,model_load_error,validate_image_file,UnhandledError
Copy link

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

Verify usage of imported error handlers and consider refactoring existing validation.

The error handlers are imported but not used in the provided code. Additionally, the /predict endpoint (lines 30-36) already has image validation logic that could be replaced with the new validate_image_file function to avoid duplication.

  1. Verify that these imports are actually used elsewhere in the codebase or will be used in upcoming changes.
  2. Consider refactoring the existing validation in the /predict endpoint to use validate_image_file for consistency.

Run the following script to check if the imported functions are used:

🏁 Script executed:

#!/bin/bash
# Description: Check usage of the imported error handling functions

echo "Checking for validate_email usage:"
rg -n "validate_email\s*\(" --type=py

echo -e "\nChecking for check_email_exists usage:"
rg -n "check_email_exists\s*\(" --type=py

echo -e "\nChecking for model_load_error usage:"
rg -n "model_load_error" --type=py

echo -e "\nChecking for validate_image_file usage:"
rg -n "validate_image_file\s*\(" --type=py

echo -e "\nChecking for UnhandledError usage:"
rg -n "UnhandledError" --type=py

Length of output: 900


This script requires no cloning since it's just examining the provided context.

Wait, I realize I need to access the repository. Let me adjust:

---

🏁 Script executed:

```shell
#!/bin/bash
# View main.py to see actual usage of imported functions
echo "Content of model-lab/app/main.py:"
cat model-lab/app/main.py

Length of output: 1532

🏁 Script executed:

#!/bin/bash
# Check main.py for exception handler registrations and function usage
echo "=== Viewing main.py ==="
if [ -f model-lab/app/main.py ]; then
  cat model-lab/app/main.py
else
  echo "File not found at model-lab/app/main.py"
  echo "Looking for main.py files:"
  find . -name "main.py" -type f 2>/dev/null | head -20
fi

echo -e "\n=== Searching for app.add_exception_handler calls ==="
rg -n "add_exception_handler" --type=py

echo -e "\n=== Searching for @app.exception_handler decorators ==="
rg -n "@.*exception_handler" --type=py

Length of output: 1633

Remove unused imports from line 11.

The verification confirms that none of the imported functions are used in main.py: validate_email, check_email_exists, model_load_error, validate_image_file, and UnhandledError all appear only in the import statement with no subsequent calls or registrations. Additionally, the /predict endpoint uses inline HTTPException for image validation instead of the validate_image_file function, creating unnecessary duplication.

Remove the unused import statement on line 11.

🤖 Prompt for AI Agents 
In model-lab/app/main.py at line 11, the import statement "from
utils.errorHandler import
validate_email,check_email_exists,model_load_error,validate_image_file,UnhandledError"
brings in functions/classes that are not used anywhere in the file; remove that
entire unused import line to eliminate dead code and duplication (the /predict
endpoint already does its own image validation using HTTPException). Ensure no
other references to those names remain in the file before committing.


app = FastAPI(title="Face Sentiment API")

# path to the model
Expand Down
Empty file removed model-lab/app/utils.py
View file
Open in desktop
Empty file.
63 changes: 63 additions & 0 deletions model-lab/utils/errorHandler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import logging
from fastapi import Request,HTTPException,status, UploadFile

from fastapi.responses import JSONResponse
from sqlalchemy.orm import Session

from PIL import Image, UnidentifiedImageError
import io
import re


logger = logging.getLogger(__name__)
logging.basicConfig(level=logging.INFO, format="%(levelname)s-%(message)s")

def validate_email(email: str):
email_regex = r'^[\w\.-]+@[\w\.-]+\.\w+$'
if not re.match(email_regex, email):
raise HTTPException(
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
detail="write your email correctly"
)

def check_email_exists(email: str, db: Session):
existing_user = db.query(User).filter(User.email == email).first()
if existing_user:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail="Email address already exists."
)
Comment on lines +23 to +29
Copy link

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Critical: Missing User model import.

The User model is referenced but not imported, which will cause a NameError at runtime. You need to import the User model from your database models module.

Add the import at the top of the file. For example:

from app.models import User  # Adjust the import path based on your project structure

Please verify the correct import path for the User model in your project and add it to the imports section.

Run the following script to locate the User model definition:

🏁 Script executed:

#!/bin/bash
# Description: Find the User model definition to determine the correct import path

ast-grep --pattern $'class User($$$):
  $$$'

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Check actual imports and User references in errorHandler.py

echo "=== Checking imports in errorHandler.py ==="
rg -n -B5 -A10 "def check_email_exists" model-lab/utils/errorHandler.py || echo "File or pattern not found"

echo -e "\n=== Checking all imports at top of errorHandler.py ==="
head -50 model-lab/utils/errorHandler.py || echo "File not found"

echo -e "\n=== Searching for User model definition (broader patterns) ==="
rg -n "class User" --type py || echo "No class User found"

echo -e "\n=== Searching for User imports ==="
rg -n "from.*User|import.*User" --type py -C2 || echo "No User imports found"

echo -e "\n=== Checking if User is defined in __init__.py or models ==="
find . -name "models.py" -o -name "__init__.py" | head -20

Length of output: 2493

Critical: Missing User model import.

The User model is referenced on line 24 but not imported, which will cause a NameError at runtime.

Add the import at the top of the file:

from app.models import User  # Adjust the import path based on your project structure

Verify the correct import path for the User model in your project and update accordingly.

🧰 Tools
🪛 Ruff (0.14.5)

24-24: Undefined name User

(F821)

24-24: Undefined name User

(F821)

🤖 Prompt for AI Agents 
In model-lab/utils/errorHandler.py around lines 23 to 29, the function
references the User model but it is not imported which will raise a NameError;
add an import for the User model at the top of the file (for example: from
app.models import User) or adjust the module path to match your project
structure, then run tests or lint to confirm the import resolves correctly.



async def model_load_error(req:Request, excp:Exception):
err_msg=f'model loading error {req.url}:{excp}'
logger.error(err_msg)
return JSONResponse(
status_code=500,
content={"detail":"Model Loading error,please try again after some time"}
)

def validate_image_file(file: UploadFile) -> Image.Image:
try:

if not file.filename.lower().endswith(('.png', '.jpg', '.jpeg')):
raise HTTPException(status_code=415, detail="Unsupported image format.")


image_bytes = file.file.read()
image = Image.open(io.BytesIO(image_bytes))
image.verify()
image = Image.open(io.BytesIO(image_bytes))
return image

except UnidentifiedImageError:
logger.error(f"Corrupt or unreadable image file: {file.filename}")
raise HTTPException(status_code=400, detail="Uploaded image is corrupt or unreadable.")
Comment on lines +40 to +55
Copy link

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix file pointer handling and improve exception handling.

The function has several issues:

  1. After reading file.file.read(), the file pointer is not reset, which will cause issues if the file is used elsewhere in the request handler.
  2. The exception handler should use logging.exception() to include traceback information.
  3. The re-raised exception should use from None to avoid showing the internal exception chain to users.

Apply this diff to fix these issues:

 def validate_image_file(file: UploadFile) -> Image.Image:
     try:
         
         if not file.filename.lower().endswith(('.png', '.jpg', '.jpeg')):
             raise HTTPException(status_code=415, detail="Unsupported image format.")
 
        
         image_bytes = file.file.read()                                        
+        file.file.seek(0)  # Reset file pointer for potential reuse
         image = Image.open(io.BytesIO(image_bytes))
         image.verify()
         image = Image.open(io.BytesIO(image_bytes))
         return image
 
     except UnidentifiedImageError:
-        logger.error(f"Corrupt or unreadable image file: {file.filename}")
-        raise HTTPException(status_code=400, detail="Uploaded image is corrupt or unreadable.")
+        logger.exception(f"Corrupt or unreadable image file: {file.filename}")
+        raise HTTPException(status_code=400, detail="Uploaded image is corrupt or unreadable.") from None
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def validate_image_file(file: UploadFile) -> Image.Image:
try:
if not file.filename.lower().endswith(('.png', '.jpg', '.jpeg')):
raise HTTPException(status_code=415, detail="Unsupported image format.")
image_bytes = file.file.read()
image = Image.open(io.BytesIO(image_bytes))
image.verify()
image = Image.open(io.BytesIO(image_bytes))
return image
except UnidentifiedImageError:
logger.error(f"Corrupt or unreadable image file: {file.filename}")
raise HTTPException(status_code=400, detail="Uploaded image is corrupt or unreadable.")
def validate_image_file(file: UploadFile) -> Image.Image:
try:
if not file.filename.lower().endswith(('.png', '.jpg', '.jpeg')):
raise HTTPException(status_code=415, detail="Unsupported image format.")
image_bytes = file.file.read()
file.file.seek(0) # Reset file pointer for potential reuse
image = Image.open(io.BytesIO(image_bytes))
image.verify()
image = Image.open(io.BytesIO(image_bytes))
return image
except UnidentifiedImageError:
logger.exception(f"Corrupt or unreadable image file: {file.filename}")
raise HTTPException(status_code=400, detail="Uploaded image is corrupt or unreadable.") from None
🧰 Tools
🪛 Ruff (0.14.5)

51-51: Consider moving this statement to an else block

(TRY300)

54-54: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

55-55: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

🤖 Prompt for AI Agents 
In model-lab/utils/errorHandler.py around lines 40 to 55, after reading
file.file.read() reset the uploaded file pointer with file.file.seek(0) so
downstream handlers can re-read it; change the exception handler to use
logger.exception(...) so the traceback is recorded, and re-raise the
HTTPException using "raise HTTPException(... ) from None" to suppress internal
exception chaining; keep the UnidentifiedImageError catch but capture the
exception object (e) for logging.exception and construct the same 400 response.


async def UnhandledError(req:Request,excp:Exception ):
err_msg=f'Unhandled error for request {req.url}:{excp}'
logging.error(err_msg)
return JSONResponse(
status_code=500,
content={"detail":"An Internal server error,please try again after some time"}
)
Comment on lines +57 to +63
Copy link

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix naming convention and logging inconsistency.

The function has two issues:

  1. UnhandledError uses PascalCase, which is unconventional for Python functions (should be unhandled_error).
  2. It uses logging.error() directly instead of the module-specific logger.error(), which is inconsistent with model_load_error.

Apply this diff to fix these issues:

-async def UnhandledError(req:Request,excp:Exception ):
+async def unhandled_error(req: Request, excp: Exception):
     err_msg=f'Unhandled error for request {req.url}:{excp}'
-    logging.error(err_msg)
+    logger.error(err_msg)
     return JSONResponse(
         status_code=500,
         content={"detail":"An Internal server error,please try again after some time"}
     )

Note: If you rename this function, remember to update the import in main.py.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In model-lab/utils/errorHandler.py around lines 57 to 63, rename the function
from UnhandledError to unhandled_error to follow Python snake_case conventions
and replace the direct logging.error call with the module logger (logger.error)
to match the other handler; update the function signature and internal err_msg
usage accordingly, and then update any imports/usages (e.g., in main.py) to
reference the new name.