fix(logger): no longer replaces object duplicates with "CIRCULAR"

Author: CorieW

src/firebase_functions/logger.py

@@ -75,21 +75,29 @@ def _remove_circular(obj: _typing.Any,
     if refs is None:
         refs = set()
 
+    # Check if the object is already in the current recursion stack
     if id(obj) in refs:
         return "[CIRCULAR]"
 
+    # For non-primitive objects, add the current object's id to the recursion stack
     if not isinstance(obj, (str, int, float, bool, type(None))):
         refs.add(id(obj))
 
+    # Recursively process the object based on its type
     if isinstance(obj, dict):
-        return {key: _remove_circular(value, refs) for key, value in obj.items()}
+        result = {key: _remove_circular(value, refs) for key, value in obj.items()}
     elif isinstance(obj, list):
-        return [_remove_circular(value, refs) for _, value in enumerate(obj)]
+        result = [_remove_circular(item, refs) for item in obj]
     elif isinstance(obj, tuple):
-        return tuple(
-            _remove_circular(value, refs) for _, value in enumerate(obj))
+        result = tuple(_remove_circular(item, refs) for item in obj)
     else:
-        return obj
+        result = obj
+
+    # Remove the object's id from the recursion stack after processing
+    if not isinstance(obj, (str, int, float, bool, type(None))):
+        refs.remove(id(obj))
+
+    return result
 
 
 def _get_write_file(severity: LogSeverity) -> _typing.TextIO:

tests/test_logger.py

@@ -79,3 +79,97 @@ def test_message_should_be_space_separated(
         raw_log_output = capsys.readouterr().out
         log_output = json.loads(raw_log_output)
         assert log_output["message"] == expected_message
+
+    def test_remove_circular_references(self, capsys: pytest.CaptureFixture[str]):
+        # Create an object with a circular reference.
+        circ = {"b": "foo"}
+        circ["circ"] = circ
+
+        entry = {
+            "severity": "ERROR",
+            "message": "testing circular",
+            "circ": circ,
+        }
+        logger.write(entry)
+        raw_log_output = capsys.readouterr().err
+        log_output = json.loads(raw_log_output)
+
+        expected = {
+            "severity": "ERROR",
+            "message": "testing circular",
+            "circ": {"b": "foo", "circ": "[CIRCULAR]"},
+        }
+        assert log_output == expected
+
+    def test_remove_circular_references_in_arrays(self, capsys: pytest.CaptureFixture[str]):
+        # Create an object with a circular reference inside an array.
+        circ = {"b": "foo"}
+        circ["circ"] = [circ]
+
+        entry = {
+            "severity": "ERROR",
+            "message": "testing circular",
+            "circ": circ,
+        }
+        logger.write(entry)
+        raw_log_output = capsys.readouterr().err
+        log_output = json.loads(raw_log_output)
+
+        expected = {
+            "severity": "ERROR",
+            "message": "testing circular",
+            "circ": {"b": "foo", "circ": ["[CIRCULAR]"]},
+        }
+        assert log_output == expected
+
+    def test_no_false_circular_for_duplicates(self, capsys: pytest.CaptureFixture[str]):
+        # Ensure that duplicate objects (used in multiple keys) are not marked as circular.
+        obj = {"a": "foo"}
+        entry = {
+            "severity": "ERROR",
+            "message": "testing circular",
+            "a": obj,
+            "b": obj,
+        }
+        logger.write(entry)
+        raw_log_output = capsys.readouterr().err
+        log_output = json.loads(raw_log_output)
+
+        expected = {
+            "severity": "ERROR",
+            "message": "testing circular",
+            "a": {"a": "foo"},
+            "b": {"a": "foo"},
+        }
+        assert log_output == expected
+
+    def test_no_false_circular_in_array_duplicates(self, capsys: pytest.CaptureFixture[str]):
+        # Ensure that duplicate objects in arrays are not falsely detected as circular.
+        obj = {"a": "foo"}
+        arr = [
+            {"a": obj, "b": obj},
+            {"a": obj, "b": obj},
+        ]
+        entry = {
+            "severity": "ERROR",
+            "message": "testing circular",
+            "a": arr,
+            "b": arr,
+        }
+        logger.write(entry)
+        raw_log_output = capsys.readouterr().err
+        log_output = json.loads(raw_log_output)
+
+        expected = {
+            "severity": "ERROR",
+            "message": "testing circular",
+            "a": [
+                {"a": {"a": "foo"}, "b": {"a": "foo"}},
+                {"a": {"a": "foo"}, "b": {"a": "foo"}},
+            ],
+            "b": [
+                {"a": {"a": "foo"}, "b": {"a": "foo"}},
+                {"a": {"a": "foo"}, "b": {"a": "foo"}},
+            ],
+        }
+        assert log_output == expected