From b597d91af7510993ad4c736c1afce8a5a8a28802 Mon Sep 17 00:00:00 2001 From: Klaas van Schelven Date: Mon, 16 Dec 2024 11:22:32 +0100 Subject: [PATCH] Become robust for lack of .values key in exception Fix #16 --- events/views.py | 3 +- ingest/management/commands/stress_test.py | 4 ++- issues/utils.py | 34 ++++++++++++++++++++++- issues/views.py | 5 ++-- sentry/stacktraces/processing.py | 7 +++-- 5 files changed, 45 insertions(+), 8 deletions(-) diff --git a/events/views.py b/events/views.py index 53f3b7d..3413259 100644 --- a/events/views.py +++ b/events/views.py @@ -4,6 +4,7 @@ from django.utils.http import content_disposition_header from django.shortcuts import render from bugsink.decorators import event_membership_required, atomic_for_request_method +from issues.utils import get_values @atomic_for_request_method @@ -19,7 +20,7 @@ def event_download(request, event, as_attachment=False): @event_membership_required def event_plaintext(request, event): parsed_data = json.loads(event.data) - exceptions = parsed_data["exception"]["values"] if "exception" in parsed_data else None + exceptions = get_values(parsed_data["exception"]) if "exception" in parsed_data else None return render(request, "events/event_stacktrace.txt", { "event": event, diff --git a/ingest/management/commands/stress_test.py b/ingest/management/commands/stress_test.py index 0370d2c..7a9e68b 100644 --- a/ingest/management/commands/stress_test.py +++ b/ingest/management/commands/stress_test.py @@ -13,6 +13,7 @@ from django.core.management.base import BaseCommand from compat.dsn import get_store_url, get_envelope_url, get_header_value from bugsink.streams import compress_with_zlib, WBITS_PARAM_FOR_GZIP, WBITS_PARAM_FOR_DEFLATE +from issues.utils import get_values class Command(BaseCommand): @@ -93,7 +94,8 @@ class Command(BaseCommand): into_chars = lambda i: "".join([chr(ord("A") + int(c)) for c in str(i)]) # noqa unevenly_distributed_number = int(1 / (random.random() + 0.0000001)) - data["exception"]["values"][0]["type"] = "Exception" + into_chars(unevenly_distributed_number) + values = get_values(data["exception"]) + values[0]["type"] = "Exception" + into_chars(unevenly_distributed_number) data_bytes = json.dumps(data).encode("utf-8") diff --git a/issues/utils.py b/issues/utils.py index 70dc72d..d3ad045 100644 --- a/issues/utils.py +++ b/issues/utils.py @@ -11,6 +11,34 @@ def maybe_empty(s): return "" if not s else s +def get_values(obj): + """ + https://develop.sentry.dev/sdk/data-model/event-payloads/exception/ + + > The exception attribute should be an object with the attribute values containing a list of one or more values that + > are objects in the format described below. Alternatively, the exception attribute may be a flat list of objects in + > the format below. + + Same for threads & breadcrumbs: + * https://develop.sentry.dev/sdk/data-model/event-payloads/threads/ + * https://develop.sentry.dev/sdk/data-model/event-payloads/breadcrumbs/ + + This function gets that list, whether the exception already was it, or whether it was wrapped in a dict as the + single value of a "values" key. + """ + if obj is None: + # None in None out allows us to compose this function with get_path (which returns None if any key is missing). + return None + + if isinstance(obj, list): + return obj + + if isinstance(obj, dict): + return obj["values"] + + raise ValueError("Expected exception/threads/breadcrumbs to be a list or a dict, got %r" % obj) + + def get_type_and_value_for_data(data): if "exception" in data and data["exception"]: return get_exception_type_and_value_for_exception(data) @@ -51,7 +79,11 @@ def get_exception_type_and_value_for_exception(data): # is a system to help you think about cases that you didn't properly think about in the first place), # although you may also care about the root cause. (In fact, sometimes you care more about the root cause, # but I'd say you'll have to yak-shave your way there). - exception = get_path(data, "exception", "values", -1) # .values is required by the json spec, so can be done safely + values = get_values(get_path(data, "exception")) + if values is None or len(values) == 0: + return "", "" + + exception = values[-1] if not exception: return "", "" diff --git a/issues/views.py b/issues/views.py index 936710e..2a4e16f 100644 --- a/issues/views.py +++ b/issues/views.py @@ -24,6 +24,7 @@ from projects.models import ProjectMembership from .models import Issue, IssueQuerysetStateManager, IssueStateManager, TurningPoint, TurningPointKind from .forms import CommentForm +from .utils import get_values MuteOption = namedtuple("MuteOption", ["for_or_until", "period_name", "nr_of_periods", "gte_threshold"]) @@ -307,9 +308,7 @@ def issue_event_stacktrace(request, issue, event_pk=None, digest_order=None, nav parsed_data = json.loads(event.data) - # sentry/glitchtip have some code here to deal with the case that "values" is not present, and exception itself is - # the list of exceptions, but we don't aim for endless backwards compat (yet) so we don't. - exceptions = parsed_data["exception"]["values"] if "exception" in parsed_data else None + exceptions = get_values(parsed_data["exception"]) if "exception" in parsed_data else None # NOTE: I considered making this a clickable button of some sort, but decided against it in the end. Getting the UI # right is quite hard (https://ux.stackexchange.com/questions/1318) but more generally I would assume that having diff --git a/sentry/stacktraces/processing.py b/sentry/stacktraces/processing.py index e9eecb2..ed57543 100644 --- a/sentry/stacktraces/processing.py +++ b/sentry/stacktraces/processing.py @@ -3,11 +3,14 @@ from sentry.utils.safe import get_path def get_crash_frame_from_event_data(data, frame_filter=None): + from issues.utils import get_values # changed by Bugsink + values = get_values(get_path(data, "exception")) + frames = get_path( - data, "exception", "values", -1, "stacktrace", "frames" + values, -1, "stacktrace", "frames" ) or get_path(data, "stacktrace", "frames") if not frames: - threads = get_path(data, "threads", "values") + threads = get_values(get_path(data, "threads")) if threads and len(threads) == 1: frames = get_path(threads, 0, "stacktrace", "frames")