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.

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

(958) Add exception logging in debug mode #976

Merged
merged 19 commits into from
Feb 4, 2023
Merged

(958) Add exception logging in debug mode #976

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
5320752
feat: add console output for exceptions in debug mode
JacobCoffee Dec 22, 2022
5f0a127
Merge branch 'main' into 985_debug_console_log_enhancements
JacobCoffee Jan 8, 2023
0f19be7
chore: middleware debug logging updates
JacobCoffee Jan 21, 2023
5105bd2
Merge branch 'main' into 985_debug_console_log_enhancements
cofin Jan 23, 2023
ad951a3
feat: #958 testing passing
JacobCoffee Jan 23, 2023
998c4cd
Update starlite/middleware/exceptions/middleware.py
JacobCoffee Jan 24, 2023
fd3c4be
Merge branch 'main' into 985_debug_console_log_enhancements
JacobCoffee Jan 27, 2023
5f37e99
chore: fixed test for middleware debug logging (#958)
JacobCoffee Jan 27, 2023
bac6a18
Update tests/middleware/test_exception_handler_middleware.py
provinzkraut Jan 31, 2023
de75bdf
Merge branch 'main' into 985_debug_console_log_enhancements
JacobCoffee Jan 31, 2023
e42d927
chore: fixed test for middleware debug logging (#958)
JacobCoffee Feb 1, 2023
faf06a1
Merge branch 'main' into 985_debug_console_log_enhancements
peterschutt Feb 3, 2023
dc74af2
Use `get_logger` fixture to test exception logged in debug.
peterschutt Feb 3, 2023
7a173e7
Merge pull request #4 from peterschutt/985_debug_console_log_enhancem…
JacobCoffee Feb 3, 2023
a65bfbc
Merge branch 'main' into 985_debug_console_log_enhancements
JacobCoffee Feb 3, 2023
9bc01e6
chore: Updated assertions (#958)
JacobCoffee Feb 3, 2023
8a0d0ed
Update starlite/middleware/exceptions/middleware.py
JacobCoffee Feb 3, 2023
dffccbf
chore: update logger method
JacobCoffee Feb 3, 2023
bde38f2
Merge branch 'main' into 985_debug_console_log_enhancements
JacobCoffee Feb 4, 2023
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 starlite/middleware/exceptions/middleware.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ async def __call__(self, scope: "Scope", receive: "Receive", send: "Send") -> No
await self.app(scope, receive, send)
except Exception as e: # pylint: disable=broad-except
starlite_app = scope["app"]
if self.debug and (logger := starlite_app.logger):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if self.debug and (logger := starlite_app.logger):
if self.debug and (logger := starlite_app.get_logge()):

no?

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps, but it broke my test 🙃 looking into why tonight hopefully

Copy link
Member

Choose a reason for hiding this comment

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

Maybe because the function isn’t called get_logge but get_logger? 😉

Copy link
Member Author

@JacobCoffee JacobCoffee Feb 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

i fixed that as soon as i committed the suggestion and realized 😛
commit dffccbf

Copy link
Contributor

Choose a reason for hiding this comment

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

If this change breaks the test, it most likely is because we are patching app.logger with response of calling the get_logger fixture in the test, whereas for this to work, we'd have to patch the app.get_logger attribute with the get_logger fixture itself.

In the starlite constructor, we both assign the get_logger callable and a logger instance to the application instance:

        if self.logging_config:
            self.get_logger = self.logging_config.configure()
            self.logger = self.get_logger("starlite")

...and it wasn't obvious to me which one we should be using in this case either.

logger.debug("Exception in ASGI application", exc_info=True)
for hook in starlite_app.after_exception:
await hook(e, scope, starlite_app.state)

Expand Down
37 changes: 36 additions & 1 deletion tests/middleware/test_exception_handler_middleware.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@

from starlette.exceptions import HTTPException as StarletteHTTPException

from starlite import HTTPException, Request, Response, Starlite, get
from starlite import HTTPException, LoggingConfig, Request, Response, Starlite, get
from starlite.middleware.exceptions import ExceptionHandlerMiddleware
from starlite.status_codes import HTTP_500_INTERNAL_SERVER_ERROR
from starlite.testing import create_test_client

if TYPE_CHECKING:
from _pytest.logging import LogCaptureFixture

from starlite.datastructures import State
from starlite.types import Scope

Expand Down Expand Up @@ -118,3 +120,36 @@ async def after_exception_hook_handler(exc: Exception, scope: "Scope", state: "S
response = client.get("/test")
assert response.status_code == HTTP_500_INTERNAL_SERVER_ERROR
assert client.app.state.called


def test_exception_handler_middleware_debug_logging(caplog: "LogCaptureFixture") -> None:
@get("/test")
def handler() -> None:
raise ValueError("Test debug exception")

# Debug On -> Exception is logged
with caplog.at_level("DEBUG"), create_test_client(handler, logging_config=LoggingConfig()) as client:
client.app.debug = True
response = client.get("/test")
assert response.status_code == HTTP_500_INTERNAL_SERVER_ERROR
assert "Internal Server Error" in caplog.text
assert "ValueError" in response.text
assert "Test debug exception" in response.text

# Debug Off -> Exception is not logged
with caplog.at_level("INFO"), create_test_client(handler, logging_config=LoggingConfig()) as client:
client.app.debug = False
response = client.get("/test")
assert response.status_code == HTTP_500_INTERNAL_SERVER_ERROR
assert "Exception in ASGI application" not in caplog.text
assert "ValueError" in response.text
assert "Test debug exception" in response.text

# Debug On + No Logger -> No Debug Logging
with caplog.at_level("DEBUG"), create_test_client(handler, logging_config=None) as client:
client.app.debug = True
response = client.get("/test")
assert response.status_code == HTTP_500_INTERNAL_SERVER_ERROR
assert "Internal Server Error" in caplog.text
assert "Test debug exception" in response.text
assert "ValueError" in response.text