diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 4f403f4e55..ed915a8778 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -31,6 +31,7 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh - The option `attach_stacktrace` is now `True` by default, meaning the SDK will attach stack traces to messages. - The Django integration now creates spans for cache operations by default. Pass `DjangoIntegration(cache_spans=False)` to `sentry_sdk.init()` to turn them off. - The Django integration no longer force-enables cache spans when Spotlight is active and `settings.DEBUG` is `True`. The `cache_spans` option is now always respected as given. +- Exception groups in exception chains are now properly unfurled. ### Logging diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 5abd52501e..f22ae0db72 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -865,27 +865,16 @@ def exceptions_from_error( seen_exception_ids: "Optional[Set[int]]" = None, ) -> "Tuple[int, List[Dict[str, Any]]]": """ - Creates the list of exceptions. - This can include chained exceptions and exceptions from an ExceptionGroup. + Convert the given exception information into the Sentry "exception" format. - See the Exception Interface documentation for more details: - https://develop.sentry.dev/sdk/event-payloads/exception/ - - Args: - exception_id (int): - - Sequential counter for assigning ``mechanism.exception_id`` - to each processed exception. Is NOT the result of calling `id()` on the exception itself. - - parent_id (int): - - The ``mechanism.exception_id`` of the parent exception. - - Written into ``mechanism.parent_id`` in the event payload so Sentry can - reconstruct the exception tree. + This will return a list of exceptions (a flattened tree of exceptions) in the + format of the Exception Interface documentation: + https://develop.sentry.dev/sdk/data-model/event-payloads/exception/ - Not to be confused with ``seen_exception_ids``, which tracks Python ``id()`` - values for cycle detection. + This function can handle: + - simple exceptions + - chained exceptions (raise .. from ..) + - exception groups """ if seen_exception_ids is None: @@ -901,7 +890,7 @@ def exceptions_from_error( seen_exceptions.append(exc_value) seen_exception_ids.add(id(exc_value)) - parent = single_exception_from_error_tuple( + base_exception = single_exception_from_error_tuple( exc_type=exc_type, exc_value=exc_value, tb=tb, @@ -912,70 +901,60 @@ def exceptions_from_error( source=source, full_stack=full_stack, ) - exceptions = [parent] + exceptions = [base_exception] parent_id = exception_id exception_id += 1 - should_supress_context = ( + causing_exception = None + exception_source = None + + should_suppress_context = ( hasattr(exc_value, "__suppress_context__") and exc_value.__suppress_context__ # type: ignore ) - if should_supress_context: - # Add direct cause. - # The field `__cause__` is set when raised with the exception (using the `from` keyword). - exception_has_cause = ( + if should_suppress_context: + has_explicit_causing_exception = ( exc_value and hasattr(exc_value, "__cause__") and exc_value.__cause__ is not None ) - if exception_has_cause: - cause = exc_value.__cause__ # type: ignore - (exception_id, child_exceptions) = exceptions_from_error( - exc_type=type(cause), - exc_value=cause, - tb=getattr(cause, "__traceback__", None), - client_options=client_options, - mechanism=mechanism, - exception_id=exception_id, - source="__cause__", - full_stack=full_stack, - seen_exceptions=seen_exceptions, - seen_exception_ids=seen_exception_ids, - ) - exceptions.extend(child_exceptions) - + if has_explicit_causing_exception: + exception_source = "__cause__" + causing_exception = exc_value.__cause__ # type: ignore else: - # Add indirect cause. - # The field `__context__` is assigned if another exception occurs while handling the exception. - exception_has_content = ( + has_implicit_causing_exception = ( exc_value and hasattr(exc_value, "__context__") and exc_value.__context__ is not None ) - if exception_has_content: - context = exc_value.__context__ # type: ignore - (exception_id, child_exceptions) = exceptions_from_error( - exc_type=type(context), - exc_value=context, - tb=getattr(context, "__traceback__", None), - client_options=client_options, - mechanism=mechanism, - exception_id=exception_id, - source="__context__", - full_stack=full_stack, - seen_exceptions=seen_exceptions, - seen_exception_ids=seen_exception_ids, - ) - exceptions.extend(child_exceptions) + if has_implicit_causing_exception: + exception_source = "__context__" + causing_exception = exc_value.__context__ # type: ignore + + if causing_exception: + (exception_id, child_exceptions) = exceptions_from_error( + exc_type=type(causing_exception), + exc_value=causing_exception, + tb=getattr(causing_exception, "__traceback__", None), + client_options=client_options, + mechanism=mechanism, + exception_id=exception_id, + parent_id=parent_id, + source=exception_source, + full_stack=full_stack, + seen_exceptions=seen_exceptions, + seen_exception_ids=seen_exception_ids, + ) + exceptions.extend(child_exceptions) - # Add exceptions from an ExceptionGroup. + # Add child exceptions from an ExceptionGroup. is_exception_group = exc_value and hasattr(exc_value, "exceptions") if is_exception_group: - for idx, e in enumerate(exc_value.exceptions): # type: ignore + for idx, causing_exception in enumerate(exc_value.exceptions): # type: ignore (exception_id, child_exceptions) = exceptions_from_error( - exc_type=type(e), - exc_value=e, - tb=getattr(e, "__traceback__", None), + exc_type=type(causing_exception), + exc_value=causing_exception, + tb=getattr(causing_exception, "__traceback__", None), client_options=client_options, mechanism=mechanism, exception_id=exception_id, @@ -996,38 +975,25 @@ def exceptions_from_error_tuple( mechanism: "Optional[Dict[str, Any]]" = None, full_stack: "Optional[list[dict[str, Any]]]" = None, ) -> "List[Dict[str, Any]]": + """ + Convert an exception into Sentry's structured "exception" format. + + See https://develop.sentry.dev/sdk/data-model/event-payloads/exception/ + This is the entry point for exception handling. + """ exc_type, exc_value, tb = exc_info - is_exception_group = BaseExceptionGroup is not None and isinstance( - exc_value, BaseExceptionGroup + _, exceptions = exceptions_from_error( + exc_type=exc_type, + exc_value=exc_value, + tb=tb, + client_options=client_options, + mechanism=mechanism, + exception_id=0, + parent_id=0, + full_stack=full_stack, ) - if is_exception_group: - (_, exceptions) = exceptions_from_error( - exc_type=exc_type, - exc_value=exc_value, - tb=tb, - client_options=client_options, - mechanism=mechanism, - exception_id=0, - parent_id=0, - full_stack=full_stack, - ) - - else: - exceptions = [] - for exc_type, exc_value, tb in walk_exception_chain(exc_info): - exceptions.append( - single_exception_from_error_tuple( - exc_type=exc_type, - exc_value=exc_value, - tb=tb, - client_options=client_options, - mechanism=mechanism, - full_stack=full_stack, - ) - ) - exceptions.reverse() return exceptions diff --git a/tests/integrations/ariadne/test_ariadne.py b/tests/integrations/ariadne/test_ariadne.py index 616a3b9746..68617ea3d3 100644 --- a/tests/integrations/ariadne/test_ariadne.py +++ b/tests/integrations/ariadne/test_ariadne.py @@ -70,7 +70,9 @@ def test_capture_request_and_response_if_send_pii_is_on_async( assert len(events) == 1 (event,) = events - assert event["exception"]["values"][0]["mechanism"]["type"] == "ariadne" + assert len(event["exception"]["values"]) == 2 + assert event["exception"]["values"][0]["mechanism"]["type"] == "chained" + assert event["exception"]["values"][-1]["mechanism"]["type"] == "ariadne" assert event["contexts"]["response"] == { "data": { "data": {"error": None}, @@ -113,7 +115,9 @@ def graphql_server(): assert len(events) == 1 (event,) = events - assert event["exception"]["values"][0]["mechanism"]["type"] == "ariadne" + assert len(event["exception"]["values"]) == 2 + assert event["exception"]["values"][0]["mechanism"]["type"] == "chained" + assert event["exception"]["values"][-1]["mechanism"]["type"] == "ariadne" assert event["contexts"]["response"] == { "data": { "data": {"error": None}, @@ -154,7 +158,9 @@ def test_do_not_capture_request_and_response_if_send_pii_is_off_async( assert len(events) == 1 (event,) = events - assert event["exception"]["values"][0]["mechanism"]["type"] == "ariadne" + assert len(event["exception"]["values"]) == 2 + assert event["exception"]["values"][0]["mechanism"]["type"] == "chained" + assert event["exception"]["values"][-1]["mechanism"]["type"] == "ariadne" assert "data" not in event["request"] assert "response" not in event["contexts"] @@ -184,7 +190,9 @@ def graphql_server(): assert len(events) == 1 (event,) = events - assert event["exception"]["values"][0]["mechanism"]["type"] == "ariadne" + assert len(event["exception"]["values"]) == 2 + assert event["exception"]["values"][0]["mechanism"]["type"] == "chained" + assert event["exception"]["values"][-1]["mechanism"]["type"] == "ariadne" assert "data" not in event["request"] assert "response" not in event["contexts"] diff --git a/tests/integrations/huggingface_hub/test_huggingface_hub.py b/tests/integrations/huggingface_hub/test_huggingface_hub.py index 5f39d3ff88..be5260f935 100644 --- a/tests/integrations/huggingface_hub/test_huggingface_hub.py +++ b/tests/integrations/huggingface_hub/test_huggingface_hub.py @@ -799,8 +799,10 @@ def test_chat_completion_api_error( (error,) = (item.payload for item in items if item.type == "event") - assert error["exception"]["values"][0]["mechanism"]["type"] == "huggingface_hub" - assert not error["exception"]["values"][0]["mechanism"]["handled"] + assert len(error["exception"]["values"]) == 2 + assert error["exception"]["values"][0]["mechanism"]["type"] == "chained" + assert error["exception"]["values"][-1]["mechanism"]["type"] == "huggingface_hub" + assert not error["exception"]["values"][-1]["mechanism"]["handled"] sentry_sdk.flush() spans = [item.payload for item in items if item.type == "span"] diff --git a/tests/integrations/strawberry/test_strawberry.py b/tests/integrations/strawberry/test_strawberry.py index 29f937598c..182577bef4 100644 --- a/tests/integrations/strawberry/test_strawberry.py +++ b/tests/integrations/strawberry/test_strawberry.py @@ -201,7 +201,9 @@ def test_capture_request_if_available_and_send_pii_is_on( (error_event,) = events - assert error_event["exception"]["values"][0]["mechanism"]["type"] == "strawberry" + assert len(error_event["exception"]["values"]) == 2 + assert error_event["exception"]["values"][0]["mechanism"]["type"] == "chained" + assert error_event["exception"]["values"][-1]["mechanism"]["type"] == "strawberry" assert error_event["request"]["api_target"] == "graphql" assert error_event["request"]["data"] == { "query": query, @@ -255,7 +257,10 @@ def test_do_not_capture_request_if_send_pii_is_off( assert len(events) == 1 (error_event,) = events - assert error_event["exception"]["values"][0]["mechanism"]["type"] == "strawberry" + + assert len(error_event["exception"]["values"]) == 2 + assert error_event["exception"]["values"][0]["mechanism"]["type"] == "chained" + assert error_event["exception"]["values"][-1]["mechanism"]["type"] == "strawberry" assert "data" not in error_event["request"] assert "response" not in error_event["contexts"] @@ -336,7 +341,9 @@ def test_event_processor_data_collection( assert len(events) == 1 (error_event,) = events - assert error_event["exception"]["values"][0]["mechanism"]["type"] == "strawberry" + assert len(error_event["exception"]["values"]) == 2 + assert error_event["exception"]["values"][0]["mechanism"]["type"] == "chained" + assert error_event["exception"]["values"][-1]["mechanism"]["type"] == "strawberry" # request.data comes from the framework integration and must not be # overwritten by the strawberry integration diff --git a/tests/test_exceptiongroup.py b/tests/test_exceptiongroup.py index 1290b78a8d..ae476f9c33 100644 --- a/tests/test_exceptiongroup.py +++ b/tests/test_exceptiongroup.py @@ -217,7 +217,10 @@ def test_exception_chain_cause(): { "mechanism": { "handled": False, - "type": "test_suite", + "type": "chained", + "exception_id": 1, + "parent_id": 0, + "source": "__cause__", }, "module": None, "type": "TypeError", @@ -227,6 +230,7 @@ def test_exception_chain_cause(): "mechanism": { "handled": False, "type": "test_suite", + "exception_id": 0, }, "module": None, "type": "ValueError", @@ -257,7 +261,10 @@ def test_exception_chain_context(): { "mechanism": { "handled": False, - "type": "test_suite", + "type": "chained", + "exception_id": 1, + "parent_id": 0, + "source": "__context__", }, "module": None, "type": "TypeError", @@ -267,6 +274,7 @@ def test_exception_chain_context(): "mechanism": { "handled": False, "type": "test_suite", + "exception_id": 0, }, "module": None, "type": "ValueError", @@ -297,6 +305,7 @@ def test_simple_exception(): "mechanism": { "handled": False, "type": "test_suite", + "exception_id": 0, }, "module": None, "type": "ValueError", @@ -308,6 +317,90 @@ def test_simple_exception(): assert exception_values == expected_exception_values +@minimum_python_311 +def test_exception_group_chained_with_context(): + try: + try: + raise ExceptionGroup( + "group", + [ + ValueError("child1"), + ExceptionGroup( + "child2", + [ + RuntimeError("grandchild1"), + RuntimeError("grandchild2"), + ], + ), + ], + ) + finally: + raise TypeError("bar") + except BaseException as e: + exc = e + + (event, _) = event_from_exception( + exc, + client_options={ + "include_local_variables": True, + "include_source_context": True, + "max_value_length": 1024, + }, + mechanism={"type": "test_suite", "handled": False}, + ) + + exception_values = event["exception"]["values"] + + # innermost (oldest) to outermost (newest) + assert [(e["type"], e["value"]) for e in exception_values] == [ + ("RuntimeError", "grandchild2"), + ("RuntimeError", "grandchild1"), + ("ExceptionGroup", "child2"), + ("ValueError", "child1"), + ("ExceptionGroup", "group"), + ("TypeError", "bar"), + ] + + # TypeError("bar") is the outermost exception (exception_id=0) + type_error = exception_values[-1] + assert type_error["mechanism"]["type"] == "test_suite" + assert type_error["mechanism"]["exception_id"] == 0 + + # ExceptionGroup("group") is the __context__ of TypeError + group = exception_values[-2] + assert group["mechanism"]["type"] == "chained" + assert group["mechanism"]["source"] == "__context__" + assert group["mechanism"]["parent_id"] == 0 + assert group["mechanism"]["is_exception_group"] is True + + group_id = group["mechanism"]["exception_id"] + + # ValueError("child1") and ExceptionGroup("child2") are children of "group" + child1 = exception_values[-3] + assert child1["type"] == "ValueError" + assert child1["mechanism"]["source"] == "exceptions[0]" + assert child1["mechanism"]["parent_id"] == group_id + + child2 = exception_values[-4] + assert child2["type"] == "ExceptionGroup" + assert child2["mechanism"]["source"] == "exceptions[1]" + assert child2["mechanism"]["parent_id"] == group_id + assert child2["mechanism"]["is_exception_group"] is True + + child2_id = child2["mechanism"]["exception_id"] + + # RuntimeError("grandchild1") and RuntimeError("grandchild2") are children of "child2" + rt1 = exception_values[-5] + assert rt1["value"] == "grandchild1" + assert rt1["mechanism"]["source"] == "exceptions[0]" + assert rt1["mechanism"]["parent_id"] == child2_id + + rt2 = exception_values[-6] + assert rt2["value"] == "grandchild2" + assert rt2["mechanism"]["source"] == "exceptions[1]" + assert rt2["mechanism"]["parent_id"] == child2_id + + @minimum_python_311 def test_exceptiongroup_starlette_collapse(): """