Skip to content

Commit

Permalink
fix: Do not delete the app state (#2382)
Browse files Browse the repository at this point in the history
The app state is needed on the next app start, to be copied to previous app state. This is needed to determine the app start type.
  • Loading branch information
kevinrenskers authored Nov 14, 2022
1 parent 9754750 commit 2a547de
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 47 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
### Fixes

- Too long flush duration (#2370)
- Do not delete the app state when OOM tracking is disabled. The app state is needed to determine the app start type on the next app start. (#2382)

## 7.30.2

Expand Down
4 changes: 0 additions & 4 deletions Sources/Sentry/SentryAppStartTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -285,10 +285,6 @@ - (void)stop
[NSNotificationCenter.defaultCenter removeObserver:self
name:UIApplicationDidEnterBackgroundNotification
object:nil];

# if SENTRY_HAS_UIKIT
[self.appStateManager stop];
# endif
}

- (void)dealloc
Expand Down
8 changes: 0 additions & 8 deletions Sources/Sentry/SentryAppStateManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,6 @@ - (void)stop
removeObserver:self
name:SentryNSNotificationCenterWrapper.willTerminateNotificationName
object:nil];

[self deleteAppState];
}
}

Expand All @@ -124,7 +122,6 @@ - (void)dealloc
// In dealloc it's safe to unsubscribe for all, see
// https://developer.apple.com/documentation/foundation/nsnotificationcenter/1413994-removeobserver
[NSNotificationCenter.defaultCenter removeObserver:self];
[self deleteAppState];
}

/**
Expand Down Expand Up @@ -199,11 +196,6 @@ - (void)storeCurrentAppState
[self.fileManager storeAppState:[self buildCurrentAppState]];
}

- (void)deleteAppState
{
[self.fileManager deleteAppState];
}

#endif

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

- (void)storeCurrentAppState;

- (void)deleteAppState;

- (void)updateAppState:(void (^)(SentryAppState *))block;

#endif
Expand Down
27 changes: 1 addition & 26 deletions Tests/SentryTests/Helper/SentryAppStateManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,39 +65,14 @@ class SentryAppStateManagerTests: XCTestCase {
XCTAssertNil(fixture.fileManager.readAppState())
}

func testStopDeletesAppState() {
func testStopDoesNotDeleteAppState() {
XCTAssertNil(fixture.fileManager.readAppState())

sut.start()
XCTAssertNotNil(fixture.fileManager.readAppState())

sut.stop()
XCTAssertNil(fixture.fileManager.readAppState())
}

func testStopOnlyRunsLogicWhenStartCountBecomesZero() {
XCTAssertNil(fixture.fileManager.readAppState())

sut.start()
XCTAssertNotNil(fixture.fileManager.readAppState())

sut.start()

sut.stop()
XCTAssertNotNil(fixture.fileManager.readAppState())

sut.stop()
XCTAssertNil(fixture.fileManager.readAppState())
}

func testStoreAndDeleteAppState() {
XCTAssertNil(fixture.fileManager.readAppState())

sut.storeCurrentAppState()
XCTAssertNotNil(fixture.fileManager.readAppState())

sut.deleteAppState()
XCTAssertNil(fixture.fileManager.readAppState())
}

func testUpdateAppState() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class SentryAppStartTrackerTests: NotificationCenterTestCase {
func testSecondStart_AfterSystemReboot_IsColdStart() {
let previousBootTime = fixture.currentDate.date().addingTimeInterval(-1)
let appState = SentryAppState(releaseName: TestData.appState.releaseName, osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: previousBootTime)
givenPreviousAppState(appState: appState)
store(appState: appState)

startApp()

Expand All @@ -84,18 +84,33 @@ class SentryAppStartTrackerTests: NotificationCenterTestCase {

assertValidStart(type: .warm)
}

// Test for situation described in https://github.com/getsentry/sentry-cocoa/issues/2376
func testSecondStart_SystemNotRebooted_OOM_disabled_IsWarmStart() {
givenSystemNotRebooted()

fixture.options.enableOutOfMemoryTracking = false

fixture.fileManager.moveAppStateToPreviousAppState()
startApp()
assertValidStart(type: .warm)

fixture.fileManager.moveAppStateToPreviousAppState()
startApp()
assertValidStart(type: .warm)
}

func testAppUpgrade_IsColdStart() {
let appState = SentryAppState(releaseName: "0.9.0", osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: fixture.currentDate.date())
givenPreviousAppState(appState: appState)
store(appState: appState)

startApp()

assertValidStart(type: .cold)
}

func testAppWasInBackground_NoAppStartUp() {
givenPreviousAppState(appState: TestData.appState)
store(appState: TestData.appState)

startApp()

Expand All @@ -113,7 +128,7 @@ class SentryAppStartTrackerTests: NotificationCenterTestCase {
terminateApp()

let appState = SentryAppState(releaseName: "1.0.0", osVersion: "14.4.1", vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: self.fixture.currentDate.date())
givenPreviousAppState(appState: appState)
store(appState: appState)

fixture.fileManager.moveAppStateToPreviousAppState()
startApp()
Expand All @@ -126,7 +141,7 @@ class SentryAppStartTrackerTests: NotificationCenterTestCase {
*/
func testAppLaunches_PreviousBootTimeInFuture_NoAppStartUp() {
let appState = SentryAppState(releaseName: TestData.appState.releaseName, osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: fixture.currentDate.date().addingTimeInterval(1))
givenPreviousAppState(appState: appState)
store(appState: appState)

fixture.fileManager.moveAppStateToPreviousAppState()
startApp()
Expand Down Expand Up @@ -271,15 +286,15 @@ class SentryAppStartTrackerTests: NotificationCenterTestCase {
assertValidHybridStart(type: .warm)
}

private func givenPreviousAppState(appState: SentryAppState) {
private func store(appState: SentryAppState) {
fixture.fileManager.store(appState)
}

private func givenSystemNotRebooted() {
let systemBootTimestamp = fixture.currentDate.date()
fixture.sysctl.setProcessStartTimestamp(value: fixture.currentDate.date())
let appState = SentryAppState(releaseName: TestData.appState.releaseName, osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: systemBootTimestamp)
givenPreviousAppState(appState: appState)
store(appState: appState)
}

private func givenProcessStartTimestamp(processStartTimestamp: Date? = nil) {
Expand Down

0 comments on commit 2a547de

Please sign in to comment.