Skip to content

Commit b806079

Browse files
fix(sdk_crashes): KeyError for multiple exceptions (#97829)
Only keep the last exception when a crash has multiple ones. The SDK crash detection only detects crashes based on the last exception. Keeping multiple exceptions and stripping data would be the ideal solution, but that requires more changes. We're going to follow up to make this work with #97830.
1 parent 3fa39f0 commit b806079

File tree

2 files changed

+95
-5
lines changed

2 files changed

+95
-5
lines changed

src/sentry/utils/sdk_crashes/event_stripper.py

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,16 +114,32 @@ def strip_event_data(
114114
and the method only keeps SDK frames and system library frames.
115115
"""
116116

117-
frames = get_path(event_data, "exception", "values", -1, "stacktrace", "frames")
118-
if not frames:
117+
last_exception = get_path(
118+
event_data,
119+
"exception",
120+
"values",
121+
-1,
122+
)
123+
if not last_exception:
119124
return {}
120125

126+
# The SDK crash detector only detects crashes based on the last exception value.
127+
# Therefore, if the last exception doesn't contain a stacktrace something is off and
128+
# we can drop the whole event.
129+
last_exception_frames = get_path(last_exception, "stacktrace", "frames")
130+
if not last_exception_frames:
131+
return {}
132+
133+
event_data_copy = dict(event_data)
134+
121135
# We strip the frames first because applying the allowlist removes fields that are needed
122136
# for deciding wether to keep a frame or not.
123-
stripped_frames = _strip_frames(frames, sdk_crash_detector)
137+
stripped_frames = _strip_frames(last_exception_frames, sdk_crash_detector)
138+
last_exception["stacktrace"]["frames"] = stripped_frames
124139

125-
event_data_copy = dict(event_data)
126-
event_data_copy["exception"]["values"][0]["stacktrace"]["frames"] = stripped_frames
140+
# We only keep the last exception. We need to adopt the allowlist and stripping event data logic
141+
# to support multiple exceptions.
142+
event_data_copy["exception"]["values"] = [last_exception]
127143

128144
stripped_event_data = _strip_event_data_with_allowlist(event_data_copy, EVENT_DATA_ALLOWLIST)
129145

tests/sentry/utils/sdk_crashes/test_event_stripper.py

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,3 +452,77 @@ def test_strip_event_without_frames_returns_empty_dict(store_and_strip_event) ->
452452
stripped_event_data = store_and_strip_event(data=event_data)
453453

454454
assert stripped_event_data == {}
455+
456+
457+
@django_db_all
458+
@pytest.mark.snuba
459+
def test_strip_event_with_multiple_exceptions_only_keep_last_one(store_and_strip_event) -> None:
460+
event_data = get_crash_event()
461+
462+
exception_values = list(get_path(event_data, "exception", "values"))
463+
464+
crash_exception = dict(exception_values[0])
465+
set_path(crash_exception, "type", value="SIGPIPE")
466+
set_path(crash_exception, "value", value="Broken pipe")
467+
468+
exception_values.append(crash_exception)
469+
470+
set_path(event_data, "exception", "values", value=exception_values)
471+
472+
stripped_event_data = store_and_strip_event(data=event_data)
473+
474+
assert len(get_path(stripped_event_data, "exception", "values")) == 1
475+
476+
exception = get_path(stripped_event_data, "exception", "values", 0)
477+
assert exception["type"] == "SIGPIPE"
478+
assert "value" not in exception
479+
480+
481+
@django_db_all
482+
@pytest.mark.snuba
483+
def test_strip_event_with_multiple_exceptions_last_without_frames_discard_event(
484+
store_and_strip_event,
485+
) -> None:
486+
event_data = get_crash_event()
487+
488+
exception_values = list(get_path(event_data, "exception", "values"))
489+
490+
crash_exception = dict(exception_values[0])
491+
set_path(crash_exception, "type", value="SIGPIPE")
492+
set_path(crash_exception, "value", value="Broken pipe")
493+
set_path(crash_exception, "stacktrace", value=None)
494+
exception_values.append(crash_exception)
495+
496+
set_path(event_data, "exception", "values", value=exception_values)
497+
498+
stripped_event_data = store_and_strip_event(data=event_data)
499+
500+
assert stripped_event_data == {}
501+
502+
503+
@django_db_all
504+
@pytest.mark.snuba
505+
def test_strip_event_with_multiple_exceptions_first_without_frames_keeps_last_exception(
506+
store_and_strip_event,
507+
) -> None:
508+
event_data = get_crash_event()
509+
510+
exception_values = list(get_path(event_data, "exception", "values"))
511+
512+
crash_exception = dict(exception_values[0])
513+
set_path(crash_exception, "type", value="SIGPIPE")
514+
set_path(crash_exception, "value", value="Broken pipe")
515+
exception_values.append(crash_exception)
516+
517+
# Remove the stacktrace from the first exception.
518+
exception_values[0]["stacktrace"] = None
519+
520+
set_path(event_data, "exception", "values", value=exception_values)
521+
522+
stripped_event_data = store_and_strip_event(data=event_data)
523+
524+
assert len(get_path(stripped_event_data, "exception", "values")) == 1
525+
526+
exception = get_path(stripped_event_data, "exception", "values", 0)
527+
assert exception["type"] == "SIGPIPE"
528+
assert "value" not in exception

0 commit comments

Comments
 (0)