diff --git a/.sampo/changesets/canonical-exception-list-order.md b/.sampo/changesets/canonical-exception-list-order.md new file mode 100644 index 00000000..64179727 --- /dev/null +++ b/.sampo/changesets/canonical-exception-list-order.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: minor +--- + +fix(errors): emit `$exception_list` in canonical order — index `0` is the caught/outermost exception, causes follow in unwrap order, and the root cause is last (previously the list was reversed with the root cause first). This aligns posthog-python with the cross-SDK exception ordering spec. Frame order within each stacktrace is unchanged. diff --git a/posthog/exception_capture.py b/posthog/exception_capture.py index c35f14b2..627e8e47 100644 --- a/posthog/exception_capture.py +++ b/posthog/exception_capture.py @@ -10,7 +10,6 @@ from typing import TYPE_CHECKING from posthog.bucketed_rate_limiter import BucketedRateLimiter -from posthog.exception_utils import walk_exception_chain if TYPE_CHECKING: from posthog.client import Client @@ -93,14 +92,11 @@ def _exception_type(exception): getattr(exception, "__traceback__", None), ) - # PostHog groups exceptions by the root cause of the chain: - # exceptions_from_error_tuple reverses the walked chain, so - # $exception_list[0].type is the deepest cause, not the wrapping - # exception. Key on that same type so rate-limit buckets line up with - # server-side grouping (e.g. `raise RuntimeError from ZeroDivisionError` - # is keyed on ZeroDivisionError, not RuntimeError). + # Canonical `$exception_list` order puts the caught/outermost + # exception first, and server-side issue naming keys on that first + # entry. Key rate-limit buckets on the same type so they line up + # (e.g. `raise RuntimeError from ZeroDivisionError` is keyed on + # RuntimeError, matching the issue it groups into). exc_type = exc_info[0] - for chained_type, _, _ in walk_exception_chain(exc_info): - exc_type = chained_type return getattr(exc_type, "__name__", None) or "Exception" diff --git a/posthog/exception_utils.py b/posthog/exception_utils.py index 9d1b8ffa..a686a29e 100644 --- a/posthog/exception_utils.py +++ b/posthog/exception_utils.py @@ -734,8 +734,11 @@ def exceptions_from_error_tuple( single_exception_from_error_tuple(exc_type, exc_value, tb, mechanism) ) - exceptions.reverse() - + # Canonical ordering: $exception_list[0] is the caught/outermost exception, + # with each cause appended after its wrapper in unwrap order and the root + # cause last. Both branches above already build the list in this order + # (walk_exception_chain yields caught-first; exceptions_from_error keeps the + # parent before its children), so we intentionally do not reverse it. return exceptions diff --git a/posthog/test/mcp/test_fastmcp.py b/posthog/test/mcp/test_fastmcp.py index 0163de7b..87b46ea4 100644 --- a/posthog/test/mcp/test_fastmcp.py +++ b/posthog/test/mcp/test_fastmcp.py @@ -133,7 +133,11 @@ async def test_tool_call_error_is_captured_and_reraised(): assert calls and calls[0]["properties"]["$mcp_is_error"] is True exceptions = _events(client, "$exception") assert exceptions - assert exceptions[0]["properties"]["$exception_list"][0]["value"] == "explode" + # Canonical order: the caught/outermost exception (fastmcp's ToolError + # wrapper) is first, the root cause last. + exception_list = exceptions[0]["properties"]["$exception_list"] + assert exception_list[0]["value"] == "Error executing tool boom: explode" + assert exception_list[-1]["value"] == "explode" async def test_identify_sets_distinct_id_and_groups(): diff --git a/posthog/test/test_exception_capture.py b/posthog/test/test_exception_capture.py index bc632ba0..05e3d05a 100644 --- a/posthog/test/test_exception_capture.py +++ b/posthog/test/test_exception_capture.py @@ -106,23 +106,29 @@ def test_rate_limits_per_exception_type(): capture.close() -def test_rate_limit_keys_on_root_cause_of_chained_exceptions(): +def test_rate_limit_keys_on_outermost_of_chained_exceptions(): from posthog.exception_capture import ExceptionCapture - # PostHog groups by the root cause ($exception_list[0].type), so chained - # exceptions sharing a wrapper type but differing in their cause must land - # in separate buckets rather than collapsing under the wrapper. + # Canonical order puts the caught/outermost exception at + # $exception_list[0], and server-side issue naming keys on it — so chained + # exceptions sharing a wrapper type collapse into one bucket regardless of + # their cause, and a different wrapper type gets its own bucket. client = MagicMock() - capture = ExceptionCapture(client, rate_limiting_enabled=True, bucket_size=2) + capture = ExceptionCapture(client, rate_limiting_enabled=True, bucket_size=10) try: + for i in range(15): + cause = ZeroDivisionError() if i % 2 == 0 else KeyError() + capture.capture_exception(_chained_exc_info(cause, RuntimeError("wrapped"))) + + # one shared RuntimeError bucket: same arithmetic as the plain + # per-type test above (bucket size 10 -> 9 captured) + assert client.capture_exception.call_count == 9 + + # a different wrapper type has its own bucket capture.capture_exception( - _chained_exc_info(ZeroDivisionError(), RuntimeError("wrapped")) - ) - capture.capture_exception( - _chained_exc_info(KeyError(), RuntimeError("wrapped")) + _chained_exc_info(ZeroDivisionError(), ValueError("other wrapper")) ) - - assert client.capture_exception.call_count == 2 + assert client.capture_exception.call_count == 10 finally: capture.close() @@ -154,3 +160,85 @@ def test_excepthook(tmpdir): b'"$exception_list": [{"mechanism": {"type": "generic", "handled": true}, "module": null, "type": "ZeroDivisionError", "value": "division by zero", "stacktrace": {"frames": [{"platform": "python", "filename": "app.py", "abs_path"' in output ) + + +class _RootError(Exception): + pass + + +class _WrapperError(Exception): + pass + + +class _LeafOne(Exception): + pass + + +class _LeafTwo(Exception): + pass + + +def test_exception_list_canonical_order_explicit_cause(): + # Canonical ordering: $exception_list[0] is the caught/outermost exception + # and the root cause is last. For `raise B from A`, B is caught and A is the + # root cause. + from posthog.exception_utils import exceptions_from_error_tuple + + try: + try: + raise _RootError("root") + except _RootError as root: + raise _WrapperError("wrapper") from root + except _WrapperError: + exc_info = sys.exc_info() + + exceptions = exceptions_from_error_tuple(exc_info) + + types = [e["type"] for e in exceptions] + assert types == ["_WrapperError", "_RootError"] + assert exceptions[0]["value"] == "wrapper" + assert exceptions[-1]["value"] == "root" + + +def test_exception_list_canonical_order_implicit_context(): + # Implicit chaining (an exception raised while handling another) uses + # `__context__`. The caught exception is still first, root cause last. + from posthog.exception_utils import exceptions_from_error_tuple + + try: + try: + raise _RootError("root") + except _RootError: + raise _WrapperError("wrapper") + except _WrapperError: + exc_info = sys.exc_info() + + exceptions = exceptions_from_error_tuple(exc_info) + + types = [e["type"] for e in exceptions] + assert types == ["_WrapperError", "_RootError"] + assert exceptions[0]["value"] == "wrapper" + assert exceptions[-1]["value"] == "root" + + +@pytest.mark.skipif( + sys.version_info < (3, 11), + reason="ExceptionGroup requires Python 3.11+", +) +def test_exception_list_canonical_order_exception_group(): + # For an ExceptionGroup the group is the caught/outermost exception and + # comes first, with its member exceptions following. + from posthog.exception_utils import exceptions_from_error_tuple + + try: + raise ExceptionGroup( # noqa: F821 -- builtin on 3.11+ + "group", [_LeafOne("one"), _LeafTwo("two")] + ) + except BaseException: + exc_info = sys.exc_info() + + exceptions = exceptions_from_error_tuple(exc_info) + + types = [e["type"] for e in exceptions] + assert types[0] == "ExceptionGroup" + assert types[1:] == ["_LeafOne", "_LeafTwo"] diff --git a/references/public_api_snapshot.txt b/references/public_api_snapshot.txt index 848b3075..eeb5a246 100644 --- a/references/public_api_snapshot.txt +++ b/references/public_api_snapshot.txt @@ -297,7 +297,6 @@ alias posthog.disable_connection_reuse -> posthog.request.disable_connection_reu alias posthog.enable_keep_alive -> posthog.request.enable_keep_alive alias posthog.exception_capture.BucketedRateLimiter -> posthog.bucketed_rate_limiter.BucketedRateLimiter alias posthog.exception_capture.Client -> posthog.client.Client -alias posthog.exception_capture.walk_exception_chain -> posthog.exception_utils.walk_exception_chain alias posthog.exception_utils.ExcInfo -> posthog.args.ExcInfo alias posthog.exception_utils.ExceptionArg -> posthog.args.ExceptionArg alias posthog.feature_flag_evaluations.FlagValue -> posthog.types.FlagValue