From 4755b53dfaa8ad749b89ada99c5bc04e17a307b3 Mon Sep 17 00:00:00 2001
From: Andreas Motl <andreas.motl@crate.io>
Date: Mon, 30 Sep 2024 20:56:51 +0200
Subject: [PATCH] Zyp/Moksha: Improve error reporting when rule evaluation
 fails

---
 CHANGES.md                     |  1 +
 src/zyp/model/moksha.py        | 16 ++++++++++------
 tests/zyp/moksha/test_model.py | 10 ++++++++--
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index cd9e2a6..2397094 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -4,6 +4,7 @@
 - DynamoDB: Change CrateDB data model to use (`pk`, `data`, `aux`) columns
   Attention: This is a breaking change.
 - MongoDB: Handle too large `$date.$numberLong` values gracefully
+- Zyp/Moksha: Improve error reporting when rule evaluation fails
 
 ## 2024/09/26 v0.0.19
 - DynamoDB CDC: Fix `MODIFY` operation by propagating `NewImage` fully
diff --git a/src/zyp/model/moksha.py b/src/zyp/model/moksha.py
index 6b6dd07..f901274 100644
--- a/src/zyp/model/moksha.py
+++ b/src/zyp/model/moksha.py
@@ -22,11 +22,14 @@ class MokshaRule:
     disabled: t.Optional[bool] = False
 
     def compile(self):
-        return MokshaRuntimeRule(self.type, compile_expression(self.type, self.expression), disabled=self.disabled)
+        return MokshaRuntimeRule(
+            self, self.type, compile_expression(self.type, self.expression), disabled=self.disabled
+        )
 
 
 @define
 class MokshaRuntimeRule:
+    source: MokshaRule
     type: str
     transformer: MokshaTransformer
     disabled: t.Optional[bool] = False
@@ -76,10 +79,11 @@ def apply(self, data: t.Any) -> t.Any:
         for rule in self._runtime_rules:
             try:
                 data = rule.evaluate(data)
-            except Exception:
-                logger.exception(f"Error evaluating rule: {rule}")
-                if isinstance(data, map):
-                    data = list(data)
-                logger.debug(f"Error payload:\n{data}")
+            except Exception as ex:
+                logger.error(f"Error evaluating rule: {ex}. Expression: {rule.source.expression}")
+                if logger.level is logging.DEBUG:
+                    if isinstance(data, map):
+                        data = list(data)
+                    logger.debug(f"Error payload:\n{data}")
                 raise
         return data
diff --git a/tests/zyp/moksha/test_model.py b/tests/zyp/moksha/test_model.py
index 44370d6..94c3267 100644
--- a/tests/zyp/moksha/test_model.py
+++ b/tests/zyp/moksha/test_model.py
@@ -1,3 +1,4 @@
+import logging
 import re
 
 import pytest
@@ -50,22 +51,27 @@ def test_moksha_transformation_success_jq():
 
 
 def test_moksha_transformation_error_jq_scalar(caplog):
+    logging.getLogger("zyp.model.moksha").setLevel(logging.DEBUG)
     moksha = MokshaTransformation().jq(". /= 100")
     with pytest.raises(ValueError) as ex:
         moksha.apply("foo")
     assert ex.match(re.escape('string ("foo") and number (100) cannot be divided'))
 
-    assert "Error evaluating rule: MokshaRuntimeRule(type='jq'" in caplog.text
+    assert (
+        'Error evaluating rule: string ("foo") and number (100) cannot be divided. Expression: . /= 100'
+        in caplog.messages
+    )
     assert "Error payload:\nfoo" in caplog.messages
 
 
 def test_moksha_transformation_error_jq_map(caplog):
+    logging.getLogger("zyp.model.moksha").setLevel(logging.DEBUG)
     moksha = MokshaTransformation().jq(".foo")
     with pytest.raises(ValueError) as ex:
         moksha.apply(map(lambda x: x, ["foo"]))  # noqa: C417
     assert ex.match(re.escape('Cannot index array with string "foo"'))
 
-    assert "Error evaluating rule: MokshaRuntimeRule(type='jq'" in caplog.text
+    assert 'Error evaluating rule: Cannot index array with string "foo". Expression: .foo' in caplog.messages
     assert "Error payload:\n[]" in caplog.messages