Skip to content

Commit

Permalink
feat: Report start up crashes (#2220)
Browse files Browse the repository at this point in the history
If the app crashes after 2 seconds of the SDK init, the SDK makes the SDK init call blocking
by calling flush with 5 seconds.

Fixes GH-316
  • Loading branch information
philipphofmann authored Sep 29, 2022
1 parent 776d17b commit 7c867f1
Show file tree
Hide file tree
Showing 23 changed files with 377 additions and 69 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Features

- Report start up crashes (#2220)
- Add segment property to user (#2234)

## 7.26.0
Expand Down
4 changes: 4 additions & 0 deletions Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@
7B9657252683104C00C66E25 /* NSData+Sentry.h in Headers */ = {isa = PBXBuildFile; fileRef = 7B9657232683104C00C66E25 /* NSData+Sentry.h */; };
7B9657262683104C00C66E25 /* NSData+Sentry.m in Sources */ = {isa = PBXBuildFile; fileRef = 7B9657242683104C00C66E25 /* NSData+Sentry.m */; };
7B965728268321CD00C66E25 /* SentryCrashScopeObserverTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B965727268321CD00C66E25 /* SentryCrashScopeObserverTests.swift */; };
7B984A9F28E572AF001F4BEE /* CrashReportWriter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7B984A9E28E572AF001F4BEE /* CrashReportWriter.swift */; };
7B98D7BC25FB607300C5A389 /* SentryOutOfMemoryTracker.h in Headers */ = {isa = PBXBuildFile; fileRef = 7B98D7BB25FB607300C5A389 /* SentryOutOfMemoryTracker.h */; };
7B98D7CB25FB64EC00C5A389 /* SentryOutOfMemoryTrackingIntegration.h in Headers */ = {isa = PBXBuildFile; fileRef = 7B98D7CA25FB64EC00C5A389 /* SentryOutOfMemoryTrackingIntegration.h */; };
7B98D7CF25FB650F00C5A389 /* SentryOutOfMemoryTrackingIntegration.m in Sources */ = {isa = PBXBuildFile; fileRef = 7B98D7CE25FB650F00C5A389 /* SentryOutOfMemoryTrackingIntegration.m */; };
Expand Down Expand Up @@ -1138,6 +1139,7 @@
7B9657242683104C00C66E25 /* NSData+Sentry.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "NSData+Sentry.m"; sourceTree = "<group>"; };
7B965727268321CD00C66E25 /* SentryCrashScopeObserverTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryCrashScopeObserverTests.swift; sourceTree = "<group>"; };
7B9660B12783500E0014A767 /* ThreadSanitizer.sup */ = {isa = PBXFileReference; lastKnownFileType = text; path = ThreadSanitizer.sup; sourceTree = "<group>"; };
7B984A9E28E572AF001F4BEE /* CrashReportWriter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CrashReportWriter.swift; sourceTree = "<group>"; };
7B98D7BB25FB607300C5A389 /* SentryOutOfMemoryTracker.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryOutOfMemoryTracker.h; path = include/SentryOutOfMemoryTracker.h; sourceTree = "<group>"; };
7B98D7CA25FB64EC00C5A389 /* SentryOutOfMemoryTrackingIntegration.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryOutOfMemoryTrackingIntegration.h; path = include/SentryOutOfMemoryTrackingIntegration.h; sourceTree = "<group>"; };
7B98D7CE25FB650F00C5A389 /* SentryOutOfMemoryTrackingIntegration.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryOutOfMemoryTrackingIntegration.m; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2175,6 +2177,7 @@
7BED3575266F7BFF00EAA70D /* TestSentryCrashWrapper.m */,
0AABE2EF2885C2120057ED69 /* TestSentryPermissionsObserver.swift */,
0ADC33EF28D9BE690078D980 /* TestSentryUIDeviceWrapper.swift */,
7B984A9E28E572AF001F4BEE /* CrashReportWriter.swift */,
);
path = SentryCrash;
sourceTree = "<group>";
Expand Down Expand Up @@ -3558,6 +3561,7 @@
D88817DD26D72BA500BF2251 /* SentryTraceStateTests.swift in Sources */,
8E25C97525F8511A00DC215B /* TestRandom.swift in Sources */,
7B26BBFB24C0A66D00A79CCC /* SentrySdkInfoNilTests.m in Sources */,
7B984A9F28E572AF001F4BEE /* CrashReportWriter.swift in Sources */,
7BAF3DD7243DD4A1008A5414 /* TestConstants.swift in Sources */,
035E73CC27D575B3005EEB11 /* SentrySamplingProfilerTests.mm in Sources */,
7BED3576266F7BFF00EAA70D /* TestSentryCrashWrapper.m in Sources */,
Expand Down
10 changes: 9 additions & 1 deletion Sources/Sentry/SentryCrashInstallationReporter.m
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,30 @@
SentryCrashInstallationReporter ()

