fix: Drop events when before_send callbacks raise (#880)

fix: drop events when before_send raises

Author: marandaneto

.sampo/changesets/stalwart-king-erika.md

@@ -0,0 +1,5 @@
+---
+pypi/posthog: patch
+---
+
+Drop events when before_send callbacks raise exceptions

posthog/client.py

@@ -2303,7 +2303,7 @@ def _enqueue(self, msg, disable_geoip, lane=None, property_allowlist=None):
                 msg = clean(modified_msg)
             except Exception as e:
                 self.log.exception(f"Error in before_send callback: {e}")
-                # Continue with the original message if callback fails
+                return None
 
         # Re-normalized after before_send, which may have replaced or removed
         # msg["uuid"], so the returned uuid always matches the wire event.

posthog/test/test_before_send.py

@@ -145,9 +145,11 @@ def drop_test_events(event):
             self.assertEqual(enqueued_msg["event"], "keep_me")
 
     def test_before_send_callback_handles_exceptions(self):
-        """Test that exceptions in before_send don't crash the client."""
+        """Test that exceptions in before_send drop the event without crashing."""
 
         def buggy_before_send(event):
+            event["properties"]["partially_mutated"] = True
+            event["uuid"] = "invalid"
             raise ValueError("Oops!")
 
         with mock.patch("posthog.client.batch_post") as mock_post:
@@ -157,16 +159,49 @@ def buggy_before_send(event):
                 before_send=buggy_before_send,
                 sync_mode=True,
             )
-            msg_uuid = client.capture("robust_event", distinct_id="user1")
+            with (
+                mock.patch.object(
+                    client,
+                    "_normalize_event_uuid",
+                    wraps=client._normalize_event_uuid,
+                ) as normalize_uuid,
+                self.assertLogs("posthog", level="ERROR") as logs,
+            ):
+                msg_uuid = client.capture("robust_event", distinct_id="user1")
 
-            # Event should still be sent despite the exception
-            self.assertIsNotNone(msg_uuid)
+            self.assertIsNone(msg_uuid)
+            mock_post.assert_not_called()
+            normalize_uuid.assert_called_once()
+            self.assertIn("Error in before_send callback: Oops!", logs.output[0])
 
-            # Check the enqueued message
-            mock_post.assert_called_once()
-            batch_data = mock_post.call_args[1]["batch"]
-            enqueued_msg = batch_data[0]
-            self.assertEqual(enqueued_msg["event"], "robust_event")
+    def test_before_send_callback_exception_does_not_enqueue_mutated_event(self):
+        def buggy_before_send(event):
+            event["properties"]["partially_mutated"] = True
+            raise ValueError("Oops!")
+
+        client = Client(
+            FAKE_TEST_API_KEY,
+            on_error=self.set_fail,
+            before_send=buggy_before_send,
+            flush_at=1,
+        )
+        try:
+            with (
+                mock.patch("posthog.consumer.batch_post") as mock_post,
+                mock.patch.object(
+                    client._analytics_lane,
+                    "enqueue",
+                    wraps=client._analytics_lane.enqueue,
+                ) as enqueue,
+                self.assertLogs("posthog", level="ERROR"),
+            ):
+                msg_uuid = client.capture("robust_event", distinct_id="user1")
+
+            self.assertIsNone(msg_uuid)
+            enqueue.assert_not_called()
+            mock_post.assert_not_called()
+        finally:
+            client.shutdown()
 
     def test_before_send_callback_output_is_recleaned(self):
         marker = object()
@@ -186,7 +221,7 @@ def add_unsupported_value(event):
         sent_event = mock_post.call_args.kwargs["batch"][0]
         self.assertIsNone(sent_event["properties"]["marker"])
 
-    def test_before_send_callback_non_dict_output_uses_original_event(self):
+    def test_before_send_callback_non_dict_output_drops_event(self):
         with (
             mock.patch("posthog.client.batch_post") as mock_post,
             mock.patch("posthog.client.Client.log.exception") as mock_log,
@@ -196,10 +231,9 @@ def test_before_send_callback_non_dict_output_uses_original_event(self):
                 before_send=lambda _event: "invalid",
                 sync_mode=True,
             )
-            self.assertIsNotNone(client.capture("original", distinct_id="user1"))
+            self.assertIsNone(client.capture("original", distinct_id="user1"))
 
-        sent_event = mock_post.call_args.kwargs["batch"][0]
-        self.assertEqual(sent_event["event"], "original")
+        mock_post.assert_not_called()
         self.assertIn(
             "before_send must return a dict or None", mock_log.call_args.args[0]
         )