feat(grouping): Exception Groups

src/sentry/eventtypes/error.py

@@ -23,7 +23,32 @@ class ErrorEvent(BaseEvent):
     key = "error"
 
     def extract_metadata(self, data):
-        exception = get_path(data, "exception", "values", -1)
+
+        exceptions = get_path(data, "exception", "values")
+        if not exceptions:
+            return {}
+
+        # When there are multiple exceptions, we need to pick one to extract the metadata from.
+        # If the event data has been marked with a main_exception_id, then we should be able to
+        # find the exception with the matching metadata.exception_id and use that one.
+        # This can be the case for some exception groups.
+
+        # Otherwise, the default behavior is to use the last one in the list.
+
+        main_exception_id = get_path(data, "main_exception_id")
+        exception = (
+            next(
+                exception
+                for exception in exceptions
+                if get_path(exception, "mechanism", "exception_id") == main_exception_id
+            )
+            if main_exception_id
+            else None
+        )
+
+        if not exception:
+            exception = get_path(exceptions, -1)
+
         if not exception:
             return {}
 

src/sentry/grouping/strategies/newstyle.py

@@ -1,5 +1,6 @@
+import itertools
 import re
-from typing import Any, Dict, List, Optional
+from typing import Any, Dict, Generator, List, Optional
 
 from sentry.eventstore.models import Event
 from sentry.grouping.component import GroupingComponent, calculate_tree_label
@@ -14,7 +15,7 @@
 from sentry.grouping.strategies.similarity_encoders import ident_encoder, text_shingle_encoder
 from sentry.grouping.strategies.utils import has_url_origin, remove_non_stacktrace_variants
 from sentry.interfaces.exception import Exception as ChainedException
-from sentry.interfaces.exception import SingleException
+from sentry.interfaces.exception import Mechanism, SingleException
 from sentry.interfaces.stacktrace import Frame, Stacktrace
 from sentry.interfaces.threads import Threads
 from sentry.stacktraces.platform import get_behavior_family_for_platform
