From 16a4d821a3b231fc3b8ba84021f8b98159768e86 Mon Sep 17 00:00:00 2001 From: Tom Softreck Date: Sat, 19 Sep 2026 16:23:59 +0200 Subject: [PATCH 1/2] refactor(logging): extract shared error-context handling from error and critical (PLF-037) Co-authored-by: Koru Agent --- src/prefact/logging/logger.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/prefact/logging/logger.py b/src/prefact/logging/logger.py index e05d747..e322be4 100644 --- a/src/prefact/logging/logger.py +++ b/src/prefact/logging/logger.py @@ -57,18 +57,15 @@ def warning(self, message: str, **kwargs) -> None: self._log(LogLevel.WARNING, message, **kwargs) def error(self, message: str, error: Optional[Exception] = None, **kwargs) -> None: - if error: - kwargs.update( - { - "error_type": type(error).__name__, - "error_message": str(error), - "traceback": traceback.format_exc(), - } - ) - self._log(LogLevel.ERROR, message, **kwargs) + self._log_with_error_context(LogLevel.ERROR, message, error, **kwargs) def critical( self, message: str, error: Optional[Exception] = None, **kwargs + ) -> None: + self._log_with_error_context(LogLevel.CRITICAL, message, error, **kwargs) + + def _log_with_error_context( + self, level: LogLevel, message: str, error: Optional[Exception], **kwargs ) -> None: if error: kwargs.update( @@ -78,7 +75,7 @@ def critical( "traceback": traceback.format_exc(), } ) - self._log(LogLevel.CRITICAL, message, **kwargs) + self._log(level, message, **kwargs) def _log(self, level: LogLevel, message: str, **kwargs) -> None: log_record = { From 921d87bdea8354dd2f6550406cc1c99b71b482d2 Mon Sep 17 00:00:00 2001 From: Tom Softreck Date: Sat, 19 Sep 2026 17:05:17 +0200 Subject: [PATCH 2/2] test(logging): cover shared error-context handling in error and critical (PLF-037) Co-authored-by: Koru Agent --- src/prefact/logging/logger.py | 2 +- tests/test_logging.py | 76 +++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 tests/test_logging.py diff --git a/src/prefact/logging/logger.py b/src/prefact/logging/logger.py index e322be4..c296061 100644 --- a/src/prefact/logging/logger.py +++ b/src/prefact/logging/logger.py @@ -65,7 +65,7 @@ def critical( self._log_with_error_context(LogLevel.CRITICAL, message, error, **kwargs) def _log_with_error_context( - self, level: LogLevel, message: str, error: Optional[Exception], **kwargs + self, level: LogLevel, message: str, error: Exception | None, **kwargs ) -> None: if error: kwargs.update( diff --git a/tests/test_logging.py b/tests/test_logging.py new file mode 100644 index 0000000..382c142 --- /dev/null +++ b/tests/test_logging.py @@ -0,0 +1,76 @@ +"""Tests for PprefactLogger error-context handling (PLF-037).""" + +import pytest + +from prefact.logging import PprefactLogger +from prefact.logging.levels import LogLevel + + +@pytest.fixture +def records(): + captured = [] + + logger = PprefactLogger(name="prefact-test-error-context", enable_telemetry=True) + logger.add_telemetry_callback(captured.append) + logger.logger.handlers.clear() + + class _Capture: + def __init__(self): + self.captured = captured + self.logger = logger + + return _Capture() + + +def test_error_with_exception_adds_error_context(records): + logger = records.logger + try: + raise ValueError("boom") + except ValueError as exc: + logger.error("scan failed", error=exc) + + assert len(records.captured) == 1 + record = records.captured[0] + assert record["level"] == LogLevel.ERROR.value + assert record["message"] == "scan failed" + assert record["error_type"] == "ValueError" + assert record["error_message"] == "boom" + assert "ValueError" in record["traceback"] + assert "boom" in record["traceback"] + + +def test_critical_with_exception_adds_error_context(records): + logger = records.logger + try: + raise RuntimeError("fatal") + except RuntimeError as exc: + logger.critical("engine crashed", error=exc) + + assert len(records.captured) == 1 + record = records.captured[0] + assert record["level"] == LogLevel.CRITICAL.value + assert record["error_type"] == "RuntimeError" + assert record["error_message"] == "fatal" + assert "RuntimeError" in record["traceback"] + + +def test_error_without_exception_has_no_error_context(records): + records.logger.error("plain failure") + record = records.captured[0] + assert record["level"] == LogLevel.ERROR.value + assert "error_type" not in record + assert "error_message" not in record + assert "traceback" not in record + + +def test_error_context_preserves_extra_kwargs(records): + logger = records.logger + try: + raise OSError("missing") + except OSError as exc: + logger.error("io failure", error=exc, file_path="x.py", rule_id="R1") + + record = records.captured[0] + assert record["file_path"] == "x.py" + assert record["rule_id"] == "R1" + assert record["error_type"] == "OSError"