diff --git a/issues/tests.py b/issues/tests.py index 6c4ddf5..52fbc30 100644 --- a/issues/tests.py +++ b/issues/tests.py @@ -532,16 +532,16 @@ class IntegrationTest(DjangoTestCase): class GroupingUtilsTestCase(DjangoTestCase): def test_empty_data(self): - self.assertEquals("Log Message: ⋄ ", get_issue_grouper_for_data({})) + self.assertEquals("Log Message: ⋄ ", 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: ? ⋄ ", 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 ⋄ ", get_issue_grouper_for_data({"logentry": { "formatted": "msg: foobar", }})) @@ -554,32 +554,32 @@ class GroupingUtilsTestCase(DjangoTestCase): })) def test_exception_empty_trace(self): - self.assertEquals(" ⋄ ", get_issue_grouper_for_data({"exception": { + self.assertEquals(" ⋄ ", get_issue_grouper_for_data({"exception": { "values": [], }})) def test_exception_trace_no_data(self): - self.assertEquals(" ⋄ ", get_issue_grouper_for_data({"exception": { + self.assertEquals(" ⋄ ", 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 ⋄ ", 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 ⋄ ", 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 ⋄ ", 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 ⋄ ", 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 ⋄ ", get_issue_grouper_for_data({ "exception": { "values": [{ "stacktrace": { @@ -605,7 +605,7 @@ class GroupingUtilsTestCase(DjangoTestCase): })) def test_synthetic_exception_only(self): - self.assertEquals(" ⋄ ", get_issue_grouper_for_data({ + self.assertEquals(" ⋄ ", 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(" ⋄ ", get_issue_grouper_for_data({ + self.assertEquals(" ⋄ ", 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 ⋄ ", 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 ⋄ ", 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: ⋄ ⋄ fixed string", get_issue_grouper_for_data({ - "fingerprint": ["{{ default }}", "fixed string"], - })) + self.assertEquals("Log Message: ⋄ ⋄ fixed string", + get_issue_grouper_for_data({"fingerprint": ["{{ default }}", "fixed string"]})) diff --git a/issues/utils.py b/issues/utils.py index 5bcc914..2369a34 100644 --- a/issues/utils.py +++ b/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", "" + + +def get_crash_location(data): + frame = get_crash_frame_from_event_data( + data, + frame_filter=lambda x: x.get("function") not in (None, "", ""), + ) + 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 "", "" + + exception = get_path(data, "exception", "values", -1) + if not exception: + return "", "" + + 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 "", "" + + 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 "") fingerprint = data.get("fingerprint") if fingerprint: diff --git a/sentry/eventtypes/__init__.py b/sentry/eventtypes/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/sentry/eventtypes/base.py b/sentry/eventtypes/base.py deleted file mode 100644 index a3935ce..0000000 --- a/sentry/eventtypes/base.py +++ /dev/null @@ -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", "" diff --git a/sentry/eventtypes/error.py b/sentry/eventtypes/error.py deleted file mode 100644 index fa89138..0000000 --- a/sentry/eventtypes/error.py +++ /dev/null @@ -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, "", ""), - ) - 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 "", "" - - exception = get_path(data, "exception", "values", -1) - if not exception: - return "", "" - - 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 "", "" - - type_ = trim(get_path(exception, "type", default="Error"), 128) - - return type_, value