@@ -706,9 +707,15 @@ def single_exception(
 def chained_exception(
     interface: ChainedException, event: Event, context: GroupingContext, **meta: Any
 ) -> ReturnedVariants:
+
+    # Get all the exceptions to consider.
+    all_exceptions = interface.exceptions()
+
+    # Filter them according to rules for handling exception groups.
+    exceptions = filter_exceptions_for_exception_groups(all_exceptions, context, event, **meta)
+
     # Case 1: we have a single exception, use the single exception
     # component directly to avoid a level of nesting
-    exceptions = interface.exceptions()
     if len(exceptions) == 1:
         return context.get_grouping_component(exceptions[0], event=event, **meta)
 
@@ -733,6 +740,83 @@ def chained_exception(
     return rv
 
 
+# See https://github.com/getsentry/rfcs/blob/main/text/0079-exception-groups.md#sentry-issue-grouping
+def filter_exceptions_for_exception_groups(
+    exceptions: List[SingleException], context: GroupingContext, event: Event, **meta: Any
+) -> List[SingleException]:
+
+    # This function only filters exceptions if there are at least two exceptions.
+    if len(exceptions) <= 1:
+        return exceptions
+
+    # Reconstruct the tree of exceptions if the required data is present.
+    nodes = {}
+    for exception in reversed(exceptions):
+        exception._children = []
+        mechanism: Mechanism = exception.mechanism
+        if mechanism and mechanism.exception_id is not None:
+            nodes[mechanism.exception_id] = exception
+            if mechanism.parent_id is not None:
+                nodes[mechanism.parent_id]._children.append(exception)
+
+    # If the nodes list is empty, then we don't have the required data to reconstruct the tree.
+    if len(nodes) == 0:
+        return exceptions
+
+    # Traverse the tree recursively from the root node to get all "top-level exceptions"
+    # and sort for consistency.
+    top_level_exceptions = sorted(
+        get_top_level_exceptions(nodes[0]), key=lambda exception: str(exception.type), reverse=True
+    )
+
+    # Figure out the distinct exceptions, grouping by the hashes of their grouping components.
+    distinct_exceptions = [
+        next(group)
+        for _, group in itertools.groupby(
+            top_level_exceptions,
+            key=lambda exception: [
+                component.get_hash()
+                for component in context.get_grouping_component(
+                    exception, event=event, **meta
+                ).values()
+            ],
+        )
+    ]
+
+    # If there's only one distinct top-level exception in the group,
+    # use it and its first-path children, but throw out the exception group and any copies.
+    # For example, Group<['Da', 'Da', 'Da']> should just be treated as a single 'Da'.
+    # We'll also set the main_exception_id, which is used in the extract_metadata function
+    # in src/sentry/eventtypes/error.py - which will ensure the issue is titled by this
+    # item rather than the exception group.
+    if len(distinct_exceptions) == 1:
+        event.data["main_exception_id"] = distinct_exceptions[0].mechanism.exception_id
+        return list(get_first_path(distinct_exceptions[0]))
+
+    # When there's more than one distinct top-level exception, return one of each of them AND the root exception group.
+    # NOTE: This deviates from the original RFC, because finding a common ancestor that shares
+    # one of each top-level exception that is _not_ the root is overly complicated.
+    # Also, it's more likely the stack trace of the root exception will be more meaningful
+    # than one of an inner exception group.
+    distinct_exceptions.append(nodes[0])
+    return distinct_exceptions
+
+
+def get_top_level_exceptions(node: SingleException) -> Generator[SingleException, None, None]:
+    if node.mechanism.is_exception_group:
+        yield from itertools.chain.from_iterable(
+            get_top_level_exceptions(child) for child in node._children
+        )
+    else:
+        yield node
+
+
+def get_first_path(node: SingleException) -> Generator[SingleException, None, None]:
+    yield node
+    if len(node._children) > 0:
+        yield from get_first_path(node._children[0])
+
+
 @chained_exception.variant_processor
 def chained_exception_variant_processor(
     variants: ReturnedVariants, context: GroupingContext, **meta: Any

src/sentry/interfaces/exception.py

@@ -465,15 +465,15 @@ def to_string(self, event, is_public=False, **kwargs):
         return ("".join(output)).strip()
 
     def get_stacktrace(self, *args, **kwargs):
-        exc = self.values[0]
+        exc = self.values[-1]
         if exc.stacktrace:
             return exc.stacktrace.get_stacktrace(*args, **kwargs)
         return ""
 
     def iter_tags(self):
-        if not self.values or not self.values[0]:
+        if not self.values or not self.values[-1]:
             return
 
-        mechanism = self.values[0].mechanism
+        mechanism = self.values[-1].mechanism
         if mechanism:
             yield from mechanism.iter_tags()

tests/sentry/event_manager/interfaces/snapshots/test_exception/test_context_with_mechanism.pysnap

@@ -1,5 +1,5 @@
 ---
-created: '2023-04-19T16:08:28.133847Z'
+created: '2023-05-05T19:28:49.089651Z'
 creator: sentry
 source: tests/sentry/event_manager/interfaces/test_exception.py
 ---
@@ -9,9 +9,7 @@ get_api_context:
   hasSystemFrames: true
   values:
   - mechanism:
-      exception_id: 1
-      is_exception_group: true
-      parent_id: 0
+      exception_id: 0
       source: __context__
       type: generic
     module: foo.bar
@@ -41,12 +39,13 @@ get_api_context:
     threadId: null
     type: ValueError
     value: hello world
+tags:
+- - mechanism
+  - generic
 to_json:
   values:
   - mechanism:
-      exception_id: 1
-      is_exception_group: true
-      parent_id: 0
+      exception_id: 0
       source: __context__
       type: generic
     module: foo.bar

tests/sentry/event_manager/interfaces/snapshots/test_exception/test_context_with_two_exceptions_having_mechanism.pysnap

@@ -0,0 +1,115 @@
+---
+created: '2023-05-05T19:28:49.100372Z'
+creator: sentry
+source: tests/sentry/event_manager/interfaces/test_exception.py
+---
+errors: null
+get_api_context:
+  excOmitted: null
+  hasSystemFrames: true
+  values:
+  - mechanism:
+      exception_id: 1
+      handled: true
+      parent_id: 0
+      source: __context__
+      type: chained
+    module: foo.bar
+    rawStacktrace: null
+    stacktrace:
+      frames:
+      - absPath: foo/baz.py
+        colNo: null
+        context: []
+        errors: null
+        filename: foo/baz.py
+        function: null
+        inApp: true
+        instructionAddr: null
+        lineNo: 1
+        module: null
+        package: null
+        platform: null
+        rawFunction: null
+        symbol: null
+        symbolAddr: null
+        trust: null
+        vars: null
+      framesOmitted: null
+      hasSystemFrames: true
+      registers: null
+    threadId: null
+    type: ValueError
+    value: hello world
+  - mechanism:
+      exception_id: 0
+      handled: false
+      is_exception_group: true
+      source: __context__
+      type: generic
+    module: foo.bar
+    rawStacktrace: null
+    stacktrace:
+      frames:
+      - absPath: foo/baz.py
+        colNo: null
+        context: []
+        errors: null
+        filename: foo/baz.py
+        function: null
+        inApp: true
+        instructionAddr: null
+        lineNo: 1
+        module: null
+        package: null
+        platform: null
+        rawFunction: null
+        symbol: null
+        symbolAddr: null
+        trust: null
+        vars: null
+      framesOmitted: null
+      hasSystemFrames: true
+      registers: null
+    threadId: null
+    type: ValueError
+    value: hello world
+tags:
+- - handled
+  - 'no'
+- - mechanism
+  - generic
+to_json:
+  values:
+  - mechanism:
+      exception_id: 1
+      handled: true
+      parent_id: 0
+      source: __context__
+      type: chained
+    module: foo.bar
+    stacktrace:
+      frames:
+      - abs_path: foo/baz.py
+        filename: foo/baz.py
+        in_app: true
+        lineno: 1
+    type: ValueError
+    value: hello world
+  - mechanism:
+      exception_id: 0
+      handled: false
+      is_exception_group: true
+      source: __context__
+      type: generic
+    module: foo.bar
+    stacktrace:
+      frames:
+      - abs_path: foo/baz.py
+        filename: foo/baz.py
+        in_app: true
+        lineno: 1
+    type: ValueError
+    value: hello world
+to_string: "ValueError: hello world\n  File \"foo/baz.py\", line 1\n\nValueError:\
+  \ hello world\n  File \"foo/baz.py\", line 1"

tests/sentry/event_manager/interfaces/test_exception.py

@@ -15,14 +15,18 @@ def inner(data):
 
         interface = evt.interfaces.get("exception")
 
-        insta_snapshot(
-            {
-                "errors": evt.data.get("errors"),
-                "to_json": interface and interface.to_json(),
-                "get_api_context": interface and interface.get_api_context(),
-                "to_string": interface and interface.to_string(evt),
-            }
-        )
+        snapshot_values = {
+            "errors": evt.data.get("errors"),
+            "to_json": interface and interface.to_json(),
+            "get_api_context": interface and interface.get_api_context(),
+            "to_string": interface and interface.to_string(evt),
+        }
+
+        tags = sorted(interface.iter_tags())
+        if len(tags) > 0:
+            snapshot_values["tags"] = tags
+
+        insta_snapshot(snapshot_values)
 
     return inner
 
@@ -209,11 +213,48 @@ def test_context_with_mechanism(make_exception_snapshot):
                     "mechanism": {
                         "type": "generic",
                         "source": "__context__",
-                        "is_exception_group": True,
+                        "exception_id": 0,
+                    },
+                }
+            ]
+        )
+    )
+
+
+def test_context_with_two_exceptions_having_mechanism(make_exception_snapshot):
+    make_exception_snapshot(
+        dict(
+            values=[
+                {
+                    "type": "ValueError",
+                    "value": "hello world",
+                    "module": "foo.bar",
+                    "stacktrace": {
+                        "frames": [{"filename": "foo/baz.py", "lineno": 1, "in_app": True}]
+                    },
+                    "mechanism": {
+                        "type": "chained",
+                        "handled": True,
+                        "source": "__context__",
                         "exception_id": 1,
                         "parent_id": 0,
                     },
-                }
+                },
+                {
+                    "type": "ValueError",
+                    "value": "hello world",
+                    "module": "foo.bar",
+                    "stacktrace": {
+                        "frames": [{"filename": "foo/baz.py", "lineno": 1, "in_app": True}]
+                    },
+                    "mechanism": {
+                        "type": "generic",
+                        "handled": False,
+                        "source": "__context__",
+                        "is_exception_group": True,
+                        "exception_id": 0,
+                    },
+                },
             ]
         )
     )