Conversation
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
Adds a Python logging logger to the ASGI module so errors are surfaced to stderr instead of silently swallowed.
Issues
-
Bug —
logger.exception()in done callback loses traceback (asgi.py:24):logger.exception()relies onsys.exc_info()to capture the traceback, but future done callbacks execute outside anyexceptblock, sosys.exc_info()returns(None, None, None). The exception type and traceback are silently dropped. Uselogger.error(..., exc_info=exc)to pass the exception object directly — Python's logging will extract the traceback from it. -
Bug — two test assertions will fail (
worker.py:175-176,worker.py:203-204):r.getMessage()only returns the formatted message string (e.g."Unhandled exception in background task"), not the exception text. The tests check for the exception's message (e.g."post-response error for testing") insidegetMessage(), which will never match. The assertions need to inspectr.exc_infoor user.exc_text/ format the full record instead.
| background_tasks.discard(f) | ||
| exc = f.exception() if not f.cancelled() else None | ||
| if exc is not None: | ||
| logger.exception("Unhandled exception in background task") |
There was a problem hiding this comment.
logger.exception() calls sys.exc_info() internally, but future done callbacks don't run inside an except block, so exc_info is always (None, None, None) — the traceback is silently lost.
Pass the exception object directly via exc_info=:
| logger.exception("Unhandled exception in background task") | |
| logger.error("Unhandled exception in background task", exc_info=exc) |
|
I'm Bonk, and I've done a quick review of your PR. Adds a Python Issues found
All three issues have concrete suggestion comments posted on the PR. |
ryanking13
force-pushed
the
gyeongjae/asgi-logging-error
branch
from
e833768 to
045fa61
Compare
dom96
left a comment
dom96
left a comment
There was a problem hiding this comment.
Nice! Awesome QoL improvement, thanks!
2 participants
Adds logger to the Python ASGI module, users can overwrite the logger behavior by
before this change, all the errors were not showed to the user at all, while after this change, errors are caught and showed to stderr.
For example, the bug reported in https://discord.com/channels/595317990191398933/1224715939581530112/1473759889582194758 which showed nothing now shows
which helps us to debug the issue.