fixes indentify errors

Author: germankay

ckanext/push_errors/logging.py

@@ -8,6 +8,7 @@
 from ckan.plugins import toolkit
 from ckanext.push_errors import __VERSION__ as push_errors_version
 from ckanext.push_errors.redis import get_cache
+from ckanext.push_errors.utils import get_error_log_id
 
 log = logging.getLogger(__name__)
 
@@ -63,7 +64,12 @@ def emit(self, record):
                 f'[{extras.get("name")}]::{extras.get("levelname")}::'
                 f'{extras.get("asctime")}'
             )
-            push_message(msg)
+
+            # Generate unique log ID
+            error_id = get_error_log_id(record)
+
+            # Send with additional context
+            push_message(msg, extra_context={"error_id": error_id})
 
 
 def push_message(message, extra_context={}):

ckanext/push_errors/plugin.py

@@ -7,6 +7,7 @@
 from ckanext.push_errors.logging import PushErrorHandler, push_message
 from ckanext.push_errors.cli import push_errors as push_errors_commands
 from ckanext.push_errors.error_logger import log_error
+from ckanext.push_errors.utils import get_error_trace_id
 
 from ckanext.push_errors.blueprints.push_errors import push_error_bp
 
@@ -59,16 +60,17 @@ def error_handler(exception):
             params = toolkit.request.args if toolkit.request else '-'
             path = toolkit.request.path if toolkit.request else '-'
             user = current_user.name if current_user else '-'
+            error_id = get_error_trace_id(exception)
 
             error_message = (
-                f'INTERNAL_ERROR `{exception_str}` \n\t'
-                f'TRACE\n```{trace}```\n\t'
-                f'on page {path}\n\t'
-                f'params: {params}\n\t'
+                f'INTERNAL_ERROR `{exception_str}`\n'
+                f'TRACE\n'
+                f'```{trace}```\n'
+                f'on page {path}\n'
+                f'params: {params}\n'
                 f'by user *{user}*'
             )
-            push_message(error_message)
-            # Continue to raise the error
+            push_message(error_message, extra_context={'error_id': error_id})
             raise exception
 
         app.register_error_handler(Exception, error_handler)
@@ -81,14 +83,8 @@ def make_error_log_middleware(self, app, config):
         # Prepare and add the PushErrorHandler
         push_error_handler = PushErrorHandler()
         push_error_handler.setLevel(logging.ERROR)
-
-        # Add to the ckan logger
-        ckan_log = logging.getLogger('ckan')
-        ckan_log.addHandler(push_error_handler)
-        # Add to the ckanext logger for all extensions
-        ckanext_log = logging.getLogger('ckanext')
-        ckanext_log.addHandler(push_error_handler)
-
+        logging.getLogger('ckan').addHandler(push_error_handler)
+        logging.getLogger('ckanext').addHandler(push_error_handler)
         return app
 
     # IClick

ckanext/push_errors/tests/test_logging.py

@@ -2,7 +2,7 @@
 import logging
 import pytest
 from datetime import datetime
-from unittest.mock import patch, ANY
+from unittest.mock import patch
 from ckanext.push_errors.logging import push_message, PushErrorHandler
 from ckanext.push_errors import __VERSION__ as push_errors_version
 
@@ -85,4 +85,7 @@ def test_critical_error_logging(self):
             push_error_handler = PushErrorHandler()
             log.addHandler(push_error_handler)
             log.critical("This is a critical error!")
-            mock_push_message.assert_called_once_with(ANY)
+            mock_push_message.assert_called_once()
+            args, kwargs = mock_push_message.call_args
+            assert "This is a critical error!" in args[0]
+            assert "error_id" in kwargs["extra_context"]

ckanext/push_errors/tests/test_push_errors_plugin.py

@@ -1,5 +1,5 @@
 import pytest
-from unittest.mock import patch, ANY, MagicMock
+from unittest.mock import patch, MagicMock
 from werkzeug.exceptions import InternalServerError, Unauthorized, Forbidden, NotFound
 from ckanext.push_errors.plugin import PushErrorsPlugin
 
@@ -55,7 +55,10 @@ def test_middleware_handles_multiple_exceptions(mock_push_message, exception):
     if isinstance(exception, (Unauthorized, Forbidden, NotFound)):
         mock_push_message.assert_not_called()
     else:
