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

Formats full crash reports with extra diagnostic information #258

Merged
merged 1 commit into from
Feb 16, 2023

Conversation

matux
Copy link
Collaborator

@matux matux commented Feb 15, 2023

Description of the change

This formatter will generate a valid Apple crash report version 104 using the extended diagnostic information generated by our diagnostics filter.

+9000 lines correspond to a crash reports used in unit tests. Another ~200 to Xcode related project files such as schemes, hopefully knowing this makes the PR less scary.


RollbarCrashFormattingFilter class is our hook into KSCrash report post-crash processing that happens the next time the app is opened after a crash occurred, and before sending the report to Rollbar.

The formatting occurs after the RollbarCrashDiagnosticFilter is done extracting all the extra diagnostic information we use.

The entry point is the filterReports function. We receive an array of reports, each report is a hashmap (a Dictionary in Apple parlance), that we validate for correctness and then format by producing by making use of an internal micro-DSL that helps us format the report with which must simplicity, we then send back the report as an NSString to the KSCrash processing pipeline, where we eventually receive it and send it to Rollbar.

Diagnostic/RollbarCrashFormattingFilter.swift would be a good place to start a review.


The formatter uses a mini-DSL created for the purpose of building complex multiline strings in a simple, straightforward way, the DSL is defined in FormatBuilder.swift.

Files in the Report dir correspond to the data structures in the json report. Not all objects in the json structure have a defined data struct.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Maintenance
  • New release

Related issues

Shortcut stories and GitHub issues (delete irrelevant)

Checklists

Development

  • Lint rules pass locally
  • The code changed/added as part of this pull request has been covered with tests
  • All tests related to the changed code pass in development

Code review

  • This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
  • "Ready for review" label attached to the PR and reviewers assigned
  • Issue from task tracker has a link to this pull request
  • Changes have been reviewed by at least one other engineer

@matux matux self-assigned this Feb 15, 2023
@matux matux changed the base branch from master to collect_crashes_with_stacktraces February 15, 2023 19:06
@matux matux requested a review from diegov February 16, 2023 03:33
@matux matux marked this pull request as ready for review February 16, 2023 03:34
Copy link

@tarsolya tarsolya left a comment

Choose a reason for hiding this comment

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

@matux It was a long time ago when I was working with Swift the last time, so I can't attest to the language "correctness", but the the functionality and the new format of crash reports with the additional diag info was pretty straightforward to understand for me.

I defer back to you (or someone else with more up to date swift exp) on language / swift specific technicalities.

@matux
Copy link
Collaborator Author

matux commented Feb 16, 2023

@tarsolya Much appreciated!

@matux matux merged commit 63cbf19 into collect_crashes_with_stacktraces Feb 16, 2023
@matux matux deleted the formatter branch February 16, 2023 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants