From 3877fe017291a2cff7cb9b7bab99b6981293a4ed Mon Sep 17 00:00:00 2001
From: Aliaksandr Kuzmik <98702584+alexkuzmik@users.noreply.github.com>
Date: Thu, 9 Nov 2023 14:46:26 +0100
Subject: [PATCH] [NA] Filter log user feedback errors (#94)

* Add error filtering to log_user_feedback, add option to use filter without a summary

* Fix lint errors
---
 src/comet_llm/api.py                           |  3 ++-
 src/comet_llm/exceptions/filter_decorator.py   |  9 ++++++---
 tests/unit/exceptions/test_filter_decorator.py | 15 +++++++++++++++
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/comet_llm/api.py b/src/comet_llm/api.py
index 1f59e07f49..1f98eed5a0 100644
--- a/src/comet_llm/api.py
+++ b/src/comet_llm/api.py
@@ -15,12 +15,13 @@
 import logging
 from typing import Optional
 
-from . import experiment_info, logging_messages
+from . import config, exceptions, experiment_info, logging_messages
 from .experiment_api import ExperimentAPI
 
 LOGGER = logging.getLogger(__name__)
 
 
+@exceptions.filter(allow_raising=config.raising_enabled())
 def log_user_feedback(id: str, score: float, api_key: Optional[str] = None) -> None:
     """
     Logs user feedback for the provided Prompt or Chain ID. This will
diff --git a/src/comet_llm/exceptions/filter_decorator.py b/src/comet_llm/exceptions/filter_decorator.py
index 8a9c989701..d62baa862a 100644
--- a/src/comet_llm/exceptions/filter_decorator.py
+++ b/src/comet_llm/exceptions/filter_decorator.py
@@ -14,7 +14,7 @@
 
 import functools
 import logging
-from typing import TYPE_CHECKING, Any, Callable
+from typing import TYPE_CHECKING, Any, Callable, Optional
 
 from comet_llm import logging as comet_logging
 
@@ -24,14 +24,17 @@
 LOGGER = logging.getLogger(__name__)
 
 
-def filter(allow_raising: bool, summary: "summary.Summary") -> Callable:
+def filter(
+    allow_raising: bool, summary: Optional["summary.Summary"] = None
+) -> Callable:
     def decorator(function: Callable) -> Callable:
         @functools.wraps(function)
         def wrapper(*args, **kwargs) -> Any:  # type: ignore
             try:
                 return function(*args, **kwargs)
             except Exception as exception:
-                summary.increment_failed()
+                if summary is not None:
+                    summary.increment_failed()
 
                 if allow_raising:
                     raise
diff --git a/tests/unit/exceptions/test_filter_decorator.py b/tests/unit/exceptions/test_filter_decorator.py
index 457f8f35d9..e9bef03f98 100644
--- a/tests/unit/exceptions/test_filter_decorator.py
+++ b/tests/unit/exceptions/test_filter_decorator.py
@@ -61,3 +61,18 @@ def f():
             extra={"show_traceback": True}
         )
         assert f() is None
+
+
+def test_filter__upraising_not_allowed__summary_not_passed_to_filter__function_raised_exception__nothing_done_with_summary():
+    @filter_decorator.filter(allow_raising=False)
+    def f():
+        raise exceptions.CometLLMException("some-message", log_message_once=True)
+    with Scenario() as s:
+        s.comet_logging.log_once_at_level(
+            filter_decorator.LOGGER,
+            logging.ERROR,
+            "some-message",
+            exc_info=True,
+            extra={"show_traceback": True}
+        )
+        assert f() is None