diff --git a/CHANGELOG.md b/CHANGELOG.md index 6573fca2..c01041f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +## XX.XX.XX +* Fixed a bug where a NullPointerException in SDKCore.recover() would permanently block SDK initialization when a crash file from a previous session existed on disk. + ## 24.1.4 * ! Minor breaking change ! User properties will now be automatically saved under the following conditions: * When an event is recorded diff --git a/app-java/src/main/java/ly/count/java/demo/ReproduceIssue263.java b/app-java/src/main/java/ly/count/java/demo/ReproduceIssue263.java new file mode 100644 index 00000000..bd9a89be --- /dev/null +++ b/app-java/src/main/java/ly/count/java/demo/ReproduceIssue263.java @@ -0,0 +1,118 @@ +package ly.count.java.demo; + +import java.io.BufferedWriter; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import ly.count.sdk.java.Config; +import ly.count.sdk.java.Countly; +import org.json.JSONObject; + +/** + * Reproducer for GitHub issue #263: + * "NullPointerException in SDKCore.recover() permanently blocks SDK initialization + * when a crash file exists" + * + * This simulates the scenario where: + * 1. A previous app run crashed and left a [CLY]_crash_* file on disk + * 2. The app restarts and calls Countly.init() + * 3. SDKCore.recover() finds the crash file and tries to process it + * + * WITHOUT the fix: NPE at networking.check(config) because networking is null + * WITH the fix: SDK initializes normally, crash is queued as a request + */ +public class ReproduceIssue263 { + + public static void main(String[] args) { + String[] sdkStorageRootPath = { System.getProperty("user.home"), "__COUNTLY", "java_issue_263" }; + File sdkStorageRootDirectory = new File(String.join(File.separator, sdkStorageRootPath)); + + if (!(sdkStorageRootDirectory.exists() && sdkStorageRootDirectory.isDirectory())) { + if (!sdkStorageRootDirectory.mkdirs()) { + System.out.println("[FAIL] Directory creation failed"); + return; + } + } + + // Step 1: Plant a fake crash file as if a previous run crashed + long crashTimestamp = System.currentTimeMillis() - 2000; + File crashFile = new File(sdkStorageRootDirectory, "[CLY]_crash_" + crashTimestamp); + + JSONObject crashData = new JSONObject(); + crashData.put("_error", "java.lang.RuntimeException: simulated crash from previous session\n" + + "\tat com.example.App.doSomething(App.java:42)\n" + + "\tat com.example.App.main(App.java:10)"); + crashData.put("_nonfatal", false); + crashData.put("_os", "Java"); + crashData.put("_os_version", System.getProperty("java.version")); + crashData.put("_device", "ReproducerDevice"); + crashData.put("_resolution", "1920x1080"); + + try (BufferedWriter writer = Files.newBufferedWriter(crashFile.toPath())) { + writer.write(crashData.toString()); + } catch (IOException e) { + System.out.println("[FAIL] Could not write crash file: " + e.getMessage()); + return; + } + + System.out.println("[INFO] Planted crash file: " + crashFile.getAbsolutePath()); + System.out.println("[INFO] Crash file exists: " + crashFile.exists()); + System.out.println(); + + // Step 2: Initialize SDK — this is where the NPE would occur + System.out.println("[TEST] Initializing SDK with crash file present..."); + System.out.println("[TEST] If issue #263 is NOT fixed, you will see a NullPointerException below."); + System.out.println(); + + try { + Config config = new Config("https://test.server.ly", "TEST_APP_KEY", sdkStorageRootDirectory) + .setLoggingLevel(Config.LoggingLevel.DEBUG) + .enableFeatures(Config.Feature.CrashReporting, Config.Feature.Events, Config.Feature.Sessions); + + Countly.instance().init(config); + + System.out.println(); + System.out.println("[PASS] SDK initialized successfully!"); + System.out.println("[INFO] Crash file still exists: " + crashFile.exists()); + + if (!crashFile.exists()) { + System.out.println("[PASS] Crash file was processed and removed during recovery."); + } else { + System.out.println("[WARN] Crash file was NOT removed — recovery may have partially failed."); + } + + // Check for request files (crash should be converted to a request) + File[] requestFiles = sdkStorageRootDirectory.listFiles( + (dir, name) -> name.startsWith("[CLY]_request_")); + if (requestFiles != null && requestFiles.length > 0) { + System.out.println("[PASS] Found " + requestFiles.length + " request file(s) — crash was queued for sending."); + } + + // Clean shutdown + Countly.instance().halt(); + System.out.println("[INFO] SDK stopped cleanly."); + + } catch (NullPointerException e) { + System.out.println(); + System.out.println("[FAIL] *** NullPointerException — Issue #263 is NOT fixed! ***"); + System.out.println("[FAIL] " + e.getMessage()); + e.printStackTrace(); + } catch (Exception e) { + System.out.println(); + System.out.println("[FAIL] Unexpected exception: " + e.getClass().getSimpleName() + ": " + e.getMessage()); + e.printStackTrace(); + } finally { + // Cleanup: remove test files + System.out.println(); + System.out.println("[INFO] Cleaning up test directory..."); + File[] files = sdkStorageRootDirectory.listFiles(); + if (files != null) { + for (File f : files) { + f.delete(); + } + } + sdkStorageRootDirectory.delete(); + System.out.println("[INFO] Done."); + } + } +} diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/SDKCore.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/SDKCore.java index b912c567..1a273436 100644 --- a/sdk-java/src/main/java/ly/count/sdk/java/internal/SDKCore.java +++ b/sdk-java/src/main/java/ly/count/sdk/java/internal/SDKCore.java @@ -459,8 +459,6 @@ public void init(final InternalConfig givenConfig) { modules.remove(feature); } - recover(config); - if (config.isDefaultNetworking()) { networking = new DefaultNetworking(); @@ -513,17 +511,23 @@ public Integer remaningRequests() { } } + recover(config); + user = new UserImpl(config); initFinished(config); } - private void initFinished(final InternalConfig config) { - modules.forEach((feature, module) -> module.initFinished(config)); - if (config.isDefaultNetworking()) { + private void checkNetworking(InternalConfig config) { + if (networking != null) { networking.check(config); } } + private void initFinished(final InternalConfig config) { + modules.forEach((feature, module) -> module.initFinished(config)); + checkNetworking(config); + } + public UserImpl user() { return user; } @@ -650,13 +654,13 @@ protected void recover(InternalConfig config) { public void onSignal(InternalConfig config, int id) { if (id == Signal.DID.getIndex()) { - networking.check(config); + checkNetworking(config); } } public void onSignal(InternalConfig config, int id, String param) { if (id == Signal.Ping.getIndex()) { - networking.check(config); + checkNetworking(config); } else if (id == Signal.Crash.getIndex()) { processCrash(config, Long.parseLong(param)); } @@ -679,7 +683,7 @@ private boolean processCrash(InternalConfig config, Long id) { if (Storage.push(config, request)) { L.i("[SDKCore] Added request " + request.storageId() + " instead of crash " + crash.storageId()); - networking.check(config); + checkNetworking(config); Boolean success = Storage.remove(config, crash); return (success != null) && success; } else { diff --git a/sdk-java/src/test/java/ly/count/sdk/java/internal/ScenarioInitRecoveryTests.java b/sdk-java/src/test/java/ly/count/sdk/java/internal/ScenarioInitRecoveryTests.java new file mode 100644 index 00000000..da006423 --- /dev/null +++ b/sdk-java/src/test/java/ly/count/sdk/java/internal/ScenarioInitRecoveryTests.java @@ -0,0 +1,359 @@ +package ly.count.sdk.java.internal; + +import java.io.File; +import ly.count.sdk.java.Config; +import ly.count.sdk.java.Countly; +import org.json.JSONObject; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** + * Tests for SDK initialization ordering and recovery scenarios. + * + * These tests verify that the SDK initializes correctly when leftover + * crash files or session files exist from a previous run. This covers + * the fix for GitHub issue #263 (NPE in SDKCore.recover() when a crash + * file exists) and related initialization ordering concerns. + */ +@RunWith(JUnit4.class) +public class ScenarioInitRecoveryTests { + + /** Time to wait for async storage/networking operations to settle */ + private static final int ASYNC_SETTLE_MS = 200; + + @Before + public void beforeTest() { + TestUtils.createCleanTestState(); + } + + @After + public void stop() { + Countly.instance().halt(); + } + + // ── Test helpers ──────────────────────────────────────────────────── + + private JSONObject createCrashData(String error, boolean nonfatal) { + JSONObject data = new JSONObject(); + data.put("_error", error); + data.put("_nonfatal", nonfatal); + return data; + } + + private long plantCrashFile(JSONObject crashData) { + long crashId = TimeUtils.uniqueTimestampMs(); + TestUtils.writeToFile("crash_" + crashId, crashData.toString()); + return crashId; + } + + private long plantCrashFileAndInit(Config config) throws InterruptedException { + long crashId = plantCrashFile(createCrashData("java.lang.RuntimeException: test", false)); + Countly.instance().init(config); + Thread.sleep(ASYNC_SETTLE_MS); + return crashId; + } + + private void assertCrashFileRemoved(long crashId) { + File crashFile = new File(TestUtils.getTestSDirectory(), "[CLY]_crash_" + crashId); + Assert.assertFalse("Crash file " + crashId + " should be removed after recovery", crashFile.exists()); + } + + private void assertCrashFileExists(long crashId) { + File crashFile = new File(TestUtils.getTestSDirectory(), "[CLY]_crash_" + crashId); + Assert.assertTrue("Crash file " + crashId + " should still exist", crashFile.exists()); + } + + private void assertMinRequestFiles(int expectedMin) { + File testDir = TestUtils.getTestSDirectory(); + File[] requestFiles = testDir.listFiles((dir, name) -> name.startsWith("[CLY]_request_")); + Assert.assertNotNull("Request file listing should not be null", requestFiles); + Assert.assertTrue("Expected at least " + expectedMin + " request file(s), found " + requestFiles.length, + requestFiles.length >= expectedMin); + } + + private void withNullNetworking(Runnable action) { + Networking saved = SDKCore.instance.networking; + SDKCore.instance.networking = null; + try { + action.run(); + } finally { + SDKCore.instance.networking = saved; + } + } + + // ── Crash recovery tests ──────────────────────────────────────────── + + /** + * "init_withExistingCrashFile" + * Primary regression test for issue #263. + * Crash file should be converted into a request and removed from disk. + */ + @Test + public void init_withExistingCrashFile() throws InterruptedException { + JSONObject crashData = createCrashData( + "java.lang.RuntimeException: test crash\n\tat com.test.App.main(App.java:10)", false); + crashData.put("_os", "Java"); + crashData.put("_os_version", "17"); + crashData.put("_device", "TestDevice"); + + long crashId = plantCrashFile(crashData); + assertCrashFileExists(crashId); + + Countly.instance().init(TestUtils.getBaseConfig().enableFeatures(Config.Feature.CrashReporting)); + Thread.sleep(ASYNC_SETTLE_MS); + + assertCrashFileRemoved(crashId); + assertMinRequestFiles(1); + } + + /** + * "init_withExistingCrashFile_noFeatureEnabled" + * Crash file recovery happens at SDKCore level regardless of feature flags. + * The crash file should still be processed and removed. + */ + @Test + public void init_withExistingCrashFile_noFeatureEnabled() throws InterruptedException { + long crashId = plantCrashFile(createCrashData("java.lang.NullPointerException: test", true)); + + Countly.instance().init(TestUtils.getBaseConfig()); + Thread.sleep(ASYNC_SETTLE_MS); + + Assert.assertTrue("SDK should be initialized", Countly.isInitialized()); + assertCrashFileRemoved(crashId); + assertMinRequestFiles(1); + } + + /** + * "init_withMultipleCrashFiles" + * All crash files should be converted to requests and removed. + */ + @Test + public void init_withMultipleCrashFiles() throws InterruptedException { + long[] crashIds = new long[3]; + for (int i = 0; i < 3; i++) { + crashIds[i] = plantCrashFile(createCrashData("Exception #" + i, i % 2 == 0)); + } + + Countly.instance().init(TestUtils.getBaseConfig().enableFeatures(Config.Feature.CrashReporting)); + Thread.sleep(ASYNC_SETTLE_MS); + + for (int i = 0; i < 3; i++) { + assertCrashFileRemoved(crashIds[i]); + } + assertMinRequestFiles(3); + } + + // ── Session recovery tests ────────────────────────────────────────── + + /** + * "init_withExistingSessionFile" + * Session recovery calls session.end() -> onSignal(Ping) -> networking.check(). + * Uses stop() (not halt()) to preserve session files on disk. + */ + @Test + public void init_withExistingSessionFile() throws InterruptedException { + Countly.instance().init(TestUtils.getBaseConfig().enableFeatures(Config.Feature.Sessions)); + Countly.instance().session().begin(); + Thread.sleep(ASYNC_SETTLE_MS); + + // stop() preserves files; halt() would delete them + Countly.instance().stop(); + + Countly.instance().init(TestUtils.getBaseConfig().enableFeatures(Config.Feature.Sessions)); + Thread.sleep(ASYNC_SETTLE_MS); + + Assert.assertTrue("SDK should be initialized after session recovery", Countly.isInitialized()); + } + + /** + * "init_withCrashAndSessionFiles" + * Both crash files and session files present simultaneously. + * Verifies recover() processes both crash loop and session loop without interference. + */ + @Test + public void init_withCrashAndSessionFiles() throws InterruptedException { + // First init: create a session file + Countly.instance().init(TestUtils.getBaseConfig() + .enableFeatures(Config.Feature.Sessions, Config.Feature.CrashReporting)); + Countly.instance().session().begin(); + Thread.sleep(ASYNC_SETTLE_MS); + Countly.instance().stop(); + + // Plant a crash file on top of the leftover session file + long crashId = plantCrashFile(createCrashData("java.lang.RuntimeException: dual recovery", false)); + + // Re-init should recover both + Countly.instance().init(TestUtils.getBaseConfig() + .enableFeatures(Config.Feature.Sessions, Config.Feature.CrashReporting)); + Thread.sleep(ASYNC_SETTLE_MS); + + Assert.assertTrue("SDK should initialize with both crash and session files", Countly.isInitialized()); + assertCrashFileRemoved(crashId); + } + + // ── Corrupt/empty file resilience ─────────────────────────────────── + + /** + * "init_withCorruptCrashFile" + * Corrupt crash file should not block initialization. + * processCrash returns false when Storage.read fails, so the file is NOT removed + * (the remove only happens after successful push). The SDK should still init. + */ + @Test + public void init_withCorruptCrashFile() throws InterruptedException { + long crashId = System.currentTimeMillis() - 1000; + TestUtils.writeToFile("crash_" + crashId, "this is not valid json {{{"); + + Countly.instance().init(TestUtils.getBaseConfig().enableFeatures(Config.Feature.CrashReporting)); + Thread.sleep(ASYNC_SETTLE_MS); + + Assert.assertTrue("SDK should be initialized even with corrupt crash file", Countly.isInitialized()); + // Corrupt file is NOT cleaned up by processCrash (it returns false at the null check) + // This is expected — the file will be retried on next init + } + + /** + * "init_emptyCrashFile" + * Empty crash file should not cause initialization failure. + */ + @Test + public void init_emptyCrashFile() throws InterruptedException { + long crashId = System.currentTimeMillis() - 1000; + TestUtils.writeToFile("crash_" + crashId, ""); + + Countly.instance().init(TestUtils.getBaseConfig().enableFeatures(Config.Feature.CrashReporting)); + Thread.sleep(ASYNC_SETTLE_MS); + + Assert.assertTrue("SDK should initialize with empty crash file", Countly.isInitialized()); + } + + // ── Feature combination tests ─────────────────────────────────────── + + /** + * "init_withCrashFile_remoteConfigEnabled" + * Exercises both crash recovery and ModuleRemoteConfig.initFinished() + * needing networking to be ready. + */ + @Test + public void init_withCrashFile_remoteConfigEnabled() throws InterruptedException { + long crashId = plantCrashFileAndInit(TestUtils.getBaseConfig() + .enableFeatures(Config.Feature.CrashReporting, Config.Feature.RemoteConfig)); + + Assert.assertTrue("SDK should initialize with crash file + remote config", Countly.isInitialized()); + assertCrashFileRemoved(crashId); + assertMinRequestFiles(1); + } + + /** + * "init_withCrashFile_locationEnabled" + * ModuleLocation.initFinished() -> sendLocation() -> onSignal(Ping) -> networking.check(). + */ + @Test + public void init_withCrashFile_locationEnabled() throws InterruptedException { + long crashId = plantCrashFileAndInit(TestUtils.getBaseConfig() + .enableFeatures(Config.Feature.CrashReporting, Config.Feature.Location) + .setLocation("US", "New York", "40.7128,-74.0060", null)); + + Assert.assertTrue("SDK should initialize with crash file + location", Countly.isInitialized()); + assertCrashFileRemoved(crashId); + assertMinRequestFiles(1); + } + + /** + * "init_withCrashFile_allFeaturesEnabled" + * Stress test: all module initFinished() paths exercised simultaneously. + */ + @Test + public void init_withCrashFile_allFeaturesEnabled() throws InterruptedException { + long crashId = plantCrashFileAndInit(TestUtils.getBaseConfig() + .enableFeatures( + Config.Feature.CrashReporting, + Config.Feature.Events, + Config.Feature.Sessions, + Config.Feature.Views, + Config.Feature.Location, + Config.Feature.RemoteConfig, + Config.Feature.Feedback + )); + + Assert.assertTrue("SDK should initialize with crash file + all features", Countly.isInitialized()); + assertCrashFileRemoved(crashId); + assertMinRequestFiles(1); + } + + // ── Networking null-safety tests ──────────────────────────────────── + + /** + * "init_networkingNullSafety_onSignalDID" + * Validates the null guard in SDKCore.onSignal(config, id) for DID signal. + */ + @Test + public void init_networkingNullSafety_onSignalDID() { + Countly.instance().init(TestUtils.getBaseConfig()); + + withNullNetworking(() -> + SDKCore.instance.onSignal(SDKCore.instance.config, SDKCore.Signal.DID.getIndex())); + + Assert.assertTrue("SDK should remain functional", Countly.isInitialized()); + } + + /** + * "init_networkingNullSafety_onSignalPing" + * Validates the null guard in SDKCore.onSignal(config, id, param) for Ping signal. + */ + @Test + public void init_networkingNullSafety_onSignalPing() { + Countly.instance().init(TestUtils.getBaseConfig()); + + withNullNetworking(() -> + SDKCore.instance.onSignal(SDKCore.instance.config, SDKCore.Signal.Ping.getIndex(), null)); + + Assert.assertTrue("SDK should remain functional", Countly.isInitialized()); + } + + /** + * "init_networkingNullSafety_processCrash" + * Plants a crash file AFTER init, nulls networking, then triggers the crash signal. + * This exercises the null guard inside processCrash() directly. + */ + @Test + public void init_networkingNullSafety_processCrash() throws InterruptedException { + Countly.instance().init(TestUtils.getBaseConfig().enableFeatures(Config.Feature.CrashReporting)); + Thread.sleep(ASYNC_SETTLE_MS); + + // Plant crash after init so it hasn't been processed yet + long crashId = plantCrashFile(createCrashData("java.lang.RuntimeException: post-init crash", false)); + + withNullNetworking(() -> + SDKCore.instance.onSignal(SDKCore.instance.config, SDKCore.Signal.Crash.getIndex(), String.valueOf(crashId))); + + Assert.assertTrue("SDK should remain functional after processCrash with null networking", Countly.isInitialized()); + } + + // ── Regression tests ──────────────────────────────────────────────── + + /** + * "init_repeatInit_withCrashFile" + * Verifies crash file is removed on first init so it doesn't permanently block startup. + * This was the user-visible symptom of issue #263. + */ + @Test + public void init_repeatInit_withCrashFile() throws InterruptedException { + long crashId = plantCrashFileAndInit( + TestUtils.getBaseConfig().enableFeatures(Config.Feature.CrashReporting)); + + Assert.assertTrue("First init should succeed", Countly.isInitialized()); + assertCrashFileRemoved(crashId); + + Countly.instance().halt(); + + // Second init — no crash file to recover + Countly.instance().init(TestUtils.getBaseConfig().enableFeatures(Config.Feature.CrashReporting)); + Thread.sleep(ASYNC_SETTLE_MS); + Assert.assertTrue("Second init should succeed without leftover crash files", Countly.isInitialized()); + } +}