Skip to content

Commit

Permalink
feat: Parse file metadata into reports when incomplete
Browse files Browse the repository at this point in the history
The bare minimum for determining the type of a report and gathering
diagnostic data is for there to be a `crash.error` section of the
underlying report. This is used to determine if the report is
incomplete. Fallback values are distributed and used as needed without
depending on an individual value to determine "incomplete-ness".
  • Loading branch information
kattrali committed Feb 19, 2019
1 parent d42d649 commit d272aed
Show file tree
Hide file tree
Showing 13 changed files with 228 additions and 14 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
Changelog
=========

## TBD

### Enhancements

* Capture basic report diagnostics in the file path in case of crash report
content corruption

## 5.17.3 (2018-12-19)

### Bug Fixes
Expand Down
4 changes: 4 additions & 0 deletions OSX/Bugsnag.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
objects = {

/* Begin PBXBuildFile section */
8A12006C221C50F40008C9C3 /* BSGFilepathTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 8A12006B221C50F40008C9C3 /* BSGFilepathTests.m */; };
8A2C8FAC1C6BC1F700846019 /* Bugsnag.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 8A2C8FA11C6BC1F700846019 /* Bugsnag.framework */; };
8A2C8FCD1C6BC2C800846019 /* Bugsnag.h in Headers */ = {isa = PBXBuildFile; fileRef = 8A2C8FBB1C6BC2C800846019 /* Bugsnag.h */; settings = {ATTRIBUTES = (Public, ); }; };
8A2C8FCE1C6BC2C800846019 /* Bugsnag.m in Sources */ = {isa = PBXBuildFile; fileRef = 8A2C8FBC1C6BC2C800846019 /* Bugsnag.m */; };
Expand Down Expand Up @@ -188,6 +189,7 @@
/* End PBXContainerItemProxy section */

