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

Parse notable addresses for Swift Assertion failure messages #188

Merged
merged 6 commits into from
Oct 12, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
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
7 changes: 7 additions & 0 deletions Source/BugsnagCrashReport.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,4 +187,11 @@ initWithErrorName:(NSString *_Nonnull)name
* Device state such as oreground status and run duration
*/
@property(nonatomic, readwrite, copy, nullable) NSDictionary *appState;


/**
* Returns the enhanced error message for the thread, or nil if none exists.
*/
- (NSString *_Nullable)enhancedErrorMessageForThread:(NSDictionary *_Nullable)thread;

@end
60 changes: 58 additions & 2 deletions Source/BugsnagCrashReport.m
Original file line number Diff line number Diff line change
Expand Up @@ -540,8 +540,15 @@ - (NSArray *)serializeThreadsWithException:(NSMutableDictionary *)exception {
for (NSDictionary *thread in [self threads]) {
NSArray *backtrace = thread[@"backtrace"][@"contents"];
BOOL stackOverflow = [thread[@"stack"][@"overflow"] boolValue];

if ([thread[@"crashed"] boolValue]) {
BOOL isCrashedThread = [thread[@"crashed"] boolValue];

if (isCrashedThread) {
NSString *errMsg = [self enhancedErrorMessageForThread:thread];

if (errMsg) { // use enhanced error message (currently swift assertions)
BSGDictInsertIfNotNil(exception, errMsg, @"message");
}

NSUInteger seen = 0;
NSMutableArray *stacktrace = [NSMutableArray array];

Expand Down Expand Up @@ -587,4 +594,53 @@ - (NSArray *)serializeThreadsWithException:(NSMutableDictionary *)exception {
return bugsnagThreads;
}

/**
* Returns the enhanced error message for the thread, or nil if none exists.
*
* This relies very heavily on heuristics rather than any documented APIs.
*/
- (NSString *)enhancedErrorMessageForThread:(NSDictionary *)thread {
NSDictionary *notableAddresses = thread[@"notable_addresses"];
NSMutableArray *msgBuffer = [NSMutableArray new];
BOOL hasReservedWord = NO;

if (notableAddresses) {
for (NSString *key in notableAddresses) {
if (![key hasPrefix:@"stack"]) { // skip stack frames, only use register values
NSDictionary *data = notableAddresses[key];
NSString *contentValue = data[@"value"];

hasReservedWord = hasReservedWord || [self isReservedWord:contentValue];

// must be a string that isn't a reserved word and isn't a filepath
if ([@"string" isEqualToString:data[@"type"]]
&& ![self isReservedWord:contentValue]
&& !([[contentValue componentsSeparatedByString:@"/"] count] > 2)) {

[msgBuffer addObject:contentValue];
}
}
}
[msgBuffer sortUsingSelector:@selector(localizedCaseInsensitiveCompare:)];
}

if (hasReservedWord && [msgBuffer count] > 0) { // needs to have a reserved word used + a message
return [msgBuffer componentsJoinedByString:@" | "];
} else {
return nil;
}
}

/**
* Determines whether a string is a "reserved word" that identifies it as a known value.
*
* For fatalError, preconditionFailure, and assertionFailure, "fatal error" will be in one of the registers.
*
* For assert, "assertion failed" will be in one of the registers.
*/
- (BOOL)isReservedWord:(NSString *)contentValue {
return [@"assertion failed" isEqualToString:contentValue]
|| [@"fatal error" isEqualToString:contentValue];
}

@end
3 changes: 1 addition & 2 deletions Source/BugsnagCrashSentry.m
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ - (void)install:(BugsnagConfiguration *)config

BugsnagSink *sink = [[BugsnagSink alloc] initWithApiClient:apiClient];
[BSG_KSCrash sharedInstance].sink = sink;
// We don't use this feature yet, so we turn it off
[BSG_KSCrash sharedInstance].introspectMemory = NO;
[BSG_KSCrash sharedInstance].introspectMemory = YES;
[BSG_KSCrash sharedInstance].deleteBehaviorAfterSendAll =
BSG_KSCDeleteOnSucess;
[BSG_KSCrash sharedInstance].onCrash = onCrash;
Expand Down
69 changes: 69 additions & 0 deletions Tests/BugsnagCrashReportTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -127,4 +127,73 @@ - (void)testNotifyReleaseStagesSkipsSendFromConfig {
XCTAssertFalse([report shouldBeSent]);
}

- (void)testEnhancedErrorMessage {
BugsnagCrashReport *errorReport = [BugsnagCrashReport new];
NSMutableDictionary *thread = [NSMutableDictionary new];

// nil for empty threads
XCTAssertNil([errorReport enhancedErrorMessageForThread:thread]);
NSMutableDictionary *addresses = [NSMutableDictionary new];

// nil for empty notable addresses
thread[@"notable_addresses"] = addresses;
XCTAssertNil([errorReport enhancedErrorMessageForThread:thread]);

// nil for "fatal error" with no additional dict present

for (NSString *reservedWord in @[@"fatal error", @"assertion failed"]) {
addresses[@"r14"] = @{
@"address": @4511089532,
@"type": @"string",
@"value": reservedWord
};
XCTAssertNil([errorReport enhancedErrorMessageForThread:thread]);
}

// returns msg for "fatal error" with additional dict present
addresses[@"r12"] = @{
@"address": @4511086448,
@"type": @"string",
@"value": @"Whoops - fatalerror"
};
XCTAssertEqualObjects(@"Whoops - fatalerror", [errorReport enhancedErrorMessageForThread:thread]);


// ignores additional dict if more than 2 "/" present
addresses[@"r24"] = @{
@"address": @4511084983,
@"type": @"string",
@"value": @"/usr/include/lib/something.swift"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any value in including these addresses in other metadata?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Quite possibly - I'll look into what sort of values are available for our Swift/Obj-C examples.

};
XCTAssertEqualObjects(@"Whoops - fatalerror", [errorReport enhancedErrorMessageForThread:thread]);

// ignores dict if not type string
addresses[@"r25"] = @{
@"address": @4511084983,
@"type": @"long",
@"value": @"Swift is hard"
};
XCTAssertEqualObjects(@"Whoops - fatalerror", [errorReport enhancedErrorMessageForThread:thread]);

// sorts and concatenates multiple multiple messages
addresses[@"r26"] = @{
@"address": @4511082095,
@"type": @"string",
@"value": @"Swift is hard"
};
XCTAssertEqualObjects(@"Swift is hard | Whoops - fatalerror", [errorReport enhancedErrorMessageForThread:thread]);

// ignores stack frames
addresses[@"stack523409"] = @{
@"address": @4511080001,
@"type": @"string",
@"value": @"Not a register"
};
XCTAssertEqualObjects(@"Swift is hard | Whoops - fatalerror", [errorReport enhancedErrorMessageForThread:thread]);

// ignores values if no reserved word used
addresses[@"r14"] = nil;
XCTAssertNil([errorReport enhancedErrorMessageForThread:thread]);
}

@end
4 changes: 2 additions & 2 deletions examples/swift-ios/Podfile.lock
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
PODS:
- Bugsnag (5.11.2)
- Bugsnag (5.12.1)

DEPENDENCIES:
- Bugsnag (from `../..`)
Expand All @@ -9,7 +9,7 @@ EXTERNAL SOURCES:
:path: ../..

SPEC CHECKSUMS:
Bugsnag: daab816e964545823e9f9b826486bd30c67f22da
Bugsnag: c8b1ccbb22acf100ce5b496bdc223be3184c7a57

PODFILE CHECKSUM: 2107babfbfdb18f0288407b9d9ebd48cbee8661c

Expand Down