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

Fix session delivery logic to always use the delivery queue #295

Merged
merged 10 commits into from
Jul 16, 2018
9 changes: 0 additions & 9 deletions BugsnagApiClient.h

This file was deleted.

11 changes: 0 additions & 11 deletions BugsnagApiClient.m

This file was deleted.

2 changes: 2 additions & 0 deletions Source/BugsnagConfiguration.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ BugsnagBreadcrumbs *breadcrumbs;

/**
* Determines whether app sessions should be tracked automatically. By default this value is true.
* If this value is updated after +[Bugsnag start] is called, only subsequent automatic sessions
* will be captured.
*/
@property BOOL shouldAutoCaptureSessions;

Expand Down
17 changes: 0 additions & 17 deletions Source/BugsnagConfiguration.m
Original file line number Diff line number Diff line change
Expand Up @@ -218,23 +218,6 @@ - (void)setApiKey:(NSString *)apiKey {
}
}

@synthesize shouldAutoCaptureSessions = _shouldAutoCaptureSessions;

- (BOOL)shouldAutoCaptureSessions {
return _shouldAutoCaptureSessions;
}

- (void)setShouldAutoCaptureSessions:(BOOL)shouldAutoCaptureSessions {
@synchronized (self) {
_shouldAutoCaptureSessions = shouldAutoCaptureSessions;

if (shouldAutoCaptureSessions) { // track any existing sessions
Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC this logic (or similar) may have been added to enable automatic session tracking on React Native. Have we tested the effects of these changes on downstream notifiers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bugsnag-react-native will need a slight tweak in the initialization logic. While configuring the JS options, when all of the following are true:

  • Bugsnag has already been configured on the native side
  • The native configuration has shouldAutoCaptureSessions set to NO
  • The JS configuration has autoCaptureSessions set to true

Then [BugsnagReactNative start] should call startSession

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@martin308 do you foresee a problem with this on the unity side? My hunch is that it’s even less of a problem since the automatic session logic now lives on the C# side.

Choose a reason for hiding this comment

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

Yeh there wouldn't be an issue with it the way it is setup right now. For the native notifiers we turn off automatic session tracking

BugsnagSessionTracker *sessionTracker = [Bugsnag notifier].sessionTracker;
[sessionTracker onAutoCaptureEnabled];
}
}
}