/* Begin PBXFileReference section */
8A12006B221C50F40008C9C3 /* BSGFilepathTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = BSGFilepathTests.m; sourceTree = "<group>"; };
8A2C8FA11C6BC1F700846019 /* Bugsnag.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Bugsnag.framework; sourceTree = BUILT_PRODUCTS_DIR; };
8A2C8FA61C6BC1F700846019 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
8A2C8FAB1C6BC1F700846019 /* Tests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = Tests.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
Expand Down Expand Up @@ -469,6 +471,7 @@
8A2C8FAF1C6BC1F700846019 /* Tests */ = {
isa = PBXGroup;
children = (
8A12006B221C50F40008C9C3 /* BSGFilepathTests.m */,
E7CE78861FD94E40001D07E0 /* KSCrash */,
E791482D1FD82B0C003EFEBF /* BugsnagSessionTest.m */,
E791482B1FD82B0C003EFEBF /* BugsnagSessionTrackerTest.m */,
Expand Down Expand Up @@ -933,6 +936,7 @@
8ACF0F752018136200173809 /* BugsnagCrashReportTests.m in Sources */,
E7CE78CC1FD94E77001D07E0 /* KSSystemInfo_Tests.m in Sources */,
E7CE78C91FD94E77001D07E0 /* KSSignalInfo_Tests.m in Sources */,
8A12006C221C50F40008C9C3 /* BSGFilepathTests.m in Sources */,
E7CE78C61FD94E77001D07E0 /* KSMach_Tests.m in Sources */,
E7CE78D61FD94E9E001D07E0 /* FileBasedTestCase.m in Sources */,
E7CE78D51FD94E93001D07E0 /* XCTestCase+KSCrash.m in Sources */,
Expand Down
16 changes: 16 additions & 0 deletions Source/BugsnagCrashReport.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ NSString *_Nonnull BSGFormatSeverity(BSGSeverity severity);

@interface BugsnagCrashReport : NSObject

/**
* Create a new crash report from a JSON crash report generated by
* BugsnagCrashSentry
*
* @param report a BugsnagCrashSentry JSON report
* @param metadata additional report info encoded as a string
*
* @return a Bugsnag crash report
*/
- (instancetype _Nonnull)initWithKSReport:(NSDictionary *_Nonnull)report
fileMetadata:(NSString *)metadata;

/**
* Create a new crash report from a JSON crash report generated by
* BugsnagCrashSentry
Expand Down Expand Up @@ -193,6 +205,10 @@ __deprecated_msg("Use toJson: instead.");
*/
@property(readwrite, copy, nullable) NSDictionary *appState;

/**
* If YES, a complete report was not able to be obtained at generation time
*/
@property (readonly, nonatomic, getter=isIncomplete) BOOL incomplete;

/**
* Returns the enhanced error message for the thread, or nil if none exists.
Expand Down
73 changes: 67 additions & 6 deletions Source/BugsnagCrashReport.m
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@
}

NSString *_Nonnull BSGParseErrorClass(NSDictionary *error,
NSString *errorType) {
NSString *errorType,
NSString *fallbackValue) {
NSString *errorClass;

if ([errorType isEqualToString:BSGKeyCppException]) {
Expand All @@ -86,7 +87,7 @@
}

if (!errorClass) { // use a default value
errorClass = @"Exception";
errorClass = fallbackValue.length > 0 ? fallbackValue : @"Exception";
}
return errorClass;
}
Expand Down Expand Up @@ -184,6 +185,13 @@ + (instancetype)errorDataFromThreads:(NSArray *)threads;
- (instancetype)initWithClass:(NSString *_Nonnull)errorClass message:(NSString *_Nonnull)errorMessage NS_DESIGNATED_INITIALIZER;
@end

@interface FallbackReportData : NSObject
@property (nonatomic, strong) NSString *errorClass;
@property (nonatomic, getter=isUnhandled) BOOL unhandled;
@property (nonatomic) BSGSeverity severity;
- (instancetype)initWithMetadata:(NSString *)metadata;
@end

@interface BugsnagCrashReport ()

/**
Expand Down Expand Up @@ -212,25 +220,33 @@ @interface BugsnagCrashReport ()
@property(nonatomic, readwrite, copy, nullable) NSDictionary *customException;
@property(nonatomic) BugsnagSession *session;

@property (nonatomic, readwrite, getter=isIncomplete) BOOL incomplete;
@end

@implementation BugsnagCrashReport

- (instancetype)initWithKSReport:(NSDictionary *)report {
return [self initWithKSReport:report fileMetadata:@""];
}

- (instancetype)initWithKSReport:(NSDictionary *)report
fileMetadata:(NSString *)metadata {
if (self = [super init]) {
FallbackReportData *fallback = [[FallbackReportData alloc] initWithMetadata:metadata];
_notifyReleaseStages =
[report valueForKeyPath:@"user.config.notifyReleaseStages"];
_releaseStage = BSGParseReleaseStage(report);

_error = [report valueForKeyPath:@"crash.error"];
_incomplete = report.count == 0;
_errorType = _error[BSGKeyType];
_threads = [report valueForKeyPath:@"crash.threads"];
RegisterErrorData *data = [RegisterErrorData errorDataFromThreads:_threads];
if (data) {
_errorClass = data.errorClass;
_errorClass = data.errorClass ?: fallback.errorClass;
_errorMessage = data.errorMessage;
} else {
_errorClass = BSGParseErrorClass(_error, _errorType);
_errorClass = BSGParseErrorClass(_error, _errorType, fallback.errorClass);
_errorMessage = BSGParseErrorMessage(report, _error, _errorType);
}
_binaryImages = report[@"binary_images"];
Expand Down Expand Up @@ -259,7 +275,7 @@ - (instancetype)initWithKSReport:(NSDictionary *)report {
// only makes sense to use serialised value for handled exceptions
_depth = [[report valueForKeyPath:@"user.state.crash.depth"]
unsignedIntegerValue];
} else { // the event was unhandled.
} else if (_errorType != nil) { // the event was unhandled.
BOOL isSignal = [BSGKeySignal isEqualToString:_errorType];
SeverityReasonType severityReason =
isSignal ? Signal : UnhandledException;
Expand All @@ -268,6 +284,11 @@ - (instancetype)initWithKSReport:(NSDictionary *)report {
severity:BSGSeverityError
attrValue:_errorClass];
_depth = 0;
} else { // Incomplete report
SeverityReasonType severityReason = [fallback isUnhandled] ? UnhandledException : HandledError;
_handledState = [BugsnagHandledState handledStateWithSeverityReason:severityReason
severity:fallback.severity
attrValue:nil];
}
_severity = _handledState.currentSeverity;

Expand Down Expand Up @@ -478,7 +499,11 @@ - (NSDictionary *)toJson {
BSGDictSetSafeObject(event, BSGFormatSeverity(self.severity), BSGKeySeverity);
BSGDictSetSafeObject(event, [self breadcrumbs], BSGKeyBreadcrumbs);
BSGDictSetSafeObject(event, metaData, BSGKeyMetaData);


if ([self isIncomplete]) {
BSGDictSetSafeObject(event, @YES, BSGKeyIncomplete);
}

NSDictionary *device = [self.device bsg_mergedInto:self.deviceState];
BSGDictSetSafeObject(event, device, BSGKeyDevice);

Expand Down Expand Up @@ -620,6 +645,42 @@ - (NSString *_Nullable)enhancedErrorMessageForThread:(NSDictionary *_Nullable)th

@end

@implementation FallbackReportData

- (instancetype)initWithMetadata:(NSString *)metadata {
if (self = [super init]) {
NSString *separator = @"-";
NSString *location = metadata;
NSRange range = [location rangeOfString:separator options:NSBackwardsSearch];
if (range.location != NSNotFound) {
_errorClass = [location substringFromIndex:range.location + 1];
location = [location substringToIndex:range.location];
}
range = [location rangeOfString:separator options:NSBackwardsSearch];
if (range.location != NSNotFound) {
NSString *value = [location substringFromIndex:range.location + 1];
_unhandled = ![value isEqualToString:@"h"];
location = [location substringToIndex:range.location + 1];
} else {
_unhandled = YES;
}
range = [location rangeOfString:separator options:NSBackwardsSearch];
if (range.location != NSNotFound) {
NSString *value = [location substringFromIndex:range.location];
if ([value isEqualToString:@"w"]) {
_severity = BSGSeverityWarning;
} else if ([value isEqualToString:@"i"]) {
_severity = BSGSeverityInfo;
} else {
_severity = BSGSeverityError;
}
}
}
return self;
}

@end

@implementation RegisterErrorData
+ (instancetype)errorDataFromThreads:(NSArray *)threads {
for (NSDictionary *thread in threads) {
Expand Down
1 change: 1 addition & 0 deletions Source/BugsnagKeys.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ static NSString *const BSGKeyName = @"name";
static NSString *const BSGKeyTimestamp = @"timestamp";
static NSString *const BSGKeyType = @"type";
static NSString *const BSGKeyMetaData = @"metaData";
static NSString *const BSGKeyIncomplete = @"incomplete";
static NSString *const BSGKeyId = @"id";
static NSString *const BSGKeyUser = @"user";
static NSString *const BSGKeyEmail = @"email";
Expand Down
13 changes: 8 additions & 5 deletions Source/BugsnagSink.m
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,13 @@ - (void)filterReports:(NSDictionary <NSString *, NSDictionary *> *)reports
NSMutableArray *bugsnagReports = [NSMutableArray new];
BugsnagConfiguration *configuration = [Bugsnag configuration];

for (NSDictionary *report in reports) {
BugsnagCrashReport *bugsnagReport = [[BugsnagCrashReport alloc] initWithKSReport:report];
BOOL incompleteReport = (![@"standard" isEqualToString:[report valueForKeyPath:@"report.type"]] ||
[[report objectForKey:@"incomplete"] boolValue]);
for (NSString *fileKey in reports) {
NSDictionary *report = reports[fileKey];
BugsnagCrashReport *bugsnagReport = [[BugsnagCrashReport alloc] initWithKSReport:report
fileMetadata:fileKey];
BOOL incompleteReport = ([bugsnagReport isIncomplete]
|| ![@"standard" isEqualToString:[report valueForKeyPath:@"report.type"]]
|| [[report objectForKey:@"incomplete"] boolValue]);

if (incompleteReport) { // append app/device data as this is unlikely to change between sessions
NSDictionary *sysInfo = [BSG_KSSystemInfo systemInfo];
Expand Down Expand Up @@ -120,7 +123,7 @@ - (void)filterReports:(NSDictionary <NSString *, NSDictionary *> *)reports
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
for (BugsnagBeforeNotifyHook hook in configuration.beforeNotifyHooks) {
if (reportData) {
reportData = hook(reports, reportData);
reportData = hook(bugsnagReports, reportData);
} else {
break;
}
Expand Down
9 changes: 7 additions & 2 deletions Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,13 @@ static char *bsg_g_stateFilePath;
#pragma mark - Utility -
// ============================================================================
static const int bsg_filepath_len = 512;
static const int bsg_error_class_filepath_len = 21;

static inline BSG_KSCrash_Context *crashContext(void) {
return &bsg_g_crashReportContext;
}

int bsg_create_filepath(char *base, char filepath[bsg_filepath_len], char severity, char error_class[21]) {
int bsg_create_filepath(char *base, char filepath[bsg_filepath_len], char severity, char error_class[bsg_error_class_filepath_len]) {
int length;
for (length = 0; length < bsg_filepath_len; length++) {
if (base[length] == '\0') {
Expand All @@ -87,10 +88,13 @@ int bsg_create_filepath(char *base, char filepath[bsg_filepath_len], char severi
filepath[length++] = '-';
filepath[length++] = context->crash.crashType == BSG_KSCrashTypeUserReported ? 'h' : 'u';
filepath[length++] = '-';
for (int i = 0; i < 21; i++) {
for (int i = 0; i < bsg_error_class_filepath_len; i++) {
char c = error_class[i];
if (c == '\0')
break;
else if (c == 47 || c > 126 || c <= 0)
// disallow '/' and characters outside of the ascii range
continue;
filepath[length++] = c;
}
// add suffix
Expand All @@ -99,6 +103,7 @@ int bsg_create_filepath(char *base, char filepath[bsg_filepath_len], char severi
filepath[length++] = 's';
filepath[length++] = 'o';
filepath[length++] = 'n';
filepath[length++] = '\0';

return length;
}
Expand Down
50 changes: 50 additions & 0 deletions Tests/BSGFilepathTests.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//
// BSGFilepathTests.m
// Tests
//
// Created by Delisa on 2/19/19.
// Copyright © 2019 Bugsnag. All rights reserved.
//

#import <XCTest/XCTest.h>
#include <string.h>


int bsg_create_filepath(char *base, char filepath[512], char severity, char error_class[21]);

@interface BSGFilepathTests : XCTestCase

@end

@implementation BSGFilepathTests

- (void)testEncodeCharacters {
char *base = "/path/to/it/imagine this is a UUID.json";
char filepath[512];
bsg_create_filepath(base, filepath, 'w', "😃 HappyError");
XCTAssertEqual(0, strcmp(filepath, "/path/to/it/imagine this is a UUID-w-u- HappyError.json"));
}

- (void)testTruncateUnicodeCharacters {
char *base = "/path/to/it/imagine this is a UUID.json";
char filepath[512];
// The char limit is not on a character boundary
bsg_create_filepath(base, filepath, 'e', "AnExtremelyLongLong😃");
XCTAssertEqual(0, strcmp(filepath, "/path/to/it/imagine this is a UUID-e-u-AnExtremelyLongLong.json"));
}

- (void)testEmptyErrorClassFromUnicode {
char *base = "/path/to/it/imagine this is a UUID.json";
char filepath[512];
bsg_create_filepath(base, filepath, 'e', "🀦🀨🁺😃");
XCTAssertEqual(0, strcmp(filepath, "/path/to/it/imagine this is a UUID-e-u-.json"));
}

- (void)testErrorClassLength {
char *base = "imagine this is a UUID.json";
char filepath[512];
bsg_create_filepath(base, filepath, 'i', "AnExtremelyLongLongErrorNameOmg");
XCTAssertEqual(0, strcmp(filepath, "imagine this is a UUID-i-u-AnExtremelyLongLongEr.json"));
}

@end
Loading

0 comments on commit d272aed

Please sign in to comment.