Skip to content
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.

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

[Bug] Fix incorrect error handling #65

Closed
phoenixpereira opened this issue Dec 14, 2024 · 3 comments · Fixed by #69
Closed

[Bug] Fix incorrect error handling #65

phoenixpereira opened this issue Dec 14, 2024 · 3 comments · Fixed by #69
Assignees
Labels
bug Something isn't working

Comments

@phoenixpereira
Copy link
Member

Description of the Bug

When making an invalid API request or a request for a course id that doesn't exist, it returns a response code of 500.

Reproduction Steps

Send a request to https://courses-api.csclub.org.au/subjects or https://courses-api.csclub.org.au/courses/aaa.

Expected Behaviour

When making an invalid API request, it should return a response code of 400 instead of 500. Similarly, requests with a non-existing course id should return 404 instead of 500.

Additional Notes

For some reason, restarting the Docker container temporarily fixes this issue.

@phoenixpereira phoenixpereira added the bug Something isn't working label Dec 14, 2024
@phoenixpereira phoenixpereira moved this to Todo in Courses API Dec 14, 2024
@SouthernPolaris SouthernPolaris self-assigned this Jan 23, 2025
@rohitkrsoni
Copy link
Contributor

Hi @phoenixpereira and @SouthernPolaris , I think the issue lies in here at line 345, as course.course_id is getting accessed before the exception is raised even though course could be null.

courses-api/src/server.py

Lines 334 to 349 in 18d1ca2

def get_course(course_cid: str, db: Session = Depends(get_db)):
"""Course details route, takes in an id returns the courses' info and classes.
Args:
course_cid (string, required): The id to search for.
Returns:
dict: A dictionary containing the course information and classes.
"""
course = db.query(Course).filter(Course.id == course_cid).first()
course_id = course.course_id
if not course:
raise HTTPException(status_code=404, detail="Course not found")

Let me know if I can work on this issue, will check it on my local for subjects and courses

thanks,
Rohit

@phoenixpereira
Copy link
Member Author

Hi @phoenixpereira and @SouthernPolaris , I think the issue lies in here at line 345, as course.course_id is getting accessed before the exception is raised even though course could be null.

courses-api/src/server.py

Lines 334 to 349 in 18d1ca2

def get_course(course_cid: str, db: Session = Depends(get_db)):
"""Course details route, takes in an id returns the courses' info and classes.

 Args: 
     course_cid (string, required): The id to search for. 

 Returns: 
     dict: A dictionary containing the course information and classes. 
 """ 
 course = db.query(Course).filter(Course.id == course_cid).first() 

 course_id = course.course_id 

 if not course: 
     raise HTTPException(status_code=404, detail="Course not found") 

Let me know if I can work on this issue, will check it on my local for subjects and courses

thanks, Rohit

@rohitkrsoni Yeah that might be it, if can you fix it that will be great!

@rohitkrsoni
Copy link
Contributor

rohitkrsoni commented Mar 4, 2025

PR: #69, someone please review it, thanks

@github-project-automation github-project-automation bot moved this from Todo to Done in Courses API Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants