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

Add sendThreads property to BugsnagConfiguration #549

Merged
merged 2 commits into from
Apr 21, 2020
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ Bugsnag Notifiers on other platforms.

## Enhancements

* Add `sendThreads` property to `BugsnagConfiguration`
[#549](https://github.com/bugsnag/bugsnag-cocoa/pull/549)

* Add structured app/device fields to `BugsnagSession`
[#546](https://github.com/bugsnag/bugsnag-cocoa/pull/546)

Expand Down
32 changes: 32 additions & 0 deletions Source/BugsnagConfiguration.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,28 @@
@class BugsnagUser;
@class BugsnagEndpointConfiguration;

/**
* Controls whether Bugsnag should capture and serialize the state of all threads at the time
* of an error.
*/
typedef NS_ENUM(NSInteger, BSGThreadSendPolicy) {

/**
* Threads should be captured for all events.
*/
BSGThreadSendPolicyAlways = 0,

/**
* Threads should be captured for unhandled events only.
*/
BSGThreadSendPolicyUnhandledOnly = 1,

/**
* Threads should never be captured.
*/
BSGThreadSendPolicyNever = 2
};

/**
* BugsnagConfiguration error constants
*/
Expand Down Expand Up @@ -134,6 +156,16 @@ typedef NS_OPTIONS(NSUInteger, BSGErrorType) {
*/
@property(readwrite, strong, nonnull) NSURLSession *session;

/**
* Controls whether Bugsnag should capture and serialize the state of all threads at the time
* of an error.
*
* By default sendThreads is set to BSGThreadSendPolicyAlways. This can be set to
* BSGThreadSendPolicyNever to disable or BSGThreadSendPolicyUnhandledOnly
* to only do so for unhandled errors.
*/
@property BSGThreadSendPolicy sendThreads;

/**
* Optional handler invoked when an error or crash occurs
*/
Expand Down
2 changes: 2 additions & 0 deletions Source/BugsnagConfiguration.m
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ - (nonnull id)copyWithZone:(nullable NSZone *)zone {
[copy setPlugins:[self.plugins copy]];
[copy setReleaseStage:self.releaseStage];
[copy setSession:[self.session copy]];
[copy setSendThreads:self.sendThreads];
[copy setUser:self.user.userId
withEmail:self.user.emailAddress
andName:self.user.name];
Expand Down Expand Up @@ -196,6 +197,7 @@ - (instancetype _Nonnull)initWithApiKey:(NSString *_Nonnull)apiKey
_redactedKeys = @[@"password"];
_breadcrumbs = [BugsnagBreadcrumbs new];
_autoTrackSessions = YES;
_sendThreads = BSGThreadSendPolicyAlways;
// Default to recording all error types
_enabledErrorTypes = BSGErrorTypesCPP
| BSGErrorTypesMach
Expand Down
15 changes: 9 additions & 6 deletions Source/BugsnagCrashSentry.m
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@ - (void)install:(BugsnagConfiguration *)config
onCrash:(BSGReportCallback)onCrash
{
BugsnagSink *sink = [[BugsnagSink alloc] initWithApiClient:apiClient];
[BSG_KSCrash sharedInstance].sink = sink;
[BSG_KSCrash sharedInstance].introspectMemory = YES;
[BSG_KSCrash sharedInstance].deleteBehaviorAfterSendAll =
BSG_KSCrash *ksCrash = [BSG_KSCrash sharedInstance];
ksCrash.sink = sink;
ksCrash.introspectMemory = YES;
ksCrash.deleteBehaviorAfterSendAll =
BSG_KSCDeleteOnSuccess;
[BSG_KSCrash sharedInstance].onCrash = onCrash;
[BSG_KSCrash sharedInstance].maxStoredReports = BSG_MAX_STORED_REPORTS;
ksCrash.onCrash = onCrash;
ksCrash.maxStoredReports = BSG_MAX_STORED_REPORTS;
ksCrash.threadTracingEnabled = (int) config.sendThreads;

// User reported events are *always* handled
BSG_KSCrashType crashTypes = BSG_KSCrashTypeUserReported;
Expand All @@ -44,8 +46,9 @@ - (void)install:(BugsnagConfiguration *)config

bsg_kscrash_setHandlingCrashTypes(crashTypes);

if (![[BSG_KSCrash sharedInstance] install])
if ((![ksCrash install])) {
bsg_log_err(@"Failed to install crash handler. No exceptions will be reported!");
}

[sink.apiClient flushPendingData];
}
Expand Down
3 changes: 2 additions & 1 deletion Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#import "BSG_KSCrashReportFilterCompletion.h"
#import "BSG_KSCrashReportWriter.h"
#import "BSG_KSCrashType.h"
#import "BugsnagConfiguration.h"

typedef enum {
BSG_KSCDeleteNever,
Expand Down Expand Up @@ -155,7 +156,7 @@ typedef enum {
/**
* If YES, thread traces will be collected with each report.
*/
@property(nonatomic, readwrite, assign) BOOL threadTracingEnabled;
@property(nonatomic, readwrite, assign) int threadTracingEnabled;

/**
* If YES, binary images will be collected for each report.
Expand Down
4 changes: 2 additions & 2 deletions Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.m
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ - (id)initWithReportFilesDirectory:(NSString *)reportFilesDirectory {

self.suspendThreadsForUserReported = YES;
self.reportWhenDebuggerIsAttached = NO;
self.threadTracingEnabled = YES;
self.threadTracingEnabled = BSGThreadSendPolicyAlways;
self.writeBinaryImagesForUserReported = YES;
}
return self;
Expand Down Expand Up @@ -191,7 +191,7 @@ - (void)setReportWhenDebuggerIsAttached:(BOOL)reportWhenDebuggerIsAttached {
bsg_kscrash_setReportWhenDebuggerIsAttached(reportWhenDebuggerIsAttached);
}

- (void)setThreadTracingEnabled:(BOOL)threadTracingEnabled {
- (void)setThreadTracingEnabled:(int)threadTracingEnabled {
_threadTracingEnabled = threadTracingEnabled;
bsg_kscrash_setThreadTracingEnabled(threadTracingEnabled);
}
Expand Down
2 changes: 1 addition & 1 deletion Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.c
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,6 @@ void bsg_kscrash_setReportWhenDebuggerIsAttached(
reportWhenDebuggerIsAttached;
}

void bsg_kscrash_setThreadTracingEnabled(bool threadTracingEnabled) {
void bsg_kscrash_setThreadTracingEnabled(int threadTracingEnabled) {
crashContext()->crash.threadTracingEnabled = threadTracingEnabled;
}
2 changes: 1 addition & 1 deletion Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ void bsg_kscrash_setSuspendThreadsForUserReported(
void bsg_kscrash_setReportWhenDebuggerIsAttached(
bool reportWhenDebuggerIsAttached);

void bsg_kscrash_setThreadTracingEnabled(bool threadTracingEnabled);
void bsg_kscrash_setThreadTracingEnabled(int threadTracingEnabled);

void bsg_kscrash_setWriteBinaryImagesForUserReported(
bool writeBinaryImagesForUserReported);
Expand Down
32 changes: 18 additions & 14 deletions Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashReport.c
Original file line number Diff line number Diff line change
Expand Up @@ -959,8 +959,13 @@ void bsg_kscrw_i_writeThread(const BSG_KSCrashReportWriter *const writer,
const char *const key,
const BSG_KSCrash_SentryContext *const crash,
const thread_t thread, const int index,
const bool writeNotableAddresses) {
const bool writeNotableAddresses,
const bool recordAllThreads) {
bool isCrashedThread = thread == crash->offendingThread;
if (!isCrashedThread && !recordAllThreads) {
return;
}

BSG_STRUCT_MCONTEXT_L machineContextBuffer;
uintptr_t backtraceBuffer[BSG_kMaxBacktraceDepth];
int backtraceLength = sizeof(backtraceBuffer) / sizeof(*backtraceBuffer);
Expand Down Expand Up @@ -1012,7 +1017,8 @@ void bsg_kscrw_i_writeThread(const BSG_KSCrashReportWriter *const writer,
void bsg_kscrw_i_writeAllThreads(const BSG_KSCrashReportWriter *const writer,
const char *const key,
const BSG_KSCrash_SentryContext *const crash,
bool writeNotableAddresses) {
bool writeNotableAddresses,
const bool recordAllThreads) {
const task_t thisTask = mach_task_self();
thread_act_array_t threads;
mach_msg_type_number_t numThreads;
Expand All @@ -1028,7 +1034,7 @@ void bsg_kscrw_i_writeAllThreads(const BSG_KSCrashReportWriter *const writer,
{
for (mach_msg_type_number_t i = 0; i < numThreads; i++) {
bsg_kscrw_i_writeThread(writer, NULL, crash, threads[i], (int)i,
writeNotableAddresses);
writeNotableAddresses, recordAllThreads);
}
}
writer->endContainer(writer);
Expand Down Expand Up @@ -1539,7 +1545,7 @@ void bsg_kscrashreport_writeMinimalReport(
writer, BSG_KSCrashField_CrashedThread, &crashContext->crash,
crashContext->crash.offendingThread,
bsg_kscrw_i_threadIndex(crashContext->crash.offendingThread),
false);
false, false);
bsg_kscrw_i_writeError(writer, BSG_KSCrashField_Error,
&crashContext->crash);
}
Expand Down Expand Up @@ -1610,16 +1616,14 @@ void bsg_kscrashreport_writeStandardReport(

writer->beginObject(writer, BSG_KSCrashField_Crash);
{
// Don't write the threads for user reported crashes to improve
// performance
if (crashContext->crash.threadTracingEnabled == true ||
crashContext->crash.crashType != BSG_KSCrashTypeUserReported) {
bsg_kscrw_i_writeAllThreads(
writer, BSG_KSCrashField_Threads, &crashContext->crash,
crashContext->config.introspectionRules.enabled);
}
bsg_kscrw_i_writeError(writer, BSG_KSCrashField_Error,
&crashContext->crash);
// Conditionally write threads depending on user configuration
int sendPolicy = crashContext->crash.threadTracingEnabled;
bool unhandledCrash = crashContext->crash.crashType != BSG_KSCrashTypeUserReported;
bool recordAllThreads = sendPolicy == 0 || (unhandledCrash && sendPolicy == 1);

bsg_kscrw_i_writeAllThreads(writer, BSG_KSCrashField_Threads, &crashContext->crash,
crashContext->config.introspectionRules.enabled, recordAllThreads);
bsg_kscrw_i_writeError(writer, BSG_KSCrashField_Error,&crashContext->crash);
}
writer->endContainer(writer);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ void bsg_kscrashsentry_resumeThreads(void) {

void bsg_kscrashsentry_clearContext(BSG_KSCrash_SentryContext *context) {
void (*onCrash)(void *) = context->onCrash;
bool threadTracingEnabled = context->threadTracingEnabled;
int threadTracingEnabled = context->threadTracingEnabled;
bool reportWhenDebuggerIsAttached = context->reportWhenDebuggerIsAttached;
bool suspendThreadsForUserReported = context->suspendThreadsForUserReported;
bool writeBinaryImagesForUserReported =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,11 @@ typedef struct BSG_KSCrash_SentryContext {
/** If true, will send reports even if debugger is attached. */
bool reportWhenDebuggerIsAttached;

/** If true, will trace threads and report binary images. */
bool threadTracingEnabled;
/**
* The methodology used for tracing threads and report binary images.
* The value will be equal to an enum value from BSGThreadSendPolicy
*/
int threadTracingEnabled;

/** If true, will record binary images. */
bool writeBinaryImagesForUserReported;
Expand Down
11 changes: 10 additions & 1 deletion Tests/BugsnagConfigurationTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,11 @@ - (void) testClearOnSendBlock {
XCTAssertEqual([[configuration onSendBlocks] count], 2);
}

- (void)testSendThreadsDefault {
BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
XCTAssertEqual(BSGThreadSendPolicyAlways, config.sendThreads);
}

- (void)testNSCopying {
BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];

Expand All @@ -796,6 +801,7 @@ - (void)testNSCopying {
[config setContext:@"context1"];
[config setAppType:@"The most amazing app, a brilliant app, the app to end all apps"];
[config setPersistUser:YES];
[config setSendThreads:BSGThreadSendPolicyUnhandledOnly];
BugsnagOnSendBlock onSendBlock1 = ^BOOL(BugsnagEvent * _Nonnull event) { return true; };
BugsnagOnSendBlock onSendBlock2 = ^BOOL(BugsnagEvent * _Nonnull event) { return true; };

Expand All @@ -810,7 +816,10 @@ - (void)testNSCopying {

// Redacted keys
XCTAssertEqualObjects(config.redactedKeys, clone.redactedKeys);


// sendThreads
XCTAssertEqual(config.sendThreads, clone.sendThreads);

// Object
[clone setUser:@"Cthulu" withEmail:@"[email protected]" andName:@"Howard"];
XCTAssertEqualObjects(config.user.userId, @"foo");
Expand Down
1 change: 1 addition & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ The exact error is available using the `BSGConfigurationErrorDomain` and
+ config.removeOnBreadcrumb(block:)

+ config.redactedKeys
+ config.sendThreads
```

#### Renames
Expand Down
1 change: 1 addition & 0 deletions examples/objective-c-ios/Bugsnag Test App/AppDelegate.m
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ - (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(

NSString *apiKey = @"<YOUR_APIKEY_HERE>";
BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:apiKey];
configuration.sendThreads = BSGThreadSendPolicyAlways;
[Bugsnag startBugsnagWithConfiguration:configuration];

return YES;
Expand Down