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

feat: Report start up crashes #2220

Merged
merged 22 commits into from
Sep 29, 2022
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Features

- Report start up crashes (#2220)

## 7.26.0

### Features
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
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ typedef struct {
/** If true, the application crashed on the previous launch. */
bool crashedLastLaunch;

/** Total time in seconds from the crash state init to the last crash. Only contains a value
* bigger than zero if crashedLastLaunch is true. */
double durationFromCrashStateInitToLastCrash;

// Live data

/** If true, the application crashed on this launch. */
Expand Down Expand Up @@ -93,6 +97,8 @@ void sentrycrashstate_initialize(const char *stateFilePath);
*/
bool sentrycrashstate_reset(void);

const char *sentrycrashstate_filePath(void);

/** Notify the crash reporter of the application active state.
*
* @param isActive true if the application is active, otherwise false.
Expand All @@ -116,7 +122,7 @@ void sentrycrashstate_notifyAppCrash(void);

/** Read-only access into the current state.
*/
const SentryCrash_AppState *const sentrycrashstate_currentState(void);
const SentryCrash_AppState *sentrycrashstate_currentState(void);

/** Access the Monitor API.
*/
Expand Down
8 changes: 8 additions & 0 deletions Tests/Resources/CrashState_legacy_1.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"version": 1,
"crashedLastLaunch": false,
"activeDurationSinceLastCrash": 2.5,
"backgroundDurationSinceLastCrash": 5.0,
"launchesSinceLastCrash": 10,
"sessionsSinceLastCrash": 10
}
Loading