Add fallback metadata in the event of report file corruption #327
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kattrali
changed the title
kattrali
force-pushed
the
kattrali/low-disk-space-optim
branch
from
d272aed to
6911165
Compare
|
|
||
|
|
||
| if ([self isIncomplete]) { | ||
| BSGDictSetSafeObject(event, @YES, BSGKeyIncomplete); |
There was a problem hiding this comment.
Do we want to include the field if it's false also? This would allow filtering on the value either way
| @@ -620,6 +645,42 @@ - (NSString *_Nullable)enhancedErrorMessageForThread:(NSDictionary *_Nullable)th | |||
|
|
|||
| @end | |||
|
|
|||
| @implementation FallbackReportData | |||
|
|
|||
| - (instancetype)initWithMetadata:(NSString *)metadata { | |||
There was a problem hiding this comment.
It'd be nice to separate this out into individual methods to improve readability (e.g. one that returns the severity, another that returns the handled state, etc).
Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_Deadlock.m
Show resolved
Hide resolved
Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_Signal.c
Show resolved
Hide resolved
Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_User.c
Outdated
Show resolved
Hide resolved
kattrali
force-pushed
the
kattrali/low-disk-space-optim
branch
from
6911165 to
e849ff2
Compare
This change formats report file names to include minimal metadata: * Add accessor for report contents indexed by filename * Add async-safe customized filepath generator * Update the callbacks to explicitly include the report count Incomplete reports have been corrupted or otherwise could not be written, and thus have no content in the report data dictionary. Fallback values are parsed from the file name metadata and used as needed without depending on an individual value to determine "incomplete-ness".
kattrali
force-pushed
the
kattrali/low-disk-space-optim
branch
from
e849ff2 to
fdb769b
Compare
The change to filenames means that both crashes can now be reported since they will use different paths and avoid overwriting the other.
CodeHex
reviewed
Feb 20, 2019
There was a problem hiding this comment.
Looking through the changes, it seems OK to me but I'm not very familiar with codebase. Tried this locally by forcing the reports to be incomplete in various ways with various exceptions. Verified that the incomplete flag is set properly in the pipeline. LGTM once the tests pass
CodeHex
approved these changes
Feb 20, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goal
Capture and report basic report metadata in the event that a crash report file is corrupted or otherwise could not be written.
Design
The file path calculation logic is altered to include the severity, handled-ness, and first few characters of the error class, which are parsed and used in the event that the file cannot be read.
This change refactors the
sendReports/filterReportsparameter types to include the file path, which required some alteration to theSinkandApiClientcallback type signatures since an array was no longer being passed around (and we only used the count of the array anyhow).Tests