@property (nonatomic, strong) SentryInAppLogic *inAppLogic;
@property (nonatomic, strong) SentryCrashWrapper *crashWrapper;
@property (nonatomic, strong) SentryDispatchQueueWrapper *dispatchQueue;

@end

@implementation SentryCrashInstallationReporter

- (instancetype)initWithInAppLogic:(SentryInAppLogic *)inAppLogic
crashWrapper:(SentryCrashWrapper *)crashWrapper
dispatchQueue:(SentryDispatchQueueWrapper *)dispatchQueue
{
if (self = [super initWithRequiredProperties:[NSArray new]]) {
self.inAppLogic = inAppLogic;
self.crashWrapper = crashWrapper;
self.dispatchQueue = dispatchQueue;
}
return self;
}

- (id<SentryCrashReportFilter>)sink
{
return [[SentryCrashReportSink alloc] initWithInAppLogic:self.inAppLogic];
return [[SentryCrashReportSink alloc] initWithInAppLogic:self.inAppLogic
crashWrapper:self.crashWrapper
dispatchQueue:self.dispatchQueue];
}

- (void)sendAllReports
Expand Down
15 changes: 13 additions & 2 deletions Sources/Sentry/SentryCrashIntegration.m
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,10 @@ - (void)startCrashHandler
[[SentryInAppLogic alloc] initWithInAppIncludes:self.options.inAppIncludes
inAppExcludes:self.options.inAppExcludes];

installation = [[SentryCrashInstallationReporter alloc] initWithInAppLogic:inAppLogic];
installation = [[SentryCrashInstallationReporter alloc]
initWithInAppLogic:inAppLogic
crashWrapper:self.crashAdapter
dispatchQueue:self.dispatchQueueWrapper];

canSendReports = YES;
}
Expand Down Expand Up @@ -137,12 +140,20 @@ - (void)startCrashHandler
// just not call sendAllReports as it doesn't make sense to call it twice as described
// above.
if (canSendReports) {
[installation sendAllReports];
[SentryCrashIntegration sendAllSentryCrashReports];
}
};
[self.dispatchQueueWrapper dispatchOnce:&installationToken block:block];
}

/**
* Internal, only needed for testing.
*/
+ (void)sendAllSentryCrashReports
{
[installation sendAllReports];
}

- (void)uninstall
{
if (nil != installation) {
Expand Down
84 changes: 55 additions & 29 deletions Sources/Sentry/SentryCrashReportSink.m
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@
#import "SentryAttachment.h"
#import "SentryClient.h"
#import "SentryCrash.h"
#include "SentryCrashMonitor_AppState.h"
#import "SentryCrashReportConverter.h"
#import "SentryCrashWrapper.h"
#import "SentryDefines.h"
#import "SentryDispatchQueueWrapper.h"
#import "SentryEvent.h"
#import "SentryException.h"
#import "SentryHub.h"
Expand All @@ -13,23 +16,75 @@
#import "SentryScope.h"
#import "SentryThread.h"

static const NSTimeInterval SENTRY_APP_START_CRASH_DURATION_THRESHOLD = 2.0;
static const NSTimeInterval SENTRY_APP_START_CRASH_FLUSH_DURATION = 5.0;

@interface
SentryCrashReportSink ()

@property (nonatomic, strong) SentryInAppLogic *inAppLogic;
@property (nonatomic, strong) SentryCrashWrapper *crashWrapper;
@property (nonatomic, strong) SentryDispatchQueueWrapper *dispatchQueue;

@end

@implementation SentryCrashReportSink

- (instancetype)initWithInAppLogic:(SentryInAppLogic *)inAppLogic
crashWrapper:(SentryCrashWrapper *)crashWrapper
dispatchQueue:(SentryDispatchQueueWrapper *)dispatchQueue
{
if (self = [super init]) {
self.inAppLogic = inAppLogic;
self.crashWrapper = crashWrapper;
self.dispatchQueue = dispatchQueue;
}
return self;
}

- (void)filterReports:(NSArray *)reports
onCompletion:(SentryCrashReportFilterCompletion)onCompletion
{
NSTimeInterval durationFromCrashStateInitToLastCrash
= self.crashWrapper.durationFromCrashStateInitToLastCrash;
if (durationFromCrashStateInitToLastCrash > 0
&& durationFromCrashStateInitToLastCrash <= SENTRY_APP_START_CRASH_DURATION_THRESHOLD) {
SENTRY_LOG_WARN(@"Startup crash: detected.");
[self sendReports:reports onCompletion:onCompletion];

[SentrySDK flush:SENTRY_APP_START_CRASH_FLUSH_DURATION];
SENTRY_LOG_DEBUG(@"Startup crash: Finished flushing.");

} else {
[self.dispatchQueue
dispatchAsyncWithBlock:^{ [self sendReports:reports onCompletion:onCompletion]; }];
}
}

- (void)sendReports:(NSArray *)reports onCompletion:(SentryCrashReportFilterCompletion)onCompletion
{
NSMutableArray *sentReports = [NSMutableArray new];
for (NSDictionary *report in reports) {
SentryCrashReportConverter *reportConverter =
[[SentryCrashReportConverter alloc] initWithReport:report inAppLogic:self.inAppLogic];
if (nil != [SentrySDK.currentHub getClient]) {
SentryEvent *event = [reportConverter convertReportToEvent];
if (nil != event) {
[self handleConvertedEvent:event report:report sentReports:sentReports];
}
} else {
SENTRY_LOG_ERROR(
@"Crash reports were found but no [SentrySDK.currentHub getClient] is set. "
@"Cannot send crash reports to Sentry. This is probably a misconfiguration, "
@"make sure you set the client with [SentrySDK.currentHub bindClient] before "
@"calling startCrashHandlerWithError:.");
}
}
if (onCompletion) {
onCompletion(sentReports, TRUE, nil);
}
}

- (void)handleConvertedEvent:(SentryEvent *)event
report:(NSDictionary *)report
sentReports:(NSMutableArray *)sentReports
Expand All @@ -46,33 +101,4 @@ - (void)handleConvertedEvent:(SentryEvent *)event
[SentrySDK captureCrashEvent:event withScope:scope];
}

- (void)filterReports:(NSArray *)reports
onCompletion:(SentryCrashReportFilterCompletion)onCompletion
{
dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0ul);
dispatch_async(queue, ^{
NSMutableArray *sentReports = [NSMutableArray new];
for (NSDictionary *report in reports) {
SentryCrashReportConverter *reportConverter =
[[SentryCrashReportConverter alloc] initWithReport:report
inAppLogic:self.inAppLogic];
if (nil != [SentrySDK.currentHub getClient]) {
SentryEvent *event = [reportConverter convertReportToEvent];
if (nil != event) {
[self handleConvertedEvent:event report:report sentReports:sentReports];
}
} else {
SENTRY_LOG_ERROR(
@"Crash reports were found but no [SentrySDK.currentHub getClient] is set. "
@"Cannot send crash reports to Sentry. This is probably a misconfiguration, "
@"make sure you set the client with [SentrySDK.currentHub bindClient] before "
@"calling startCrashHandlerWithError:.");
}
}
if (onCompletion) {
onCompletion(sentReports, TRUE, nil);
}
});
}

@end
5 changes: 5 additions & 0 deletions Sources/Sentry/SentryCrashWrapper.m
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ - (BOOL)crashedLastLaunch
return SentryCrash.sharedInstance.crashedLastLaunch;
}

- (NSTimeInterval)durationFromCrashStateInitToLastCrash
{
return sentrycrashstate_currentState()->durationFromCrashStateInitToLastCrash;
}

- (NSTimeInterval)activeDurationSinceLastCrash
{
return SentryCrash.sharedInstance.activeDurationSinceLastCrash;
Expand Down
2 changes: 2 additions & 0 deletions Sources/Sentry/SentryFileManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ - (nullable instancetype)initWithOptions:(SentryOptions *)options
= NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES)
.firstObject;

SENTRY_LOG_DEBUG(@"SentryFileManager.cachePath: %@", cachePath);

self.sentryPath = [cachePath stringByAppendingPathComponent:@"io.sentry"];
self.sentryPath =
[self.sentryPath stringByAppendingPathComponent:[options.parsedDsn getHash]];
Expand Down
6 changes: 4 additions & 2 deletions Sources/Sentry/include/SentryCrashInstallationReporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@
#import "SentryDefines.h"
#import <Foundation/Foundation.h>

@class SentryInAppLogic;
@class SentryInAppLogic, SentryCrashWrapper, SentryDispatchQueueWrapper;

NS_ASSUME_NONNULL_BEGIN

@interface SentryCrashInstallationReporter : SentryCrashInstallation
SENTRY_NO_INIT

- (instancetype)initWithInAppLogic:(SentryInAppLogic *)inAppLogic;
- (instancetype)initWithInAppLogic:(SentryInAppLogic *)inAppLogic
crashWrapper:(SentryCrashWrapper *)crashWrapper
dispatchQueue:(SentryDispatchQueueWrapper *)dispatchQueue;

- (void)sendAllReports;

Expand Down
5 changes: 5 additions & 0 deletions Sources/Sentry/include/SentryCrashIntegration.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ static NSString *const SentryDeviceContextAppMemoryKey = @"app_memory";

+ (void)enrichScope:(SentryScope *)scope crashWrapper:(SentryCrashWrapper *)crashWrapper;

