fix: Prevent core Client methods from raising exceptions (#327)

* fix: Prevent core Client methods from raising exceptions

The goal is to ensure that our client doesn't cause a panic in an
end-user application. This change updates
capture/set/set_once/group_identify/alias to swallow and log any
exceptions that occur. Note that this won't prevent errors from
propagating via the `on_error` callback if an error occurs while
processing the queue.

* test: Remove assertions that capture raises

These tests were broken anyways. Capture would only raise because it was
being called with no arguments, not because api_key or host are None.

Author: dustinbyrne

posthog/client.py

@@ -3,7 +3,7 @@
 import os
 import sys
 from datetime import datetime, timedelta
-from typing import Any, Dict, Optional, Union
+from typing import Any, Callable, Dict, Optional, Union
 from typing_extensions import Unpack
 from uuid import uuid4
 
@@ -99,6 +99,34 @@ def add_context_tags(properties):
     return properties
 
 
+def no_throw(default_return=None):
+    """
+    Decorator to prevent raising exceptions from public API methods.
+    Note that this doesn't prevent errors from propagating via `on_error`.
+    Exceptions will still be raised if the debug flag is enabled.
+
+    Args:
+        default_return: Value to return on exception (default: None)
+    """
+
+    def decorator(func):
+        from functools import wraps
+
+        @wraps(func)
+        def wrapper(self, *args, **kwargs):
+            try:
+                return func(self, *args, **kwargs)
+            except Exception as e:
+                if self.debug:
+                    raise e
+                self.log.exception(f"Error in {func.__name__}: {e}")
+                return default_return
+
+        return wrapper
+
+    return decorator
+
+
 class Client(object):
     """
     This is the SDK reference for the PostHog Python SDK.
@@ -481,6 +509,7 @@ def get_flags_decision(
 
         return normalize_flags_response(resp_data)
 
+    @no_throw()
     def capture(
         self, event: str, **kwargs: Unpack[OptionalCaptureArgs]
     ) -> Optional[str]:
@@ -657,6 +686,7 @@ def _parse_send_feature_flags(self, send_feature_flags) -> SendFeatureFlagsOptio
                 f"Expected bool or dict."
             )
 
+    @no_throw()
     def set(self, **kwargs: Unpack[OptionalSetArgs]) -> Optional[str]:
         """
         Set properties on a person profile.
@@ -690,6 +720,8 @@ def set(self, **kwargs: Unpack[OptionalSetArgs]) -> Optional[str]:
 
         Category:
             Identification
+
+        Note: This method will not raise exceptions. Errors are logged.
         """
         distinct_id = kwargs.get("distinct_id", None)
         properties = kwargs.get("properties", None)
@@ -716,6 +748,7 @@ def set(self, **kwargs: Unpack[OptionalSetArgs]) -> Optional[str]:
 
         return self._enqueue(msg, disable_geoip)
 
+    @no_throw()
     def set_once(self, **kwargs: Unpack[OptionalSetArgs]) -> Optional[str]:
         """
         Set properties on a person profile only if they haven't been set before.
@@ -734,6 +767,8 @@ def set_once(self, **kwargs: Unpack[OptionalSetArgs]) -> Optional[str]:
 
         Category:
             Identification
+
+        Note: This method will not raise exceptions. Errors are logged.
         """
         distinct_id = kwargs.get("distinct_id", None)
         properties = kwargs.get("properties", None)
@@ -759,6 +794,7 @@ def set_once(self, **kwargs: Unpack[OptionalSetArgs]) -> Optional[str]:
 
         return self._enqueue(msg, disable_geoip)
 
+    @no_throw()
     def group_identify(
         self,
         group_type: str,
@@ -791,6 +827,8 @@ def group_identify(
 
         Category:
             Identification
+
+        Note: This method will not raise exceptions. Errors are logged.
         """
         properties = properties or {}
 
@@ -815,6 +853,7 @@ def group_identify(
 
         return self._enqueue(msg, disable_geoip)
 
+    @no_throw()
     def alias(
         self,
         previous_id: str,
@@ -840,6 +879,8 @@ def alias(
 
         Category:
             Identification
+
+        Note: This method will not raise exceptions. Errors are logged.
         """
         (distinct_id, personless) = get_identity_state(distinct_id)
 

posthog/test/test_client.py

@@ -2423,3 +2423,46 @@ def test_context_tags_added(self):
             batch_data = mock_post.call_args[1]["batch"]
             msg = batch_data[0]
             self.assertEqual(msg["properties"]["$context_tags"], ["random_tag"])
+
+    @mock.patch(
+        "posthog.client.Client._enqueue", side_effect=Exception("Unexpected error")
+    )
+    def test_methods_handle_exceptions(self, mock_enqueue):
+        """Test that all decorated methods handle exceptions gracefully."""
+        client = Client("test-key")
+
+        test_cases = [
+            ("capture", ["test_event"], {}),
+            ("set", [], {"distinct_id": "some-id", "properties": {"a": "b"}}),
+            ("set_once", [], {"distinct_id": "some-id", "properties": {"a": "b"}}),
+            ("group_identify", ["group-type", "group-key"], {}),
+            ("alias", ["some-id", "new-id"], {}),
+        ]
+
+        for method_name, args, kwargs in test_cases:
+            with self.subTest(method=method_name):
+                method = getattr(client, method_name)
+                result = method(*args, **kwargs)
+                self.assertEqual(result, None)
+
+    @mock.patch(
+        "posthog.client.Client._enqueue", side_effect=Exception("Expected error")
+    )
+    def test_debug_flag_re_raises_exceptions(self, mock_enqueue):
+        """Test that methods re-raise exceptions when debug=True."""
+        client = Client("test-key", debug=True)
+
+        test_cases = [
+            ("capture", ["test_event"], {}),
+            ("set", [], {"distinct_id": "some-id", "properties": {"a": "b"}}),
+            ("set_once", [], {"distinct_id": "some-id", "properties": {"a": "b"}}),
+            ("group_identify", ["group-type", "group-key"], {}),
+            ("alias", ["some-id", "new-id"], {}),
+        ]
+
+        for method_name, args, kwargs in test_cases:
+            with self.subTest(method=method_name):
+                method = getattr(client, method_name)
+                with self.assertRaises(Exception) as cm:
+                    method(*args, **kwargs)
+                self.assertEqual(str(cm.exception), "Expected error")

posthog/test/test_module.py

@@ -18,14 +18,6 @@ def setUp(self):
             "testsecret", host="http://localhost:8000", on_error=self.failed
         )
 
-    def test_no_api_key(self):
-        self.posthog.api_key = None
-        self.assertRaises(Exception, self.posthog.capture)
-
-    def test_no_host(self):
-        self.posthog.host = None
-        self.assertRaises(Exception, self.posthog.capture)
-
     def test_track(self):
         res = self.posthog.capture("python module event", distinct_id="distinct_id")
         self._assert_enqueue_result(res)