diff --git a/.sampo/changesets/quiet-frost-tapio.md b/.sampo/changesets/quiet-frost-tapio.md new file mode 100644 index 000000000..660fd5ceb --- /dev/null +++ b/.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`. diff --git a/posthog/exception_utils.py b/posthog/exception_utils.py index 8608b96f0..a6f5883b2 100644 --- a/posthog/exception_utils.py +++ b/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,13 +1051,50 @@ 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) @@ -1062,15 +1102,20 @@ def construct_artificial_traceback(e): 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): diff --git a/posthog/test/test_exception_capture.py b/posthog/test/test_exception_capture.py index 011677779..a8ca09ff4 100644 --- a/posthog/test/test_exception_capture.py +++ b/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