-        mock_push_message.assert_called_once_with(ANY)
+        mock_push_message.assert_called_once()
+        args, kwargs = mock_push_message.call_args
+        assert "Internal error" in args[0]
+        assert "error_id" in kwargs["extra_context"]
 
 
 @pytest.mark.ckan_config("ckanext.push_errors.traceback_length", "1000")

ckanext/push_errors/tests/test_utils.py

@@ -0,0 +1,91 @@
+import logging
+from ckanext.push_errors.utils import get_error_trace_id, get_error_log_id
+
+
+class CustomError(Exception):
+    pass
+
+
+def raise_custom_error():
+    """Raises a CustomError to simulate an error scenario for testing purposes."""
+    raise CustomError("Simulated error")
+
+
+def test_get_error_trace_id_returns_path_and_line():
+    """
+    Test that get_error_trace_id returns a string containing the file path and line number
+    where the CustomError was raised, formatted as 'path/to/file.py:<line>'.
+    Ensures the returned trace_id contains a colon and does not end with '.py'.
+    """
+    try:
+        raise_custom_error()
+    except CustomError as e:
+        trace_id = get_error_trace_id(e)
+        assert ":" in trace_id
+        assert not trace_id.endswith(".py")  # it only ends with ":<line>"
+
+
+def test_get_error_trace_id_unknown_when_no_tb():
+    """
+    Test that get_error_trace_id returns 'unknown' when the exception has no traceback.
+
+    This test creates a CustomError instance without a traceback and verifies that
+    the get_error_trace_id function returns the string 'unknown' as expected.
+    """
+    e = CustomError("No traceback")
+    e.__traceback__ = None
+    trace_id = get_error_trace_id(e)
+    assert trace_id == "unknown"
+
+
+def test_get_error_log_id_from_exc_info():
+    """
+    Tests the get_error_log_id function to ensure it generates an appropriate log identifier
+    from a logging record containing exception information.
+
+    - Creates a test logger.
+    - Raises and catches a custom exception.
+    - Generates a log record with the captured exception.
+    - Obtains the log identifier using get_error_log_id.
+    - Verifies that the identifier does not start with "loghash:" and contains ":".
+
+    This function checks that get_error_log_id correctly processes exception information
+    in a logging record and generates the expected identifier.
+    """
+    logger = logging.getLogger("testlogger")
+
+    try:
+        raise_custom_error()
+    except CustomError as e:
+        record = logger.makeRecord(
+            name="testlogger",
+            level=logging.CRITICAL,
+            fn="test_file.py",
+            lno=99,
+            msg="An error occurred",
+            args=(),
+            exc_info=(type(e), e, e.__traceback__),
+            func="test_func",
+            extra=None
+        )
+        log_id = get_error_log_id(record)
+        assert log_id.startswith("loghash:") is False
+        assert ":" in log_id
+
+
+def test_get_error_log_id_from_message_hash():
+    """
+    Test that get_error_log_id generates a unique log ID for a given LogRecord.
+
+    This test creates a LogRecord with a critical error message and verifies that
+    the generated log ID starts with the expected prefix ("loghash:") and has a
+    length greater than 12 characters, ensuring the uniqueness and format of the
+    log identifier.
+    """
+    record = logging.LogRecord(
+        name="test", level=logging.CRITICAL, pathname="", lineno=0,
+        msg="Some random critical error", args=(), exc_info=None
+    )
+    log_id = get_error_log_id(record)
+    assert log_id.startswith("loghash:")
+    assert len(log_id) > 12

ckanext/push_errors/utils.py

@@ -0,0 +1,30 @@
+import traceback
+import hashlib
+import logging
+
+
+def get_error_trace_id(error: Exception) -> str:
+    """
+    Generates a unique identifier for an exception using the file path and line number.
+    """
+    tb = traceback.extract_tb(error.__traceback__)
+    if tb:
+        file_path, line_number, _, _ = tb[-1]
+        return f"{file_path}:{line_number}"
+    return "unknown"
+
+
+def get_error_log_id(record: logging.LogRecord) -> str:
+    """
+    Generates a unique identifier for a log record. Uses traceback info if present,
+    otherwise hashes the message to produce a shorter ID.
+    """
+    if record.exc_info:
+        tb = traceback.extract_tb(record.exc_info[2])
+        if tb:
+            file_path, line_number, _, _ = tb[-1]
+            return f"{file_path}:{line_number}"
+    # Fallback to hash of the log message
+    message = record.getMessage()
+    msg_hash = hashlib.sha256(message.encode("utf-8")).hexdigest()
+    return f"loghash:{msg_hash[:12]}"