Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this pull request! The email notification system is well-implemented, and the profile schemas are perfect. I am approving this to help you progress, but I have one suggestion for improvement.
In src/routes/profiles.py, the try...except Exception: blocks for token validation and avatar upload are too broad. This can hide different kinds of errors under a single message, making debugging harder. It's better practice to catch more specific exceptions, like JWT-related errors for token validation and the StorageError you've imported for file uploads.
This is a minor point in an otherwise fantastic submission. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| # 1) Token validation (expired/invalid) | ||
| try: | ||
| payload = jwt.decode_access_token(token) # <-- звір назву у своєму JWTAuthManager | ||
| except Exception: |
There was a problem hiding this comment.
Using a generic except Exception: can hide different kinds of token errors (like an invalid signature or a malformed token) under a single "Token has expired." message. It's better practice to catch more specific exceptions from your JWT library to provide more accurate error details to the client.
| # або put_object(bucket, key, bytes, content_type)->None + get_url(key)->str | ||
| avatar_url = s3_client.upload_file(avatar, key) # <-- підлаштуй під свій інтерфейс | ||
|
|
||
| except Exception: |
There was a problem hiding this comment.
This except Exception: is very broad and could catch errors that are not related to the file upload itself. Consider catching a more specific exception from the storage client, like the StorageError you've imported on line 17. This ensures that only storage-related issues are reported as an upload failure.
2 participants
No description provided.