Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 24 additions & 10 deletions litestar/logging/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,20 +120,34 @@ def _default_exception_logging_handler_factory(is_struct_logger: bool) -> Except
if is_struct_logger:

def _default_exception_logging_handler(logger: Logger, scope: Scope, tb: list[str]) -> None:
logger.exception(
"Uncaught exception",
connection_type=scope["type"],
path=scope["path"],
)
if tb:
logger.exception(
"Uncaught exception",
connection_type=scope["type"],
path=scope["path"],
)
else:
logger.error(
"Uncaught exception",
connection_type=scope["type"],
path=scope["path"],
)

else:

def _default_exception_logging_handler(logger: Logger, scope: Scope, tb: list[str]) -> None:
logger.exception(
"Uncaught exception (connection_type=%s, path=%r):",
scope["type"],
scope["path"],
)
if tb:
logger.exception(
"Uncaught exception (connection_type=%s, path=%r):",
scope["type"],
scope["path"],
)
else:
logger.error(
"Uncaught exception (connection_type=%s, path=%r):",
scope["type"],
scope["path"],
)

return _default_exception_logging_handler

Expand Down
18 changes: 9 additions & 9 deletions litestar/middleware/_internal/exceptions/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,12 @@ def handle_exception_logging(self, logger: Logger, logging_config: BaseLoggingCo
exc = exc_info()
exc_detail: set[Union[Exception, int]] = {exc[0], getattr(exc[0], "status_code", None)} # type: ignore[arg-type] # noqa: UP007

if (
(
logging_config.log_exceptions == "always"
or (logging_config.log_exceptions == "debug" and self._get_debug_scope(scope))
)
and logging_config.exception_logging_handler
and exc_detail.isdisjoint(logging_config.disable_stack_trace)
):
logging_config.exception_logging_handler(logger, scope, format_exception(*exc))
handler = logging_config.exception_logging_handler
should_log = logging_config.log_exceptions == "always" or (
logging_config.log_exceptions == "debug" and self._get_debug_scope(scope)
)

if should_log and handler is not None:
include_stack_trace = exc_detail.isdisjoint(logging_config.disable_stack_trace)
tb = format_exception(*exc) if include_stack_trace else []
handler(logger, scope, tb)
40 changes: 33 additions & 7 deletions tests/unit/test_logging/test_logging_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from litestar.exceptions import HTTPException, ImproperlyConfiguredException, NotFoundException
from litestar.logging.config import (
LoggingConfig,
_default_exception_logging_handler_factory,
_get_default_handlers,
_get_default_logging_module,
default_handlers,
Expand Down Expand Up @@ -535,15 +536,15 @@ def test_excluded_fields(logging_module: str) -> None:


@pytest.mark.parametrize(
"disable_stack_trace, exception_to_raise, handler_called",
"disable_stack_trace, exception_to_raise, expect_tb",
[
# will log the stack trace
# will log WITH the stack trace (tb is non-empty)
[set(), HTTPException, True],
[set(), ValueError, True],
[{400}, HTTPException, True],
[{NameError}, ValueError, True],
[{400, NameError}, ValueError, True],
# will not log the stack trace
# will log WITHOUT the stack trace (tb is empty)
[{NotFoundException}, HTTPException, False],
[{404}, HTTPException, False],
[{ValueError}, ValueError, False],
Expand All @@ -554,7 +555,7 @@ def test_excluded_fields(logging_module: str) -> None:
def test_disable_stack_trace(
disable_stack_trace: set[Union[int, type[Exception]]],
exception_to_raise: type[Exception],
handler_called: bool,
expect_tb: bool,
) -> None:
mock_handler = MagicMock()

Expand All @@ -570,7 +571,32 @@ async def error_route() -> None:
else:
_ = client.get("/error")

if handler_called:
assert mock_handler.called, "Exception logging handler should have been called"
assert mock_handler.called, "Exception logging handler should always be called"
tb = mock_handler.call_args[0][2]
if expect_tb:
assert len(tb) > 0, "Stack trace should be present"
else:
assert not mock_handler.called, "Exception logging handler should not have been called"
assert tb == [], "Stack trace should be suppressed but handler should still be called"


def test_default_handler_uses_error_when_stack_trace_suppressed() -> None:
"""The default exception logging handler should call ``logger.error``
(not ``logger.exception``) when the stack trace is suppressed via
``disable_stack_trace``. This exercises the ``else`` branch inside
``_default_exception_logging_handler_factory(is_struct_logger=False)``.
"""
handler = _default_exception_logging_handler_factory(is_struct_logger=False)
mock_logger = MagicMock()
scope: Any = {"type": "http", "path": "/error"}

# With traceback present -> logger.exception
handler(mock_logger, scope, ["Traceback ..."])
mock_logger.exception.assert_called_once()
mock_logger.error.assert_not_called()

mock_logger.reset_mock()

# With empty traceback (stack trace suppressed) -> logger.error
handler(mock_logger, scope, [])
mock_logger.error.assert_called_once()
mock_logger.exception.assert_not_called()
49 changes: 40 additions & 9 deletions tests/unit/test_logging/test_structlog_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import datetime
import sys
from typing import Callable, Union
from typing import Any, Callable, Union
from unittest.mock import MagicMock, patch

import pytest
Expand All @@ -14,7 +14,13 @@

from litestar import get
from litestar.exceptions import HTTPException, NotFoundException
from litestar.logging.config import LoggingConfig, StructlogEventFilter, StructLoggingConfig, default_json_serializer
from litestar.logging.config import (
LoggingConfig,
StructlogEventFilter,
StructLoggingConfig,
_default_exception_logging_handler_factory,
default_json_serializer,
)
from litestar.plugins.structlog import StructlogConfig, StructlogPlugin
from litestar.serialization import decode_json
from litestar.testing import create_test_client
Expand Down Expand Up @@ -180,15 +186,15 @@ def test_structlog_config_as_json(isatty: bool, pretty_print_tty: bool, expected


@pytest.mark.parametrize(
"disable_stack_trace, exception_to_raise, handler_called",
"disable_stack_trace, exception_to_raise, expect_tb",
[
# will log the stack trace
# will log WITH the stack trace (tb is non-empty)
[set(), HTTPException, True],
[set(), ValueError, True],
[{400}, HTTPException, True],
[{NameError}, ValueError, True],
[{400, NameError}, ValueError, True],
# will not log the stack trace
# will log WITHOUT the stack trace (tb is empty)
[{NotFoundException}, HTTPException, False],
[{404}, HTTPException, False],
[{ValueError}, ValueError, False],
Expand All @@ -199,7 +205,7 @@ def test_structlog_config_as_json(isatty: bool, pretty_print_tty: bool, expected
def test_structlog_disable_stack_trace(
disable_stack_trace: set[Union[int, type[Exception]]],
exception_to_raise: type[Exception],
handler_called: bool,
expect_tb: bool,
) -> None:
mock_handler = MagicMock()

Expand All @@ -217,7 +223,32 @@ async def error_route() -> None:
else:
_ = client.get("/error")

if handler_called:
assert mock_handler.called, "Structlog exception handler should have been called"
assert mock_handler.called, "Structlog exception handler should always be called"
tb = mock_handler.call_args[0][2]
if expect_tb:
assert len(tb) > 0, "Stack trace should be present"
else:
assert not mock_handler.called, "Structlog exception handler should not have been called"
assert tb == [], "Stack trace should be suppressed but handler should still be called"


def test_structlog_default_handler_uses_error_when_stack_trace_suppressed() -> None:
"""The default structlog exception logging handler should call
``logger.error`` (not ``logger.exception``) when the stack trace is
suppressed via ``disable_stack_trace``. This exercises the ``else``
branch inside ``_default_exception_logging_handler_factory(is_struct_logger=True)``.
"""
handler = _default_exception_logging_handler_factory(is_struct_logger=True)
mock_logger = MagicMock()
scope: Any = {"type": "http", "path": "/error"}

# With traceback present -> logger.exception
handler(mock_logger, scope, ["Traceback ..."])
mock_logger.exception.assert_called_once()
mock_logger.error.assert_not_called()

mock_logger.reset_mock()

# With empty traceback (stack trace suppressed) -> logger.error
handler(mock_logger, scope, [])
mock_logger.error.assert_called_once()
mock_logger.exception.assert_not_called()
Loading