-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(proguard): Remap exception names in proguard #15795
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,9 @@ | |
|
||
logger = logging.getLogger(__name__) | ||
|
||
StacktraceInfo = namedtuple("StacktraceInfo", ["stacktrace", "container", "platforms"]) | ||
StacktraceInfo = namedtuple( | ||
"StacktraceInfo", ["stacktrace", "container", "platforms", "is_exception"] | ||
) | ||
StacktraceInfo.__hash__ = lambda x: id(x) | ||
StacktraceInfo.__eq__ = lambda a, b: a is b | ||
StacktraceInfo.__ne__ = lambda a, b: a is not b | ||
|
@@ -145,6 +147,10 @@ def preprocess_frame(self, processable_frame): | |
""" | ||
pass | ||
|
||
def process_exception(self, exception): | ||
"""Processes an exception.""" | ||
return False | ||
|
||
def process_frame(self, processable_frame, processing_task): | ||
"""Processes the processable frame and returns a tuple of three | ||
lists: ``(frames, raw_frames, errors)`` where frames is the list of | ||
|
@@ -162,26 +168,36 @@ def preprocess_step(self, processing_task): | |
return False | ||
|
||
|
||
def find_stacktraces_in_data(data, include_raw=False): | ||
def find_stacktraces_in_data(data, include_raw=False, with_exceptions=False): | ||
"""Finds all stracktraces in a given data blob and returns it | ||
together with some meta information. | ||
|
||
If `include_raw` is True, then also raw stacktraces are included. | ||
If `include_raw` is True, then also raw stacktraces are included. If | ||
`with_exceptions` is set to `True` then stacktraces of the exception | ||
are always included and the `is_exception` flag is set on that stack | ||
info object. | ||
""" | ||
rv = [] | ||
|
||
def _report_stack(stacktrace, container): | ||
if not stacktrace or not get_path(stacktrace, "frames", filter=True): | ||
def _report_stack(stacktrace, container, is_exception=False): | ||
if not is_exception and (not stacktrace or not get_path(stacktrace, "frames", filter=True)): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change is indeed confusing as we're now reporting non-existing stack traces just to get to the exception container. I also don't have a better idea, since I suppose you need the stack frame's platform to decide whether you want to touch the container exception. To be safe, you could default As an alternative, could we add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to change the entire thing to return stacks + related data and treat missing stacks as empty stacks. |
||
return | ||
|
||
platforms = set( | ||
frame.get("platform") or data.get("platform") | ||
for frame in get_path(stacktrace, "frames", filter=True, default=()) | ||
) | ||
rv.append(StacktraceInfo(stacktrace=stacktrace, container=container, platforms=platforms)) | ||
rv.append( | ||
StacktraceInfo( | ||
stacktrace=stacktrace, | ||
container=container, | ||
platforms=platforms, | ||
is_exception=is_exception, | ||
) | ||
) | ||
|
||
for exc in get_path(data, "exception", "values", filter=True, default=()): | ||
_report_stack(exc.get("stacktrace"), exc) | ||
_report_stack(exc.get("stacktrace"), exc, is_exception=with_exceptions) | ||
|
||
_report_stack(data.get("stacktrace"), None) | ||
|
||
|
@@ -278,7 +294,7 @@ def normalize_stacktraces_for_grouping(data, grouping_config=None): | |
def should_process_for_stacktraces(data): | ||
from sentry.plugins.base import plugins | ||
|
||
infos = find_stacktraces_in_data(data) | ||
infos = find_stacktraces_in_data(data, with_exceptions=True) | ||
platforms = set() | ||
for info in infos: | ||
platforms.update(info.platforms or ()) | ||
|
@@ -462,7 +478,7 @@ def dedup_errors(errors): | |
|
||
|
||
def process_stacktraces(data, make_processors=None, set_raw_stacktrace=True): | ||
infos = find_stacktraces_in_data(data) | ||
infos = find_stacktraces_in_data(data, with_exceptions=True) | ||
if make_processors is None: | ||
processors = get_processors_for_stacktraces(data, infos) | ||
else: | ||
|
@@ -486,6 +502,15 @@ def process_stacktraces(data, make_processors=None, set_raw_stacktrace=True): | |
|
||
# Process all stacktraces | ||
for stacktrace_info, processable_frames in processing_task.iter_processable_stacktraces(): | ||
# Let the stacktrace processors touch the exception | ||
if stacktrace_info.is_exception and stacktrace_info.container: | ||
for processor in processing_task.iter_processors(): | ||
if processor.process_exception(stacktrace_info.container): | ||
changed = True | ||
|
||
# If the stacktrace is empty we skip it for processing | ||
if not stacktrace_info.stacktrace: | ||
continue | ||
new_frames, new_raw_frames, errors = process_single_stacktrace( | ||
processing_task, stacktrace_info, processable_frames | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this work with generics that contain class paths?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea. At least not different to how we already do with methods