fix(errors): order and mark the stack of a never-raised exception

.sampo/changesets/quiet-frost-tapio.md

@@ -0,0 +1,5 @@
+---
+pypi/posthog: patch
+---
+
+An exception passed to `capture_exception` without being raised now gets its synthesized stack in outermost-first order, without the SDK's own capture frames, and with `mechanism.synthetic` set to `true`.

posthog/exception_utils.py

@@ -681,9 +681,12 @@ def single_exception_from_error_tuple(
         exception_value["mechanism"].setdefault("handled", True)
         exception_value["mechanism"].pop("source", None)
 
-    # Python capture inputs are runtime exceptions and this builder never
-    # replaces their stack with an SDK-generated current stack.
-    exception_value["mechanism"].setdefault("synthetic", False)
+    # The stack is synthetic only when the SDK built it, which happens for an
+    # exception that was passed to capture without ever being raised.
+    exception_value["mechanism"].setdefault(
+        "synthetic",
+        tb is not None and _is_synthesized_traceback(exc_value, tb),
+    )
 
     is_exception_group = BaseExceptionGroup is not None and isinstance(
         exc_value, BaseExceptionGroup
@@ -1048,29 +1051,71 @@ def exc_info_from_error(error):
     return exc_info
 
 
+_SYNTHESIZED_TRACEBACK_ATTR = "__posthog_synthesized_traceback"
+
+# Taken from this module's name because the posthoganalytics mirror is the same
+# code under another package name.
+_SDK_PACKAGE = __name__.partition(".")[0]
+
+
+class _SynthesizedTraceback:
+    """Holds the traceback the SDK built for an exception that was never raised."""
+
+    __slots__ = ("traceback",)
+
+    def __init__(self, traceback):
+        # type: (TracebackType) -> None
+        self.traceback = traceback
+
+    def __reduce__(self):
+        # A pickle or deep copy of the exception has no traceback to mark, and a
+        # traceback can't be pickled. Reducing to None also keeps this class out
+        # of the pickle, so it still loads where the SDK isn't installed.
+        return (type(None), ())
+
+
+def _is_synthesized_traceback(exc_value, tb):
+    # type: (Optional[BaseException], TracebackType) -> bool
+    marker = getattr(exc_value, _SYNTHESIZED_TRACEBACK_ATTR, None)
+    return marker is not None and marker.traceback is tb
+
+
+def _is_sdk_frame(frame):
+    # type: (FrameType) -> bool
+    module = frame.f_globals.get("__name__") or ""
+    return module == _SDK_PACKAGE or module.startswith(_SDK_PACKAGE + ".")
+
+
 def construct_artificial_traceback(e):
     # type: (BaseException) -> None
     if getattr(e, "__traceback__", None) is not None:
         return
 
+    # Collect the current stack innermost first, leaving out the SDK's own
+    # capture frames so the stack ends where the application called the SDK.
     depth = 0
-    frames = []
+    frames = []  # type: List[FrameType]
     while True:
         try:
             frame = sys._getframe(depth)
             depth += 1
         except ValueError:
             break
 
+        if not frames and _is_sdk_frame(frame):
+            continue
         frames.append(frame)
 
-    frames.reverse()
-
+    # Link the traceback from the innermost frame outwards, so that following
+    # tb_next walks from the outermost frame to the innermost one, the same
+    # order the interpreter uses for a raised exception.
     tb = None
     for frame in frames:
         tb = types.TracebackType(tb, frame, frame.f_lasti, frame.f_lineno)
 
-    setattr(e, "__traceback__", tb)
+    if tb is not None:
+        setattr(e, "__traceback__", tb)
+        setattr(e, _SYNTHESIZED_TRACEBACK_ATTR, _SynthesizedTraceback(tb))
 
 
 def _module_in_list(name, items):

posthog/test/test_exception_capture.py

@@ -1,9 +1,12 @@
+import copy
+import importlib.util
+import pickle
 import subprocess
 import sys
 import threading
 from textwrap import dedent
 from types import SimpleNamespace
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, patch
 
 import pytest
 
@@ -526,3 +529,177 @@ def test_last_inspected_group_member_retains_its_cause_chain():
     ]
     assert exceptions[-1]["mechanism"]["source"] == "cause"
     assert exceptions[-1]["mechanism"]["parent_id"] == 2
+
+
+_APP_MODULE_SOURCE = dedent(
+    """
+    def entry_point(capture, error):
+        return handler(capture, error)
+
+    def handler(capture, error):
+        return capture(error)
+    """
+)
+
+
+def _call_from_app_module(capture, error):
+    # Run the capture call from frames that belong to an application module,
+    # not to the posthog package, the way a real caller would.
+    namespace = {"__name__": "app"}
+    exec(compile(_APP_MODULE_SOURCE, "app.py", "exec"), namespace)
+    return namespace["entry_point"](capture, error)
+
+
+def _exceptions_for_never_raised(error):
+    from posthog.exception_utils import exc_info_from_error, exceptions_from_error_tuple
+
+    return exceptions_from_error_tuple(
+        _call_from_app_module(exc_info_from_error, error)
+    )
+
+
+def test_never_raised_exception_stack_is_outermost_first():
+    exception = _exceptions_for_never_raised(ValueError("never raised"))[0]
+
+    functions = [frame["function"] for frame in exception["stacktrace"]["frames"]]
+    assert functions.index("entry_point") < functions.index("handler")
+    assert functions[-1] == "handler"
+
+
+def test_never_raised_exception_stack_omits_sdk_frames():
+    exception = _exceptions_for_never_raised(ValueError("never raised"))[0]
+
+    functions = [frame["function"] for frame in exception["stacktrace"]["frames"]]
+    assert "construct_artificial_traceback" not in functions
+    assert "exc_info_from_error" not in functions
+
+
+def test_never_raised_exception_stack_is_marked_synthetic():
+    exception = _exceptions_for_never_raised(ValueError("never raised"))[0]
+
+    assert exception["mechanism"]["synthetic"] is True
+
+
+def test_raised_exception_passed_by_object_is_not_synthetic():
+    from posthog.exception_utils import exc_info_from_error, exceptions_from_error_tuple
+
+    try:
+        raise ValueError("raised")
+    except ValueError as error:
+        exceptions = exceptions_from_error_tuple(exc_info_from_error(error))
+
+    assert exceptions[0]["mechanism"]["synthetic"] is False
+
+
+def test_client_capture_of_never_raised_exception_sends_synthetic_stack():
+    from posthog.client import Client
+
+    client = Client("phc_test", sync_mode=True)
+    try:
+        with patch.object(client, "capture", return_value="event-id") as capture:
+            _call_from_app_module(
+                lambda error: client.capture_exception(error, distinct_id="user"),
+                ValueError("never raised"),
+            )
+
+        exception = capture.call_args.kwargs["properties"]["$exception_list"][0]
+        functions = [frame["function"] for frame in exception["stacktrace"]["frames"]]
+        assert exception["mechanism"]["synthetic"] is True
+        assert functions.index("entry_point") < functions.index("handler")
+        assert "capture_exception" not in functions
+    finally:
+        client.shutdown()
+
+
+def test_never_raised_exception_stays_pickleable_after_capture():
+    from posthog.exception_utils import exc_info_from_error
+
+    error = ValueError("never raised")
+    _call_from_app_module(exc_info_from_error, error)
+
+    restored = pickle.loads(pickle.dumps(error))
+
+    assert isinstance(restored, ValueError)
+    assert restored.args == ("never raised",)
+
+
+def test_never_raised_exception_is_not_synthetic_once_raised():
+    from posthog.exception_utils import exc_info_from_error, exceptions_from_error_tuple
+
+    error = ValueError("never raised")
+    _call_from_app_module(exc_info_from_error, error)
+
+    try:
+        raise error
+    except ValueError as raised:
+        exceptions = exceptions_from_error_tuple(exc_info_from_error(raised))
+
+    assert exceptions[0]["mechanism"]["synthetic"] is False
+
+
+def _pickle_round_trip(error):
+    return pickle.loads(pickle.dumps(error))
+
+
+@pytest.mark.parametrize("duplicate", [_pickle_round_trip, copy.copy, copy.deepcopy])
+def test_copy_of_captured_exception_is_not_synthetic_once_raised(duplicate):
+    from posthog.exception_utils import exc_info_from_error, exceptions_from_error_tuple
+
+    # Capture inline, so no application frame keeps the exception alive and the
+    # synthesized traceback is freed as soon as the original goes away. Its
+    # address is then free for the traceback of the raise below.
+    for _ in range(50):
+        error = ValueError("never raised")
+        exc_info_from_error(error)
+        duplicated = duplicate(error)
+        del error
+
+        try:
+            raise duplicated
+        except ValueError as raised:
+            exceptions = exceptions_from_error_tuple(exc_info_from_error(raised))
+
+        assert exceptions[0]["mechanism"]["synthetic"] is False
+
+
+def test_captured_exception_can_be_unpickled_without_the_sdk():
+    from posthog.exception_utils import exc_info_from_error
+
+    error = ValueError("never raised")
+    _call_from_app_module(exc_info_from_error, error)
+
+    result = subprocess.run(
+        [
+            sys.executable,
+            "-I",
+            "-S",
+            "-c",
+            "import pickle, sys; print(pickle.loads(sys.stdin.buffer.read()))",
+        ],
+        input=pickle.dumps(error),
+        capture_output=True,
+    )
+
+    assert result.returncode == 0, result.stderr
+    assert result.stdout.strip() == b"never raised"
+
+
+def test_never_raised_exception_stack_omits_sdk_frames_in_renamed_package():
+    import posthog.exception_utils as exception_utils
+
+    # The posthoganalytics mirror ships this module under another package name.
+    spec = importlib.util.spec_from_file_location(
+        "posthoganalytics.exception_utils", exception_utils.__file__
+    )
+    mirror = importlib.util.module_from_spec(spec)
+    with patch.dict(sys.modules, {spec.name: mirror}):
+        spec.loader.exec_module(mirror)
+        exception = mirror.exceptions_from_error_tuple(
+            _call_from_app_module(
+                mirror.exc_info_from_error, ValueError("never raised")
+            )
+        )[0]
+
+    functions = [frame["function"] for frame in exception["stacktrace"]["frames"]]
+    assert functions[-1] == "handler"
+    assert exception["mechanism"]["synthetic"] is True