Merge pull request #90 from digital-land/1798-jsonpath-bug-sentry-logging-error-detail

1798 jsonpath bug sentry logging error detail

Author: pooleycodes

request-api/src/crud.py

@@ -1,10 +1,14 @@
+import logging
 from sqlalchemy import func
 from sqlalchemy.orm import Session
+from sqlalchemy.exc import ProgrammingError, DataError
 
 from pagination_model import PaginatedResult, PaginationParams
 from request_model import models
 from request_model import schemas
 
+logger = logging.getLogger(__name__)
+
 
 def get_request(db: Session, request_id: int):
     return db.query(models.Request).filter(models.Request.id == request_id).first()
@@ -26,12 +30,22 @@ def get_response_details(
             func.jsonb_path_match(models.ResponseDetails.detail, jsonpath)
         )
 
-    response_details = (
-        base_query.offset(pagination_params.offset).limit(pagination_params.limit).all()
-    )
+    try:
+        response_details = (
+            base_query.offset(pagination_params.offset)
+            .limit(pagination_params.limit)
+            .all()
+        )
+        total_results = base_query.count()
+    except (ProgrammingError, DataError) as e:
+        # jsonb_path_math can raise errors if the jsonpath is invalid
+        logger.warning("Invalid JSONPath expression '%s': %s", jsonpath, str(e))
+        response_details = []
+        total_results = 0
+
     return PaginatedResult(
         params=pagination_params,
-        total_results_available=base_query.count(),
+        total_results_available=total_results,
         data=response_details,
     )
 

request-api/tests/unit/test_crud.py

@@ -0,0 +1,24 @@
+from unittest.mock import MagicMock
+
+from sqlalchemy.exc import ProgrammingError
+
+import crud
+
+
+def test_get_response_details_invalid_jsonpath_returns_empty(monkeypatch):
+    base_query = MagicMock()
+    base_query.join.return_value = base_query
+    base_query.filter.return_value = base_query
+    base_query.offset.return_value = base_query
+    base_query.limit.return_value = base_query
+    base_query.all.side_effect = ProgrammingError("bad jsonpath", None, None)
+    base_query.count = MagicMock()
+
+    db = MagicMock()
+    db.query.return_value = base_query
+
+    result = crud.get_response_details(db, request_id=1, jsonpath=";")
+
+    assert result.data == []
+    assert result.total_results_available == 0
+    base_query.count.assert_not_called()

request-processor/src/application/core/utils.py

@@ -240,51 +240,28 @@ def append_source(
 
 def create_user_friendly_error_log(exception_detail):
     """
-    Creates a user-friendly error log from CustomException detail, as the message will be displayed to end users.
+    Creates a user-friendly error message from CustomException detail, as the message may be displayed to end users.
     Keeps all info from exception_detail and adds user-friendly message.
     """
     status_code = exception_detail.get("errCode")
     exception_type = exception_detail.get("exceptionType")
     content_type = exception_detail.get("contentType")
 
     user_message = "An error occurred, please try again later."
-    user_message_detail = None
 
     if exception_type in ["SSLError", "SSLCertVerificationError"]:
         user_message = "SSL certificate verification failed"
-        user_message_detail = [
-            (
-                "We couldn't verify the SSL certificate for that link. "
-                "While it may open in some browsers, our security checks "
-                "require a complete certificate chain."
-            ),
-            (
-                "Please make sure the site is served over HTTPS and its "
-                "SSL certificate is correctly installed."
-            ),
-        ]
     elif content_type and "text/html" in content_type:
         user_message = (
             "The selected file must be a CSV, GeoJSON, GML or GeoPackage file"
         )
-        user_message_detail = (
-            "The URL returned a webpage (HTML) instead of a data file. "
-            "Please provide a direct link to the data file "
-            "(for example: .csv, .geojson, .gml or .gpkg)."
-        )
     elif status_code == "403":
         user_message = "The URL must be accessible"
-        user_message_detail = [
-            "You must host the URL on a server which does not block access due to set permissions.",
-            "Contact your IT team for support if you need it, referencing a 'HTTP status code 403' error.",
-        ]
     elif status_code == "404":
         user_message = "The URL does not exist. Check the URL you've entered is correct (HTTP 404 error)"
 
     result = dict(exception_detail)
     result["message"] = user_message
-    if user_message_detail is not None:
-        result["user_message_detail"] = user_message_detail
 
     return result
 

request-processor/src/tasks.py

@@ -173,7 +173,6 @@ def check_dataurl(request: Dict, directories=None):
         logger.info(f"Fetched resource: file_name={file_name}")
 
     except CustomException as e:
-        logger.error(f"URL FETCH Error during _fetch_resource: {e}")
         # Track in Sentry for monitoring (not as error)
         _capture_sentry_event(
             e.detail,
@@ -227,7 +226,7 @@ def check_dataurl(request: Dict, directories=None):
             )
             save_response_to_db(request_schema.id, error_log)
     else:
-        logger.error("File could not be fetched from collector")
+        logger.info("File could not be fetched from collector")
         error_log = create_generic_error_log(
             url=request_data.url,
             exception="UnknownCollectionError",