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

Author: CorieW

src/firebase_functions/logger.py

@@ -75,21 +75,32 @@ 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
+    result: _typing.Any
     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

@@ -5,6 +5,7 @@
 import pytest
 import json
 from firebase_functions import logger
+from typing import Any
 
 
 class TestLogger:
@@ -79,3 +80,147 @@ 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: Any = {"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: Any = {"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