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.

Sign up for GitHub

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

Jump to bottom

fix: Record handled report info during initial write #322

Merged
merged 1 commit into from
Dec 19, 2018
Merged

fix: Record handled report info during initial write #322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ Changelog

### Bug Fixes

* Fix case where notify() causes an unhandled report
[#322](https://github.com/bugsnag/bugsnag-cocoa/pull/322)

* Fix possible crash when fetching system info to append to a crash report
[#321](https://github.com/bugsnag/bugsnag-cocoa/pull/321)

Expand Down
8 changes: 7 additions & 1 deletion Source/BugsnagCrashSentry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@
onCrash:(BSGReportCallback)onCrash;

- (void)reportUserException:(NSString *)reportName
reason:(NSString *)reportMessage;
reason:(NSString *)reportMessage
handledState:(NSDictionary *)handledState
appState:(NSDictionary *)appState
callbackOverrides:(NSDictionary *)overrides
metadata:(NSDictionary *)metadata
config:(NSDictionary *)config
discardDepth:(int)depth;

@end
17 changes: 13 additions & 4 deletions Source/BugsnagCrashSentry.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,22 @@ - (void)install:(BugsnagConfiguration *)config
}

- (void)reportUserException:(NSString *)reportName
reason:(NSString *)reportMessage {
reason:(NSString *)reportMessage
handledState:(NSDictionary *)handledState
appState:(NSDictionary *)appState
callbackOverrides:(NSDictionary *)overrides
metadata:(NSDictionary *)metadata
config:(NSDictionary *)config
discardDepth:(int)depth {

[[BSG_KSCrash sharedInstance] reportUserException:reportName
reason:reportMessage
language:NULL
lineOfCode:@""
stackTrace:@[]
handledState:handledState
appState:appState
callbackOverrides:overrides
metadata:metadata
config:config
discardDepth:depth
terminateProgram:NO];
}

Expand Down
67 changes: 19 additions & 48 deletions Source/BugsnagNotifier.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,37 +82,23 @@
* @param writer report writer which will receive updated metadata
*/
void BSSerializeDataCrashHandler(const BSG_KSCrashReportWriter *writer, int type) {
BOOL userReported = BSG_KSCrashTypeUserReported == type;

BOOL isCrash = BSG_KSCrashTypeUserReported != type;
if (hasRecordedSessions) { // a session is available
// persist session info
writer->addStringElement(writer, "id", (const char *) sessionId);
writer->addStringElement(writer, "startedAt", (const char *) sessionStartDate);
writer->addUIntegerElement(writer, "handledCount", handledCount);

if (!userReported) {
writer->addUIntegerElement(writer, "unhandledCount", 1);
} else {
writer->addUIntegerElement(writer, "unhandledCount", 0);
}
}
if (bsg_g_bugsnag_data.configJSON) {
writer->addJSONElement(writer, "config", bsg_g_bugsnag_data.configJSON);
}
if (bsg_g_bugsnag_data.stateJSON) {
writer->addJSONElement(writer, "state", bsg_g_bugsnag_data.stateJSON);
}
if (bsg_g_bugsnag_data.metaDataJSON) {
writer->addJSONElement(writer, "metaData", bsg_g_bugsnag_data.metaDataJSON);
writer->addUIntegerElement(writer, "unhandledCount", isCrash ? 1 : 0);
}

// write additional user-supplied metadata
if (userReported) {
if (bsg_g_bugsnag_data.handledState) {
writer->addJSONElement(writer, "handledState", bsg_g_bugsnag_data.handledState);
if (isCrash) {
if (bsg_g_bugsnag_data.configJSON) {
writer->addJSONElement(writer, "config", bsg_g_bugsnag_data.configJSON);
}
if (bsg_g_bugsnag_data.stateJSON) {
writer->addJSONElement(writer, "state", bsg_g_bugsnag_data.stateJSON);
}
if (bsg_g_bugsnag_data.userOverridesJSON) {
writer->addJSONElement(writer, "overrides", bsg_g_bugsnag_data.userOverridesJSON);
if (bsg_g_bugsnag_data.metaDataJSON) {
writer->addJSONElement(writer, "metaData", bsg_g_bugsnag_data.metaDataJSON);
}
}

Expand Down Expand Up @@ -537,19 +523,6 @@ - (void)notify:(NSString *)exceptionName
if (block) {
block(report);
}

[self.metaDataLock lock];
BSSerializeJSONDictionary([report.handledState toJson],
&bsg_g_bugsnag_data.handledState);
BSSerializeJSONDictionary(report.metaData,
&bsg_g_bugsnag_data.metaDataJSON);
BSSerializeJSONDictionary(report.overrides,
&bsg_g_bugsnag_data.userOverridesJSON);

[self.state addAttribute:BSGKeySeverity
withValue:BSGFormatSeverity(report.severity)
toTabWithName:BSTabCrash];

// We discard 5 stack frames (including this one) by default,
// and sum that with the number specified by report.depth:
//
Expand All @@ -560,23 +533,21 @@ - (void)notify:(NSString *)exceptionName
// 3 -[BugsnagCrashSentry reportUserException:reason:]
// 4 -[BugsnagNotifier notify:message:block:]

NSNumber *depth = @(BSGNotifierStackFrameCount + report.depth);
[self.state addAttribute:BSAttributeDepth
withValue:depth
toTabWithName:BSTabCrash];
int depth = (int)(BSGNotifierStackFrameCount + report.depth);
kattrali marked this conversation as resolved.
Show resolved Hide resolved

NSString *reportName =
report.errorClass ?: NSStringFromClass([NSException class]);
NSString *reportMessage = report.errorMessage ?: @"";

[self.crashSentry reportUserException:reportName reason:reportMessage];
bsg_g_bugsnag_data.userOverridesJSON = NULL;
bsg_g_bugsnag_data.handledState = NULL;
[self.crashSentry reportUserException:reportName
reason:reportMessage
handledState:[handledState toJson]
appState:[self.state toDictionary]
callbackOverrides:report.overrides
metadata:[report.metaData copy]
config:[self.configuration.config toDictionary]
Copy link
Contributor

Choose a reason for hiding this comment

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

Access to self.configuration.config was previously synchronized behind a lock - is this access also synchronized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously, these values were mutated in this block. Removing the mutation removed the need for a lock.

discardDepth:depth];

// Restore metaData to pre-crash state.
[self.metaDataLock unlock];
kattrali marked this conversation as resolved.
Show resolved Hide resolved
[self metaDataChanged:self.configuration.metaData];
[[self state] clearTab:BSTabCrash];
[self addBreadcrumbWithBlock:^(BugsnagBreadcrumb *_Nonnull crumb) {
crumb.type = BSGBreadcrumbTypeError;
crumb.name = reportName;
Expand Down
27 changes: 14 additions & 13 deletions Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,24 +172,25 @@ typedef enum {
* application will terminate with an abort().
*
* @param name The exception name (for namespacing exception types).
*
* @param reason A description of why the exception occurred.
*
* @param language A unique language identifier.
*
* @param lineOfCode A copy of the offending line of code (nil = ignore).
*
* @param stackTrace An array of frames (dictionaries or strings) representing
* the call stack leading to the exception (nil = ignore).
*
* @param reason A description of why the exception occurred
* @param handledState The severity, reason, and handled-ness of the report
* @param appState breadcrumbs and other app environmental info
* @param overrides Report fields overridden by callbacks, collated in the
* final report
* @param metadata additional information to attach to the report
* @param config delivery options
* @param depth The number of frames to discard from the top of the stacktrace
* @param terminateProgram If true, do not return from this function call.
* Terminate the program instead.
*/
- (void)reportUserException:(NSString *)name
reason:(NSString *)reason
language:(NSString *)language
lineOfCode:(NSString *)lineOfCode
stackTrace:(NSArray *)stackTrace
handledState:(NSDictionary *)handledState
appState:(NSDictionary *)appState
callbackOverrides:(NSDictionary *)overrides
metadata:(NSDictionary *)metadata
config:(NSDictionary *)config
discardDepth:(int)depth
terminateProgram:(BOOL)terminateProgram;

/** If YES, user reported exceptions will suspend all threads during report
Expand Down
48 changes: 27 additions & 21 deletions Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,30 +347,23 @@ - (void)deleteAllReports {

- (void)reportUserException:(NSString *)name
reason:(NSString *)reason
language:(NSString *)language
lineOfCode:(NSString *)lineOfCode
stackTrace:(NSArray *)stackTrace
handledState:(NSDictionary *)handledState
appState:(NSDictionary *)appState
callbackOverrides:(NSDictionary *)overrides
metadata:(NSDictionary *)metadata
config:(NSDictionary *)config
discardDepth:(int)depth
terminateProgram:(BOOL)terminateProgram {
const char *cName = [name cStringUsingEncoding:NSUTF8StringEncoding];
const char *cReason = [reason cStringUsingEncoding:NSUTF8StringEncoding];
const char *cLanguage =
[language cStringUsingEncoding:NSUTF8StringEncoding];
const char *cLineOfCode =
[lineOfCode cStringUsingEncoding:NSUTF8StringEncoding];
NSError *error = nil;
NSData *jsonData =
[BSG_KSJSONCodec encode:stackTrace options:0 error:&error];
if (jsonData == nil || error != nil) {
BSG_KSLOG_ERROR(@"Error encoding stack trace to JSON: %@", error);
// Don't return, since we can still record other useful information.
}
NSString *jsonString =
[[NSString alloc] initWithData:jsonData encoding:NSUTF8StringEncoding];
const char *cStackTrace =
[jsonString cStringUsingEncoding:NSUTF8StringEncoding];

bsg_kscrash_reportUserException(cName, cReason, cLanguage, cLineOfCode,
cStackTrace, terminateProgram);
bsg_kscrash_reportUserException(cName, cReason,
[self encodeAsJSONString:handledState],
[self encodeAsJSONString:overrides],
[self encodeAsJSONString:metadata],
[self encodeAsJSONString:appState],
[self encodeAsJSONString:config],
depth,
terminateProgram);

// If bsg_kscrash_reportUserException() returns, we did not terminate.
// Set up IDs and paths for the next crash.
Expand Down Expand Up @@ -474,6 +467,19 @@ - (NSMutableData *)nullTerminated:(NSData *)data {
return mutable;
}

- (const char *)encodeAsJSONString:(id)object {
NSError *error = nil;
NSData *jsonData = [BSG_KSJSONCodec encode:object options:0 error:&error];
if (jsonData == nil || error != nil) {
BSG_KSLOG_ERROR(@"Error encoding object to JSON: %@", error);
// we can still record other useful information from the report
return NULL;
}
NSString *jsonString =
[[NSString alloc] initWithData:jsonData encoding:NSUTF8StringEncoding];
return [jsonString cStringUsingEncoding:NSUTF8StringEncoding];
}

// ============================================================================
#pragma mark - Callbacks -
// ============================================================================
Expand Down
14 changes: 9 additions & 5 deletions Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,12 +242,16 @@ void bsg_kscrash_setCrashNotifyCallback(
}

void bsg_kscrash_reportUserException(const char *name, const char *reason,
const char *language,
const char *lineOfCode,
const char *stackTrace,
const char *handledState,
const char *overrides,
const char *metadata,
const char *appState,
const char *config,
int discardDepth,
bool terminateProgram) {
bsg_kscrashsentry_reportUserException(name, reason, language, lineOfCode,
stackTrace, terminateProgram);
bsg_kscrashsentry_reportUserException(name, reason, handledState, overrides,
metadata, appState, config, discardDepth,
terminateProgram);
}

void bsg_kscrash_setSuspendThreadsForUserReported(
Expand Down
26 changes: 13 additions & 13 deletions Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,24 +175,24 @@ void bsg_kscrash_setCrashNotifyCallback(
* application will terminate with an abort().
*
* @param name The exception name (for namespacing exception types).
*
* @param reason A description of why the exception occurred.
*
* @param language A unique language identifier.
*
* @param lineOfCode A copy of the offending line of code (NULL = ignore).
*
* @param stackTrace JSON encoded array containing stack trace information (one
* frame per array entry). The frame structure can be anything you want,
* including bare strings.
*
* @param handledState The severity, reason, and handled-ness of the report
* @param appState breadcrumbs and other app environmental info
* @param overrides Report fields overridden by callbacks, collated in the
* final report
* @param metadata additional information to attach to the report
* @param discardDepth The number of frames to discard from the top of the
* stacktrace
* @param terminateProgram If true, do not return from this function call.
* Terminate the program instead.
*/
void bsg_kscrash_reportUserException(const char *name, const char *reason,
const char *language,
const char *lineOfCode,
const char *stackTrace,
const char *handledState,
const char *overrides,
const char *metadata,
const char *appState,
const char *config,
int discardDepth,
bool terminateProgram);

/** If YES, user reported exceptions will suspend all threads during report
Expand Down
39 changes: 25 additions & 14 deletions Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashReport.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1789,20 +1789,6 @@ void bsg_kscrw_i_writeError(const BSG_KSCrashReportWriter *const writer,
{
writer->addStringElement(writer, BSG_KSCrashField_Name,
crash->userException.name);
if (crash->userException.language != NULL) {
writer->addStringElement(writer, BSG_KSCrashField_Language,
crash->userException.language);
}
if (crash->userException.lineOfCode != NULL) {
writer->addStringElement(writer,
BSG_KSCrashField_LineOfCode,
crash->userException.lineOfCode);
}
if (crash->userException.customStackTrace != NULL) {
writer->addJSONElement(
writer, BSG_KSCrashField_Backtrace,
crash->userException.customStackTrace);
}
}
writer->endContainer(writer);
break;
Expand Down Expand Up @@ -2103,7 +2089,32 @@ void bsg_kscrashreport_writeStandardReport(
writer->endContainer(writer);

if (crashContext->config.onCrashNotify != NULL) {
// Write handled exception report info
writer->beginObject(writer, BSG_KSCrashField_UserAtCrash);
if (crashContext->crash.crashType == BSG_KSCrashTypeUserReported) {
if (crashContext->crash.userException.overrides != NULL) {
writer->addJSONElement(writer, BSG_KSCrashField_Overrides,
crashContext->crash.userException.overrides);
}
if (crashContext->crash.userException.handledState != NULL) {
writer->addJSONElement(writer, BSG_KSCrashField_HandledState,
crashContext->crash.userException.handledState);
}
if (crashContext->crash.userException.metadata != NULL) {
writer->addJSONElement(writer, BSG_KSCrashField_Metadata,
crashContext->crash.userException.metadata);
}
if (crashContext->crash.userException.state != NULL) {
writer->addJSONElement(writer, BSG_KSCrashField_State,
crashContext->crash.userException.state);
}
if (crashContext->crash.userException.config != NULL) {
writer->addJSONElement(writer, BSG_KSCrashField_Config,
crashContext->crash.userException.config);
}
writer->addIntegerElement(writer, BSG_KSCrashField_DiscardDepth,
crashContext->crash.userException.discardDepth);
}
{ bsg_kscrw_i_callUserCrashHandler(crashContext, writer); }
writer->endContainer(writer);
}
Expand Down
6 changes: 6 additions & 0 deletions Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashReportFields.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@
#define BSG_KSCrashField_Signal "signal"
#define BSG_KSCrashField_Subcode "subcode"
#define BSG_KSCrashField_UserReported "user_reported"
#define BSG_KSCrashField_Overrides "overrides"
#define BSG_KSCrashField_HandledState "handledState"
#define BSG_KSCrashField_Metadata "metaData"
#define BSG_KSCrashField_State "state"
#define BSG_KSCrashField_Config "config"
#define BSG_KSCrashField_DiscardDepth "depth"

#pragma mark - Process State -

Expand Down
Loading