Skip to content

Commit

Permalink
fix: Errors shortly after SentrySDK.init don't affect the session (#2430
Browse files Browse the repository at this point in the history
)
  • Loading branch information
kevinrenskers committed Nov 23, 2022
1 parent f91714d commit 9cb6c52
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 28 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ This version adds a dependency on Swift.

- Properly demangle Swift class name (#2162)

### Fixes

- Errors shortly after SentrySDK.init now affect the session (#2430)

### Breaking Changes

- Rename `- [SentrySDK startWithOptionsObject:]` to `- [SentrySDK startWithOptions:]` (#2404)
Expand Down
63 changes: 37 additions & 26 deletions Sources/Sentry/SentryHub.m
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#import "SentrySamplingContext.h"
#import "SentryScope+Private.h"
#import "SentrySerialization.h"
#import "SentrySession+Private.h"
#import "SentryTracer.h"
#import "SentryTracesSampler.h"
#import "SentryTransaction.h"
Expand All @@ -33,6 +34,7 @@
@property (nonatomic, strong) id<SentryCurrentDateProvider> currentDateProvider;
@property (nonatomic, strong) NSMutableArray<id<SentryIntegrationProtocol>> *installedIntegrations;
@property (nonatomic, strong) NSMutableSet<NSString *> *installedIntegrationNames;
@property (nonatomic) NSUInteger errorsBeforeSession;

@end

Expand All @@ -53,6 +55,7 @@ - (instancetype)initWithClient:(nullable SentryClient *)client
_installedIntegrationNames = [[NSMutableSet alloc] init];
_crashWrapper = [SentryCrashWrapper sharedInstance];
_tracesSampler = [[SentryTracesSampler alloc] initWithOptions:client.options];
_errorsBeforeSession = 0;
#if SENTRY_TARGET_PROFILING_SUPPORTED
if (client.options.isProfilingEnabled) {
_profilesSampler = [[SentryProfilesSampler alloc] initWithOptions:client.options];
Expand Down Expand Up @@ -81,20 +84,25 @@ - (void)startSession
SentrySession *lastSession = nil;
SentryScope *scope = self.scope;
SentryOptions *options = [_client options];
if (nil == options || nil == options.releaseName) {
if (options == nil || options.releaseName == nil) {
[SentryLog
logWithMessage:[NSString stringWithFormat:@"No option or release to start a session."]
andLevel:kSentryLevelError];
return;
}
@synchronized(_sessionLock) {
if (nil != _session) {
if (_session != nil) {
lastSession = _session;
}
_session = [[SentrySession alloc] initWithReleaseName:options.releaseName];

if (_errorsBeforeSession > 0 && options.enableAutoSessionTracking == true) {
_session.errors = _errorsBeforeSession;
_errorsBeforeSession = 0;
}

NSString *environment = options.environment;
if (nil != environment) {
if (environment != nil) {
_session.environment = environment;
}

Expand All @@ -119,10 +127,11 @@ - (void)endSessionWithTimestamp:(NSDate *)timestamp
@synchronized(_sessionLock) {
currentSession = _session;
_session = nil;
_errorsBeforeSession = 0;
[self deleteCurrentSession];
}

if (nil == currentSession) {
if (currentSession == nil) {
SENTRY_LOG_DEBUG(@"No session to end with timestamp.");
return;
}
Expand All @@ -145,23 +154,23 @@ - (void)closeCachedSessionWithTimestamp:(nullable NSDate *)timestamp
{
SentryFileManager *fileManager = [_client fileManager];
SentrySession *session = [fileManager readCurrentSession];
if (nil == session) {
if (session == nil) {
SENTRY_LOG_DEBUG(@"No cached session to close.");
return;
}
SENTRY_LOG_DEBUG(@"A cached session was found.");

// Make sure there's a client bound.
SentryClient *client = _client;
if (nil == client) {
if (client == nil) {
SENTRY_LOG_DEBUG(@"No client bound.");
return;
}

// The crashed session is handled in SentryCrashIntegration. Checkout the comments there to find
// out more.
if (!self.crashWrapper.crashedLastLaunch) {
if (nil == timestamp) {
if (timestamp == nil) {
SENTRY_LOG_DEBUG(@"No timestamp to close session was provided. Closing as abnormal. "
@"Using session's start time %@",
session.started);
Expand All @@ -178,7 +187,7 @@ - (void)closeCachedSessionWithTimestamp:(nullable NSDate *)timestamp

- (void)captureSession:(nullable SentrySession *)session
{
if (nil != session) {
if (session != nil) {
SentryClient *client = _client;

if (client.options.diagnosticLevel == kSentryLevelDebug) {
Expand All @@ -200,7 +209,7 @@ - (nullable SentrySession *)incrementSessionErrors
{
SentrySession *sessionCopy = nil;
@synchronized(_sessionLock) {
if (nil != _session) {
if (_session != nil) {
[_session incrementErrors];
[self storeCurrentSession:_session];
sessionCopy = [_session copy];
Expand All @@ -226,7 +235,7 @@ - (void)captureCrashEvent:(SentryEvent *)event withScope:(SentryScope *)scope
event.isCrashEvent = YES;

SentryClient *client = _client;
if (nil == client) {
if (client == nil) {
return;
}

Expand All @@ -237,7 +246,7 @@ - (void)captureCrashEvent:(SentryEvent *)event withScope:(SentryScope *)scope

// It can be that there is no session yet, because autoSessionTracking was just enabled and
// there is a previous crash on disk. In this case we just send the crash event.
if (nil != crashedSession) {
if (crashedSession != nil) {
[client captureCrashEvent:event withSession:crashedSession withScope:scope];
[fileManager deleteCrashedSession];
return;
Expand Down Expand Up @@ -283,7 +292,7 @@ - (SentryId *)captureEvent:(SentryEvent *)event
additionalEnvelopeItems:(NSArray<SentryEnvelopeItem *> *)additionalEnvelopeItems
{
SentryClient *client = _client;
if (nil != client) {
if (client != nil) {
return [client captureEvent:event
withScope:scope
additionalEnvelopeItems:additionalEnvelopeItems];
Expand Down Expand Up @@ -426,7 +435,7 @@ - (SentryId *)captureMessage:(NSString *)message
- (SentryId *)captureMessage:(NSString *)message withScope:(SentryScope *)scope
{
SentryClient *client = _client;
if (nil != client) {
if (client != nil) {
return [client captureMessage:message withScope:scope];
}
return SentryId.empty;
Expand All @@ -441,12 +450,13 @@ - (SentryId *)captureError:(NSError *)error withScope:(SentryScope *)scope
{
SentrySession *currentSession = _session;
SentryClient *client = _client;
if (nil != client) {
if (nil != currentSession) {
if (client != nil) {
if (currentSession != nil) {
return [client captureError:error
withScope:scope
incrementSessionErrors:^(void) { return [self incrementSessionErrors]; }];
} else {
_errorsBeforeSession++;
return [client captureError:error withScope:scope];
}
}
Expand All @@ -462,12 +472,13 @@ - (SentryId *)captureException:(NSException *)exception withScope:(SentryScope *
{
SentrySession *currentSession = _session;
SentryClient *client = _client;
if (nil != client) {
if (nil != currentSession) {
if (client != nil) {
if (currentSession != nil) {
return [client captureException:exception
withScope:scope
incrementSessionErrors:^(void) { return [self incrementSessionErrors]; }];
} else {
_errorsBeforeSession++;
return [client captureException:exception withScope:scope];
}
}
Expand All @@ -477,7 +488,7 @@ - (SentryId *)captureException:(NSException *)exception withScope:(SentryScope *
- (void)captureUserFeedback:(SentryUserFeedback *)userFeedback
{
SentryClient *client = _client;
if (nil != client) {
if (client != nil) {
[client captureUserFeedback:userFeedback];
}
}
Expand All @@ -489,10 +500,10 @@ - (void)addBreadcrumb:(SentryBreadcrumb *)crumb
return;
}
SentryBeforeBreadcrumbCallback callback = [options beforeBreadcrumb];
if (nil != callback) {
if (callback != nil) {
crumb = callback(crumb);
}
if (nil == crumb) {
if (crumb == nil) {
SENTRY_LOG_DEBUG(@"Discarded Breadcrumb in `beforeBreadcrumb`");
return;
}
Expand All @@ -514,7 +525,7 @@ - (SentryScope *)scope
@synchronized(self) {
if (_scope == nil) {
SentryClient *client = _client;
if (nil != client) {
if (client != nil) {
_scope = [[SentryScope alloc] initWithMaxBreadcrumbs:client.options.maxBreadcrumbs];
} else {
_scope = [[SentryScope alloc] init];
Expand All @@ -528,7 +539,7 @@ - (void)configureScope:(void (^)(SentryScope *scope))callback
{
SentryScope *scope = self.scope;
SentryClient *client = _client;
if (nil != client && nil != scope) {
if (client != nil && scope != nil) {
callback(scope);
}
}
Expand Down Expand Up @@ -592,15 +603,15 @@ - (void)removeAllIntegrations
- (void)setUser:(nullable SentryUser *)user
{
SentryScope *scope = self.scope;
if (nil != scope) {
if (scope != nil) {
[scope setUser:user];
}
}

- (void)captureEnvelope:(SentryEnvelope *)envelope
{
SentryClient *client = _client;
if (nil == client) {
if (client == nil) {
return;
}

Expand All @@ -612,7 +623,7 @@ - (SentryEnvelope *)updateSessionState:(SentryEnvelope *)envelope
if ([self envelopeContainsEventWithErrorOrHigher:envelope.items]) {
SentrySession *currentSession = [self incrementSessionErrors];

if (nil != currentSession) {
if (currentSession != nil) {
// Create a new envelope with the session update
NSMutableArray<SentryEnvelopeItem *> *itemsToSend =
[[NSMutableArray alloc] initWithArray:envelope.items];
Expand Down Expand Up @@ -643,7 +654,7 @@ - (BOOL)envelopeContainsEventWithErrorOrHigher:(NSArray<SentryEnvelopeItem *> *)
- (void)flush:(NSTimeInterval)timeout
{
SentryClient *client = _client;
if (nil != client) {
if (client != nil) {
[client flush:timeout];
}
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/Sentry/SentrySession.m
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#import "SentrySession.h"
#import "NSDate+SentryExtras.h"
#import "SentryCurrentDate.h"
#import "SentryInstallation.h"
#import "SentryLog.h"
#import "SentrySession+Private.h"

NS_ASSUME_NONNULL_BEGIN

Expand Down
4 changes: 3 additions & 1 deletion Sources/Sentry/include/SentrySession+Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ NS_ASSUME_NONNULL_BEGIN
NSString *nameForSentrySessionStatus(SentrySessionStatus status);

@interface
SentrySession (Private)
SentrySession ()

@property (nonatomic) NSUInteger errors;

- (void)setFlagInit;

Expand Down
27 changes: 27 additions & 0 deletions Tests/SentryTests/SentryHubTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,33 @@ class SentryHubTests: XCTestCase {
XCTAssertEqual(1, fixture.client.captureSessionInvocations.count)
}

func testCaptureErrorBeforeSessionStart() {
let sut = fixture.getSut()
sut.capture(error: fixture.error, scope: fixture.scope).assertIsNotEmpty()
sut.startSession()

XCTAssertEqual(fixture.client.captureErrorWithScopeInvocations.count, 1)
XCTAssertEqual(fixture.client.captureSessionInvocations.count, 1)

if let session = fixture.client.captureSessionInvocations.first {
XCTAssertEqual(session.errors, 1)
}
}

func testCaptureErrorBeforeSessionStart_DisabledAutoSessionTracking() {
fixture.options.enableAutoSessionTracking = false
let sut = fixture.getSut()
sut.capture(error: fixture.error, scope: fixture.scope).assertIsNotEmpty()
sut.startSession()

XCTAssertEqual(fixture.client.captureErrorWithScopeInvocations.count, 1)
XCTAssertEqual(fixture.client.captureSessionInvocations.count, 1)

if let session = fixture.client.captureSessionInvocations.first {
XCTAssertEqual(session.errors, 0)
}
}

func testCaptureWithoutIncreasingErrorCount() {
let sut = fixture.getSut()
sut.startSession()
Expand Down

0 comments on commit 9cb6c52

Please sign in to comment.