- (NSDictionary *)errorApiHeaders {
return @{
kHeaderApiPayloadVersion: @"4.0",
Expand Down
3 changes: 1 addition & 2 deletions Source/BugsnagFileStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@
- (NSMutableDictionary *)readFile:(NSString *)path
error:(NSError *__autoreleasing *)error;

+ (NSString *)findReportStorePath:(NSString *)customDirectory
bundleName:(NSString *)bundleName;
+ (NSString *)findReportStorePath:(NSString *)customDirectory;

- (NSString *)fileIdFromFilename:(NSString *)filename;
@end
4 changes: 2 additions & 2 deletions Source/BugsnagFileStore.m
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,9 @@ - (void)deleteFileWithId:(NSString *)fileId {
}
}

+ (NSString *)findReportStorePath:(NSString *)customDirectory
bundleName:(NSString *)bundleName {
+ (NSString *)findReportStorePath:(NSString *)customDirectory {

NSString *bundleName = [[NSBundle mainBundle] infoDictionary][@"CFBundleName"];
NSArray *directories = NSSearchPathForDirectoriesInDomains(
NSCachesDirectory, NSUserDomainMask, YES);
if ([directories count] == 0) {
Expand Down
102 changes: 41 additions & 61 deletions Source/BugsnagNotifier.m
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
#import "BugsnagLogger.h"
#import "BugsnagKeys.h"
#import "BugsnagSessionTracker.h"
#import "BugsnagSessionTrackingApiClient.h"
#import "BSG_RFC3339DateTool.h"
#import "BSG_KSCrashType.h"

Expand Down Expand Up @@ -164,12 +163,36 @@ void BSSerializeJSONDictionary(NSDictionary *dictionary, char **destination) {
}
}

/**
Save info about the current session to crash data. Ensures that session
data is written to unhandled error reports.

@param session The current session
*/
void BSGWriteSessionCrashData(BugsnagSession *session) {
if (session == nil) {
return;
}
// copy session id
const char *newSessionId = [session.sessionId UTF8String];
size_t idSize = strlen(newSessionId);
strncpy((char *)sessionId, newSessionId, idSize);
sessionId[idSize - 1] = NULL;

const char *newSessionDate = [[BSG_RFC3339DateTool stringFromDate:session.startedAt] UTF8String];
size_t dateSize = strlen(newSessionDate);
strncpy((char *)sessionStartDate, newSessionDate, dateSize);
sessionStartDate[dateSize - 1] = NULL;

// record info for C JSON serialiser
handledCount = session.handledCount;
hasRecordedSessions = true;
}

@interface BugsnagNotifier ()
@property(nonatomic) BugsnagCrashSentry *crashSentry;
@property(nonatomic) BugsnagErrorReportApiClient *errorReportApiClient;
@property(nonatomic) BugsnagSessionTrackingApiClient *sessionTrackingApiClient;
@property(nonatomic) BugsnagSessionTracker *sessionTracker;
@property(nonatomic) NSTimer *sessionTimer;
@end

@implementation BugsnagNotifier
Expand All @@ -193,39 +216,21 @@ - (id)initWithConfiguration:(BugsnagConfiguration *)initConfiguration {
self.crashSentry = [BugsnagCrashSentry new];
self.errorReportApiClient = [[BugsnagErrorReportApiClient alloc] initWithConfig:configuration
queueName:@"Error API queue"];
self.sessionTrackingApiClient = [[BugsnagSessionTrackingApiClient alloc] initWithConfig:configuration
queueName:@"Session API queue"];

self.sessionTracker = [[BugsnagSessionTracker alloc] initWithConfig:initConfiguration
apiClient:self.sessionTrackingApiClient
callback:^(BugsnagSession *session) {

// copy session id
const char *newSessionId = [session.sessionId UTF8String];
size_t idSize = strlen(newSessionId);
strncpy((char *)sessionId, newSessionId, idSize);
sessionId[idSize - 1] = NULL;
bsg_g_bugsnag_data.onCrash = (void (*)(const BSG_KSCrashReportWriter *))self.configuration.onCrashHandler;

const char *newSessionDate = [[BSG_RFC3339DateTool stringFromDate:session.startedAt] UTF8String];
size_t dateSize = strlen(newSessionDate);
strncpy((char *)sessionStartDate, newSessionDate, dateSize);
sessionStartDate[dateSize - 1] = NULL;
static dispatch_once_t once_t;
dispatch_once(&once_t, ^{
[self initializeNotificationNameMap];
});

// record info for C JSON serialiser
handledCount = session.handledCount;
hasRecordedSessions = true;
}];
self.sessionTracker = [[BugsnagSessionTracker alloc] initWithConfig:initConfiguration
postRecordCallback:^(BugsnagSession *session) {
BSGWriteSessionCrashData(session);
}];

[self metaDataChanged:self.configuration.metaData];
[self metaDataChanged:self.configuration.config];
[self metaDataChanged:self.state];
bsg_g_bugsnag_data.onCrash = (void (*)(
const BSG_KSCrashReportWriter *))self.configuration.onCrashHandler;

static dispatch_once_t once_t;
dispatch_once(&once_t, ^{
[self initializeNotificationNameMap];
});
}
return self;
}
Expand Down Expand Up @@ -301,7 +306,6 @@ - (void)start {
[self.crashSentry install:self.configuration
apiClient:self.errorReportApiClient
onCrash:&BSSerializeDataCrashHandler];

[self setupConnectivityListener];
[self updateAutomaticBreadcrumbDetectionSettings];

Expand Down Expand Up @@ -354,6 +358,7 @@ - (void)start {
#endif

_started = YES;
[self.sessionTracker startNewSessionIfAutoCaptureEnabled];

// notification not received in time on initial startup, so trigger manually
[self willEnterForeground:self];
Expand Down Expand Up @@ -383,34 +388,15 @@ - (void)watchLifecycleEvents:(NSNotificationCenter *)center {
}

- (void)willEnterForeground:(id)sender {
[self.sessionTracker startNewSession:[NSDate date]
withUser:self.configuration.currentUser
autoCaptured:YES];

NSTimeInterval sessionTickSeconds = 60;

if (!self.sessionTimer) {
_sessionTimer = [NSTimer scheduledTimerWithTimeInterval:sessionTickSeconds
target:self
selector:@selector(sessionTick:)
userInfo:nil
repeats:YES];
[self sessionTick:self];
}
[self.sessionTracker handleAppForegroundEvent];
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this capture 2 sessions on application startup rather than 1? startNewSessionIfAutoCaptureEnabled will also start a session, unless I'm mistaken.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It only captures 1 event, since the foreground session only happens when coming from the background and having backgroundTime more than 60s before.

}

- (void)willEnterBackground:(id)sender {
[self.sessionTracker suspendCurrentSession:[NSDate date]];

if (self.sessionTimer) {
[self.sessionTimer invalidate];
self.sessionTimer = nil;
}

[self.sessionTracker handleAppBackgroundEvent];
}

- (void)sessionTick:(id)sender {
[self.sessionTracker send];
- (void)startSession {
[self.sessionTracker startNewSession];
}

- (void)flushPendingReports {
Expand All @@ -429,11 +415,6 @@ - (void)setupConnectivityListener {
[self.networkReachable startWatchingConnectivity];
}

- (void)startSession {
[self.sessionTracker startNewSession:[NSDate date]
withUser:self.configuration.currentUser
autoCaptured:NO];
}

- (void)notifyError:(NSError *)error
block:(void (^)(BugsnagCrashReport *))block {
Expand Down Expand Up @@ -518,8 +499,7 @@ - (void)notify:(NSString *)exceptionName
message:(NSString *)message
handledState:(BugsnagHandledState *_Nonnull)handledState
block:(void (^)(BugsnagCrashReport *))block {

[self.sessionTracker incrementHandledError];
[self.sessionTracker handleHandledErrorEvent];

BugsnagCrashReport *report = [[BugsnagCrashReport alloc]
initWithErrorName:exceptionName
Expand Down
51 changes: 38 additions & 13 deletions Source/BugsnagSessionTracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,50 @@ typedef void (^SessionTrackerCallback)(BugsnagSession *newSession);

@interface BugsnagSessionTracker : NSObject

/**
Create a new session tracker

@param config The Bugsnag configuration to use
@param callback A callback invoked each time a new session is started
@return A new session tracker
*/
- (instancetype)initWithConfig:(BugsnagConfiguration *)config
apiClient:(BugsnagSessionTrackingApiClient *)apiClient
callback:(void(^)(BugsnagSession *))callback;
postRecordCallback:(void(^)(BugsnagSession *))callback;

- (void)startNewSession:(NSDate *)date
withUser:(BugsnagUser *)user
autoCaptured:(BOOL)autoCaptured;
/**
Record and send a new session
*/
- (void)startNewSession;

/**
Record a new auto-captured session if neededed. Auto-captured sessions are only
recorded and sent if -[BugsnagConfiguration shouldAutoCaptureSessions] is YES
*/
- (void)startNewSessionIfAutoCaptureEnabled;

- (void)suspendCurrentSession:(NSDate *)date;
- (void)incrementHandledError;
- (void)send;
- (void)onAutoCaptureEnabled;
/**
Handle the app foregrounding event. If more than 30s has elapsed since being
sent to the background, records a new session if session auto-capture is
enabled.
Must be called from the main thread.
*/
- (void)handleAppForegroundEvent;

@property (readonly) BugsnagSession *currentSession;
@property (readonly) BOOL isInForeground;
/**
Handle the app backgrounding event. Tracks time between foreground and
background to determine when to automatically record a session.
Must be called from the main thread.
*/
- (void)handleAppBackgroundEvent;

/**
* Called when a session is altered
Handle some variation of Bugsnag.notify() being called.
Increases the number of handled errors recorded for the current session, if
a session exists.
*/
@property SessionTrackerCallback callback;
- (void)handleHandledErrorEvent;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it also make sense to have an unhandled equivalent of this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really; if an unhandled event occurs, the app is crashing. There can’t be more than 1 unhandled event in a single session.



@property (nonatomic, strong, readonly) BugsnagSession *currentSession;

@end
Loading