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

Use repr for error logging #1711

Merged
merged 2 commits into from
Jun 13, 2023
Merged

Conversation

Ruwann
Copy link
Member

@Ruwann Ruwann commented Jun 12, 2023

Follow-up on #1708 (comment)

As mentioned there, there is a __repr__ but not a __str__, leading to an empty log message (for HTTPExceptions).

An example when running the dev server (previously the first line would just be an empty line):

HTTPException(status_code=404, detail='Not Found')
INFO:     127.0.0.1:33538 - "GET /swaggers/ui/ HTTP/1.1" 404 Not Found

However, I would also be fine with just removing the logging here as HTTPExceptions can be part of the normal flow, without indicating an actual application error (case in point: raising a Not Found exception)

@Ruwann Ruwann requested a review from RobbeSneyders June 12, 2023 13:52
@coveralls
Copy link

Coverage Status

coverage: 94.045%. remained the same when pulling c30f74b on Ruwann:bugfix/exception-log into 55f3c59 on spec-first:main.

Copy link
Member

@RobbeSneyders RobbeSneyders left a comment

Choose a reason for hiding this comment

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

Thanks @Ruwann!

I think it's fine to log just the error when a handler is registered, and a traceback when no handler is available.

@RobbeSneyders RobbeSneyders merged commit 59a09c7 into spec-first:main Jun 13, 2023
@Ruwann Ruwann deleted the bugfix/exception-log branch June 14, 2023 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants