Better handling of exception groups

sentry_sdk/utils.py

@@ -775,14 +775,17 @@ def exceptions_from_error(
 ):
     # type: (...) -> Tuple[int, List[Dict[str, Any]]]
     """
-    Creates the list of exceptions.
-    This can include chained exceptions and exceptions from an ExceptionGroup.
-
-    See the Exception Interface documentation for more details:
-    https://develop.sentry.dev/sdk/event-payloads/exception/
+    Converts the given exception information into the Sentry structured "exception" format.
+    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/
+
+    This function can handle:
+    - simple exceptions
+    - chained exceptions (raise .. from ..)
+    - exception groups
     """
-
-    parent = single_exception_from_error_tuple(
+    base_exception = single_exception_from_error_tuple(
         exc_type=exc_type,
         exc_value=exc_value,
         tb=tb,
@@ -793,64 +796,63 @@ 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 = 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 = (
+    causing_exception = None
+    exception_source = None
+
+    # Add any causing exceptions, if present.
+    should_suppress_context = hasattr(exc_value, "__suppress_context__") and exc_value.__suppress_context__  # type: ignore
+    # Note: __suppress_context__ is True if the exception is raised with the `from` keyword.
+    if should_suppress_context:
+        # Explicitly chained exceptions (Like: raise NewException() from OriginalException())
+        # The field `__cause__` is set to OriginalException
+        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,
-            )
-            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 = (
+        # Implicitly chained exceptions (when an exception occurs while handling another exception)
+        # The field `__context__` is set in the exception that occurs while handling another exception,
+        # to the other exception.
+        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,
-            )
-            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,
+        )
+        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,
@@ -870,38 +872,29 @@ def exceptions_from_error_tuple(
     full_stack=None,  # type: Optional[list[dict[str, Any]]]
 ):
     # type: (...) -> List[Dict[str, Any]]
+    """
+    Convert Python's exception information into Sentry's structured "exception" format in the event.
+    See https://develop.sentry.dev/sdk/data-model/event-payloads/exception/
+    This is the entry point for the exception handling.
+    """
+    # unpack the exception info tuple
     exc_type, exc_value, tb = exc_info
 
-    is_exception_group = BaseExceptionGroup is not None and isinstance(
-        exc_value, BaseExceptionGroup
+    # let exceptions_from_error do the actual work
+    _, 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,
-                )
-            )
-
+    # make sure the exceptions are sorted
+    # from the innermost (oldest)
+    # to the outermost (newest) exception
     exceptions.reverse()
 
     return exceptions

tests/integrations/ariadne/test_ariadne.py

@@ -68,7 +68,8 @@ 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 event["exception"]["values"][0]["mechanism"]["type"] == "chained"
+    assert event["exception"]["values"][-1]["mechanism"]["type"] == "ariadne"
     assert event["contexts"]["response"] == {
         "data": {
             "data": {"error": None},
@@ -111,7 +112,9 @@ def graphql_server():
     assert len(events) == 1
 
     (event,) = events
-    assert event["exception"]["values"][0]["mechanism"]["type"] == "ariadne"
+    assert event["exception"]["values"][0]["mechanism"]["type"] == "chained"
+    assert event["exception"]["values"][-1]["mechanism"]["type"] == "ariadne"
+
     assert event["contexts"]["response"] == {
         "data": {
             "data": {"error": None},
@@ -152,7 +155,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 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"]
 
@@ -182,7 +187,8 @@ def graphql_server():
     assert len(events) == 1
 
     (event,) = events
-    assert event["exception"]["values"][0]["mechanism"]["type"] == "ariadne"
+    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"]
 

tests/integrations/strawberry/test_strawberry.py

@@ -204,7 +204,8 @@ def test_capture_request_if_available_and_send_pii_is_on(
 
     (error_event,) = events
 
-    assert error_event["exception"]["values"][0]["mechanism"]["type"] == "strawberry"
+    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,
@@ -258,7 +259,8 @@ 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 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"]
 

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",