Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sdk-crashes): Improve Cocoa crash detection #52119

Merged
merged 2 commits into from
Jul 4, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions src/sentry/utils/sdk_crashes/cocoa_sdk_crash_detector.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ def is_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool:
# The frames are ordered from caller to callee, or oldest to youngest.
# The last frame is the one creating the exception.
# Therefore, we must iterate in reverse order.
# In a first iteration of this algorithm, we checked for in_app frames, but
# customers can change the in_app configuration, so we can't rely on that.
# Furthermore, if they use static linking for including Sentry Cocoa, Cocoa SDK
# frames can be marked as in_app. Therefore, the algorithm only checks if frames
# are SDK frames or from system libraries.
for frame in reversed(frames):
# [SentrySDK crash] is a testing function causing a crash.
# Therefore, we don't want to mark it a as a SDK crash.
Expand All @@ -22,7 +27,7 @@ def is_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool:
if self.is_sdk_frame(frame):
return True

if frame.get("in_app") is True:
if not self.is_system_library_frame(frame):
return False

return False
Expand All @@ -31,7 +36,12 @@ def is_sdk_frame(self, frame: Mapping[str, Any]) -> bool:

function = frame.get("function")
if function:
function_matchers = ["*sentrycrash*", "**[[]Sentry*"]
function_matchers = [
"*sentrycrash*",
"**[[]Sentry*",
"*(Sentry*)*", # Objective-C class extension categories
"SentryMX*", # MetricKit Swift classes
]
for matcher in function_matchers:
philipphofmann marked this conversation as resolved.
Show resolved Hide resolved
if glob_match(function, matcher, ignorecase=True):
return True
Expand All @@ -44,3 +54,14 @@ def is_sdk_frame(self, frame: Mapping[str, Any]) -> bool:
return True

return False

def is_system_library_frame(self, frame: Mapping[str, Any]) -> bool:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this function from event_stripper to there.

system_library_paths = {"/System/Library/", "/usr/lib/"}

for field in self.fields_containing_paths:
for system_library_path in system_library_paths:
field_with_path = frame.get(field)
if field_with_path and field_with_path.startswith(system_library_path):
return True

return False
17 changes: 3 additions & 14 deletions src/sentry/utils/sdk_crashes/event_stripper.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,24 +153,12 @@ def _strip_frames(
We need to adapt this logic once we support other platforms.
"""

fields_containing_paths = {"package", "module", "abs_path"}

def is_system_library(frame: Mapping[str, Any]) -> bool:
system_library_paths = {"/System/Library/", "/usr/lib/system/"}

for field in fields_containing_paths:
for path in system_library_paths:
if frame.get(field, "").startswith(path):
return True

return False

def strip_frame(frame: MutableMapping[str, Any]) -> MutableMapping[str, Any]:
if sdk_crash_detector.is_sdk_frame(frame):
frame["in_app"] = True

# The path field usually contains the name of the application, which we can't keep.
for field in fields_containing_paths:
for field in sdk_crash_detector.fields_containing_paths:
if frame.get(field):
frame[field] = "Sentry.framework"
else:
Expand All @@ -181,5 +169,6 @@ def strip_frame(frame: MutableMapping[str, Any]) -> MutableMapping[str, Any]:
return [
strip_frame(frame)
for frame in frames
if sdk_crash_detector.is_sdk_frame(frame) or is_system_library(frame)
if sdk_crash_detector.is_sdk_frame(frame)
or sdk_crash_detector.is_system_library_frame(frame)
]
10 changes: 9 additions & 1 deletion src/sentry/utils/sdk_crashes/sdk_crash_detector.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
from abc import ABC, abstractmethod
from typing import Any, Mapping, Sequence
from typing import Any, Mapping, Sequence, Set


class SDKCrashDetector(ABC):
@property
def fields_containing_paths(self) -> Set[str]:
return {"package", "module", "abs_path"}

@abstractmethod
def is_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool:
"""
Expand All @@ -15,3 +19,7 @@ def is_sdk_crash(self, frames: Sequence[Mapping[str, Any]]) -> bool:
@abstractmethod
def is_sdk_frame(self, frame: Mapping[str, Any]) -> bool:
raise NotImplementedError

@abstractmethod
def is_system_library_frame(self, frame: Mapping[str, Any]) -> bool:
raise NotImplementedError
Loading