/**
* Needed for testing.
*/
+ (void)sendAllSentryCrashReports;

@end

NS_ASSUME_NONNULL_END
6 changes: 4 additions & 2 deletions Sources/Sentry/include/SentryCrashReportSink.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@
#import "SentryDefines.h"
#import <Foundation/Foundation.h>

@class SentryInAppLogic;
@class SentryInAppLogic, SentryCrashWrapper, SentryDispatchQueueWrapper;

NS_ASSUME_NONNULL_BEGIN

@interface SentryCrashReportSink : NSObject <SentryCrashReportFilter>
SENTRY_NO_INIT

- (instancetype)initWithInAppLogic:(SentryInAppLogic *)inAppLogic;
- (instancetype)initWithInAppLogic:(SentryInAppLogic *)inAppLogic
crashWrapper:(SentryCrashWrapper *)crashWrapper
dispatchQueue:(SentryDispatchQueueWrapper *)dispatchQueue;

@end

Expand Down
2 changes: 2 additions & 0 deletions Sources/Sentry/include/SentryCrashWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ SENTRY_NO_INIT

- (BOOL)crashedLastLaunch;

- (NSTimeInterval)durationFromCrashStateInitToLastCrash;

- (NSTimeInterval)activeDurationSinceLastCrash;

- (BOOL)isBeingTraced;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@

#define kKeyFormatVersion "version"
#define kKeyCrashedLastLaunch "crashedLastLaunch"
#define kKeyDurationFromCrashStateInitToLastCrash "durationFromCrashStateInitToLastCrash"
#define kKeyActiveDurationSinceLastCrash "activeDurationSinceLastCrash"
#define kKeyBackgroundDurationSinceLastCrash "backgroundDurationSinceLastCrash"
#define kKeyLaunchesSinceLastCrash "launchesSinceLastCrash"
Expand All @@ -65,6 +66,8 @@ static const char *g_stateFilePath;
/** Current state. */
static SentryCrash_AppState g_state;

static double g_crashstate_initialize_time;

static volatile bool g_isEnabled = false;

// ============================================================================
Expand Down Expand Up @@ -96,6 +99,10 @@ onFloatingPointElement(const char *const name, const double value, void *const u
return SentryCrashJSON_ERROR_INVALID_DATA;
}

if (strcmp(name, kKeyDurationFromCrashStateInitToLastCrash) == 0) {
state->durationFromCrashStateInitToLastCrash = value;
}

if (strcmp(name, kKeyActiveDurationSinceLastCrash) == 0) {
state->activeDurationSinceLastCrash = value;
}
Expand Down Expand Up @@ -277,6 +284,22 @@ saveState(const char *const path)
!= SentryCrashJSON_OK) {
goto done;
}

// SentryCrash resets the app state when enabling it in setEnabled. To keep the value alive for
// the application's lifetime, we don't modify the g_state. Instead, we only save the value to
// the crash state file without setting it to g_state. When initializing the app state, the code
// reads the value from the file and keeps it in memory. The code uses the same pattern for
// CrashedLastLaunch. Ideally, we would refactor this, but we must be aware of possible side
// effects.
double durationFromCrashStateInitToLastCrash = 0;
if (g_state.crashedThisLaunch) {
durationFromCrashStateInitToLastCrash = timeSince(g_crashstate_initialize_time);
}
if ((result = sentrycrashjson_addFloatingPointElement(&JSONContext,
kKeyDurationFromCrashStateInitToLastCrash, durationFromCrashStateInitToLastCrash))
!= SentryCrashJSON_OK) {
goto done;
}
if ((result = sentrycrashjson_addFloatingPointElement(
&JSONContext, kKeyActiveDurationSinceLastCrash, g_state.activeDurationSinceLastCrash))
!= SentryCrashJSON_OK) {
Expand Down Expand Up @@ -315,6 +338,7 @@ saveState(const char *const path)
void
sentrycrashstate_initialize(const char *const stateFilePath)
{
g_crashstate_initialize_time = getCurentTime();
g_stateFilePath = strdup(stateFilePath);
memset(&g_state, 0, sizeof(g_state));
loadState(g_stateFilePath);
Expand Down Expand Up @@ -345,6 +369,12 @@ sentrycrashstate_reset()
return false;
}

const char *
sentrycrashstate_filePath(void)
{
return g_stateFilePath;
}

void
sentrycrashstate_notifyAppActive(const bool isActive)
{
Expand Down Expand Up @@ -411,7 +441,7 @@ sentrycrashstate_notifyAppCrash(void)
}
}

const SentryCrash_AppState *const
const SentryCrash_AppState *
sentrycrashstate_currentState(void)
{
return &g_state;
Expand Down
Loading

0 comments on commit 7c867f1

Please sign in to comment.