Make <no transaction> explicit;

and more moving-around-of-code in preparation for our next step

issues/tests.py

@@ -532,16 +532,16 @@ class IntegrationTest(DjangoTestCase):
 class GroupingUtilsTestCase(DjangoTestCase):
 
     def test_empty_data(self):
-        self.assertEquals("Log Message: <no log message> ⋄ ", get_issue_grouper_for_data({}))
+        self.assertEquals("Log Message: <no log message> ⋄ <no transaction>", get_issue_grouper_for_data({}))
 
     def test_logentry_message_takes_precedence(self):
-        self.assertEquals("Log Message: msg: ? ⋄ ", get_issue_grouper_for_data({"logentry": {
+        self.assertEquals("Log Message: msg: ? ⋄ <no transaction>", get_issue_grouper_for_data({"logentry": {
             "message": "msg: ?",
             "formatted": "msg: foobar",
         }}))
 
     def test_logentry_with_formatted_only(self):
-        self.assertEquals("Log Message: msg: foobar ⋄ ", get_issue_grouper_for_data({"logentry": {
+        self.assertEquals("Log Message: msg: foobar ⋄ <no transaction>", get_issue_grouper_for_data({"logentry": {
             "formatted": "msg: foobar",
         }}))
 
@@ -554,32 +554,32 @@ class GroupingUtilsTestCase(DjangoTestCase):
         }))
 
     def test_exception_empty_trace(self):
-        self.assertEquals("<unknown> ⋄ ", get_issue_grouper_for_data({"exception": {
+        self.assertEquals("<unknown> ⋄ <no transaction>", get_issue_grouper_for_data({"exception": {
             "values": [],
         }}))
 
     def test_exception_trace_no_data(self):
-        self.assertEquals("<unknown> ⋄ ", get_issue_grouper_for_data({"exception": {
+        self.assertEquals("<unknown> ⋄ <no transaction>", get_issue_grouper_for_data({"exception": {
             "values": [{}],
         }}))
 
     def test_exception_value_only(self):
-        self.assertEquals("Error: exception message ⋄ ", get_issue_grouper_for_data({"exception": {
+        self.assertEquals("Error: exception message ⋄ <no transaction>", get_issue_grouper_for_data({"exception": {
             "values": [{"value": "exception message"}],
         }}))
 
     def test_exception_type_only(self):
-        self.assertEquals("KeyError ⋄ ", get_issue_grouper_for_data({"exception": {
+        self.assertEquals("KeyError ⋄ <no transaction>", get_issue_grouper_for_data({"exception": {
             "values": [{"type": "KeyError"}],
         }}))
 
     def test_exception_type_value(self):
-        self.assertEquals("KeyError: exception message ⋄ ", get_issue_grouper_for_data({"exception": {
+        self.assertEquals("KeyError: exception message ⋄ <no transaction>", get_issue_grouper_for_data({"exception": {
             "values": [{"type": "KeyError", "value": "exception message"}],
         }}))
 
     def test_exception_multiple_frames(self):
-        self.assertEquals("KeyError: exception message ⋄ ", get_issue_grouper_for_data({"exception": {
+        self.assertEquals("KeyError: exception message ⋄ <no transaction>", get_issue_grouper_for_data({"exception": {
             "values": [{}, {}, {}, {"type": "KeyError", "value": "exception message"}],
         }}))
 
@@ -594,7 +594,7 @@ class GroupingUtilsTestCase(DjangoTestCase):
     def test_exception_function_is_ignored_unless_specifically_synthetic(self):
         # I make no value-judgement here on whether this is something we want to replicate in the future; as it stands
         # this test just documents the somewhat surprising behavior that we inherited from GlitchTip/Sentry.
-        self.assertEquals("Error ⋄ ", get_issue_grouper_for_data({
+        self.assertEquals("Error ⋄ <no transaction>", get_issue_grouper_for_data({
             "exception": {
                 "values": [{
                     "stacktrace": {
@@ -605,7 +605,7 @@ class GroupingUtilsTestCase(DjangoTestCase):
         }))
 
     def test_synthetic_exception_only(self):
-        self.assertEquals("<unknown> ⋄ ", get_issue_grouper_for_data({
+        self.assertEquals("<unknown> ⋄ <no transaction>", get_issue_grouper_for_data({
             "exception": {
                 "values": [{
                     "mechanism": {"synthetic": True},
@@ -614,7 +614,7 @@ class GroupingUtilsTestCase(DjangoTestCase):
         }))
 
     def test_synthetic_exception_ignores_value(self):
-        self.assertEquals("<unknown> ⋄ ", get_issue_grouper_for_data({
+        self.assertEquals("<unknown> ⋄ <no transaction>", get_issue_grouper_for_data({
             "exception": {
                 "values": [{
                     "mechanism": {"synthetic": True},
@@ -624,7 +624,7 @@ class GroupingUtilsTestCase(DjangoTestCase):
         }))
 
     def test_exception_uses_function_when_top_level_exception_is_synthetic(self):
-        self.assertEquals("foo ⋄ ", get_issue_grouper_for_data({
+        self.assertEquals("foo ⋄ <no transaction>", get_issue_grouper_for_data({
             "exception": {
                 "values": [{
                     "mechanism": {"synthetic": True},
@@ -638,7 +638,7 @@ class GroupingUtilsTestCase(DjangoTestCase):
     def test_exception_with_non_string_value(self):
         # In the GlitchTip code there is a mention of value sometimes containing a non-string value. Whether this
         # happens in practice is unknown to me, but let's build something that can handle it.
-        self.assertEquals("KeyError: 123 ⋄ ", get_issue_grouper_for_data({"exception": {
+        self.assertEquals("KeyError: 123 ⋄ <no transaction>", get_issue_grouper_for_data({"exception": {
             "values": [{"type": "KeyError", "value": 123}],
         }}))
 
@@ -646,6 +646,5 @@ class GroupingUtilsTestCase(DjangoTestCase):
         self.assertEquals("fixed string", get_issue_grouper_for_data({"fingerprint": ["fixed string"]}))
 
     def test_fingerprint_with_default(self):
-        self.assertEquals("Log Message: <no log message> ⋄  ⋄ fixed string", get_issue_grouper_for_data({
-            "fingerprint": ["{{ default }}", "fixed string"],
-        }))
+        self.assertEquals("Log Message: <no log message> ⋄ <no transaction> ⋄ fixed string",
+                          get_issue_grouper_for_data({"fingerprint": ["{{ default }}", "fixed string"]}))

issues/utils.py

@@ -1,7 +1,66 @@
-from sentry.eventtypes.base import DefaultEvent
-from sentry.eventtypes.error import ErrorEvent
-
 from django.utils.encoding import force_str
+from django.template.defaultfilters import truncatechars
+
+from sentry.stacktraces.functions import get_function_name_for_frame
+from sentry.stacktraces.processing import get_crash_frame_from_event_data
+from sentry.utils.safe import get_path, trim
+
+from sentry.utils.strings import strip
+
+
+def get_type_and_value(data):
+    if "exception" in data and data["exception"]:
+        return get_exception_type_and_value_for_exception(data)
+    return get_exception_type_and_value_for_logmessage(data)
+
+
+def get_exception_type_and_value_for_logmessage(data):
+    message = strip(
+        get_path(data, "logentry", "message")
+        or get_path(data, "logentry", "formatted")
+    )
+
+    if message:
+        return "Log Message", truncatechars(message.splitlines()[0], 100)
+
+    return "Log Message", "<no log message>"
+
+
+def get_crash_location(data):
+    frame = get_crash_frame_from_event_data(
+        data,
+        frame_filter=lambda x: x.get("function") not in (None, "<redacted>", "<unknown>"),
+    )
+    if frame is not None:
+        func = get_function_name_for_frame(frame, data.get("platform"))
+        return frame.get("filename") or frame.get("abs_path"), func
+    return None, None
+
+
+def get_exception_type_and_value_for_exception(data):
+    if isinstance(data.get("exception"), list):
+        if len(data["exception"]) == 0:
+            return "<unknown>", ""
+
+    exception = get_path(data, "exception", "values", -1)
+    if not exception:
+        return "<unknown>", ""
+
+    value = trim(get_path(exception, "value", default=""), 1024)
+
+    # From the sentry docs:
+    # > An optional flag indicating that this error is synthetic. Synthetic errors are errors that carry little
+    # > meaning by themselves.
+    # If this flag is set, we ignored the Exception's type and used the function name instead (if available).
+    if get_path(exception, "mechanism", "synthetic"):
+        _, function = get_crash_location(data)
+        if function:
+            return function, ""
+        return "<unknown>", ""
+
+    type_ = trim(get_path(exception, "type", default="Error"), 128)
+
+    return type_, value
 
 
 def default_issue_grouper(title: str, transaction: str) -> str:
@@ -9,14 +68,9 @@ def default_issue_grouper(title: str, transaction: str) -> str:
 
 
 def get_issue_grouper_for_data(data):
-    if "exception" in data and data["exception"]:
-        eventtype = ErrorEvent()
-    else:
-        eventtype = DefaultEvent()
-
-    type_, value = eventtype.get_exception_type_and_value(data)
+    type_, value = get_type_and_value(data)
     title = get_title_for_exception_type_and_value(type_, value)
-    transaction = force_str(data.get("transaction") or "")
+    transaction = force_str(data.get("transaction") or "<no transaction>")
     fingerprint = data.get("fingerprint")
 
     if fingerprint:

sentry/eventtypes/base.py

@@ -1,20 +0,0 @@
-from sentry.utils.safe import get_path
-from sentry.utils.strings import strip
-from django.template.defaultfilters import truncatechars
-
-
-class DefaultEvent:
-    """The DefaultEvent is the Event for which there is no exception set. Given the implementation of `get_title`, I'd
-    actually say that this is basically the LogMessageEvent.
-    """
-
-    def get_exception_type_and_value(self, data):
-        message = strip(
-            get_path(data, "logentry", "message")
-            or get_path(data, "logentry", "formatted")
-        )
-
-        if message:
-            return "Log Message", truncatechars(message.splitlines()[0], 100)
-
-        return "Log Message", "<no log message>"

sentry/eventtypes/error.py

@@ -1,42 +0,0 @@
-from sentry.stacktraces.functions import get_function_name_for_frame
-from sentry.stacktraces.processing import get_crash_frame_from_event_data
-from sentry.utils.safe import get_path, trim
-
-
-def get_crash_location(data):
-    frame = get_crash_frame_from_event_data(
-        data,
-        frame_filter=lambda x: x.get("function") not in (None, "<redacted>", "<unknown>"),
-    )
-    if frame is not None:
-        func = get_function_name_for_frame(frame, data.get("platform"))
-        return frame.get("filename") or frame.get("abs_path"), func
-    return None, None
-
-
-class ErrorEvent:
-
-    def get_exception_type_and_value(self, data):
-        if isinstance(data.get("exception"), list):
-            if len(data["exception"]) == 0:
-                return "<unknown>", ""
-
-        exception = get_path(data, "exception", "values", -1)
-        if not exception:
-            return "<unknown>", ""
-
-        value = trim(get_path(exception, "value", default=""), 1024)
-
-        # From the sentry docs:
-        # > An optional flag indicating that this error is synthetic. Synthetic errors are errors that carry little
-        # > meaning by themselves.
-        # If this flag is set, we ignored the Exception's type and used the function name instead (if available).
-        if get_path(exception, "mechanism", "synthetic"):
-            _, function = get_crash_location(data)
-            if function:
-                return function, ""
-            return "<unknown>", ""
-
-        type_ = trim(get_path(exception, "type", default="Error"), 128)
-
-        return type_, value