diff --git a/README.md b/README.md index 7fa8a7b..6058314 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ Then add the dependency in your module-level `build.gradle.kts`: ```kotlin dependencies { - implementation("com.github.metarouterio:android-sdk:1.1.0") + implementation("com.github.metarouterio:android-sdk:1.2.0") } ``` @@ -485,7 +485,7 @@ analytics.track("Button Clicked", "buttonName" to "Submit") "app": { "name": "MyApp", "version": "1.0.0", "build": "42" }, "screen": { "width": 411, "height": 891, "density": 2.63 }, "network": { "wifi": true }, - "library": { "name": "metarouter-android-sdk", "version": "1.1.0" }, + "library": { "name": "metarouter-android-sdk", "version": "1.2.0" }, "locale": "en-US", "timezone": "America/New_York" } diff --git a/gradle.properties b/gradle.properties index c4a2180..932abf7 100644 --- a/gradle.properties +++ b/gradle.properties @@ -14,4 +14,4 @@ android.enableJetifier=false kotlin.incremental=true # SDK Version -SDK_VERSION=1.1.0 +SDK_VERSION=1.2.0 diff --git a/metarouter-sdk/build.gradle.kts b/metarouter-sdk/build.gradle.kts index 786b5fc..d9f1041 100644 --- a/metarouter-sdk/build.gradle.kts +++ b/metarouter-sdk/build.gradle.kts @@ -19,7 +19,7 @@ android { testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" consumerProguardFiles("consumer-rules.pro") - buildConfigField("String", "SDK_VERSION", "\"${project.findProperty("SDK_VERSION") ?: "1.1.0"}\"") + buildConfigField("String", "SDK_VERSION", "\"${project.findProperty("SDK_VERSION") ?: "1.2.0"}\"") } buildTypes { diff --git a/metarouter-sdk/src/main/java/com/metarouter/analytics/dispatcher/Dispatcher.kt b/metarouter-sdk/src/main/java/com/metarouter/analytics/dispatcher/Dispatcher.kt index 783b72f..2d4d7a7 100644 --- a/metarouter-sdk/src/main/java/com/metarouter/analytics/dispatcher/Dispatcher.kt +++ b/metarouter-sdk/src/main/java/com/metarouter/analytics/dispatcher/Dispatcher.kt @@ -170,12 +170,22 @@ class Dispatcher( * Uses isFlushing guard to prevent concurrent/duplicate flushes. */ suspend fun flush() { + // If a retry is already armed, it IS the next attempt. Periodic flushJob + // ticks and offer() auto-flushes during the backoff window would otherwise + // re-enter processUntilEmpty, log redundantly, and cancel/relaunch the + // retry — visible as decreasing "retrying in Xms" log spam in a retry loop. + if (retryJob?.isActive == true) return if (!isFlushing.compareAndSet(false, true)) return if (queue.size() == 0) { isFlushing.set(false); return } try { - processUntilEmpty() - if (!networkPaused) { + val hadSuccess = processUntilEmpty() + // Only fire onFlushComplete after an actual 2xx this cycle. If the + // main channel failed (5xx, network error, circuit open), a disk + // overflow drain triggered from the callback would fail on the same + // endpoint and thrash disk I/O. It'll resume on the next successful + // flush or offline→online transition. + if (hadSuccess) { onFlushComplete?.invoke() } } finally { @@ -235,7 +245,7 @@ class Dispatcher( headers["Trace"] = "true" } - Logger.log("Making API call to: $url (overflow drain, ${batch.size} events)") + Logger.log("Making API call to: $url (disk drain, ${batch.size} events)") return try { networkClient.postJson( @@ -266,31 +276,32 @@ class Dispatcher( ) } - private suspend fun processUntilEmpty() { + private suspend fun processUntilEmpty(): Boolean { if (networkPaused) { if (queue.flushToOfflineStorage()) { Logger.log("Flushed queue to offline storage") } else { Logger.log("Dispatcher skipping flush — device is offline") } - return + return false } + var anySuccess = false while (queue.size() > 0) { if (networkPaused) { Logger.log("Dispatcher aborting flush — device went offline") - return + return anySuccess } val waitMs = circuitBreaker.beforeRequest() if (waitMs > 0) { Logger.warn("Circuit breaker ${circuitBreaker.getState()}, retrying in ${waitMs}ms (${queue.size()} event(s) pending)") scheduleRetry(waitMs) - return + return anySuccess } val events = queue.drain(maxBatchSize.get()) - if (events.isEmpty()) return + if (events.isEmpty()) return anySuccess // Inject sentAt timestamp val sentAt = getIso8601Timestamp() @@ -330,15 +341,20 @@ class Dispatcher( val retryDelay = maxOf(retryFloorMs(), circuitBreaker.beforeRequest()) Logger.warn("API call failed: ${e.message}, ${queue.size()} event(s) pending retry in ${retryDelay}ms (circuit: ${circuitBreaker.getState()}, retry #${consecutiveRetries.get()})") scheduleRetry(retryDelay) - return + return anySuccess + } + + if (ResponseCategory.from(response.statusCode) == ResponseCategory.SUCCESS) { + anySuccess = true } when (val action = handleResponse(response, events)) { is FlushAction.Continue -> {} // continue loop - is FlushAction.RetryAfter -> { scheduleRetry(action.delayMs); return } - is FlushAction.Stop -> return + is FlushAction.RetryAfter -> { scheduleRetry(action.delayMs); return anySuccess } + is FlushAction.Stop -> return anySuccess } } + return anySuccess } private fun handleResponse( @@ -413,6 +429,9 @@ class Dispatcher( retryJob?.cancel() retryJob = scope.launch { if (delayMs > 0) delay(delayMs) + // Null self-reference before flushing so the retryJob guard in flush() + // doesn't short-circuit our own attempt. + retryJob = null flush() } } diff --git a/metarouter-sdk/src/main/java/com/metarouter/analytics/network/NetworkMonitor.kt b/metarouter-sdk/src/main/java/com/metarouter/analytics/network/NetworkMonitor.kt index c626b6c..fd6d20d 100644 --- a/metarouter-sdk/src/main/java/com/metarouter/analytics/network/NetworkMonitor.kt +++ b/metarouter-sdk/src/main/java/com/metarouter/analytics/network/NetworkMonitor.kt @@ -71,16 +71,17 @@ class AndroidNetworkMonitor(context: Context) : NetworkMonitor { } override fun onLost(network: Network) { - // Only mark disconnected if there's truly no active network - val hasInternet = try { - val activeNet = cm.activeNetwork - val caps = activeNet?.let { cm.getNetworkCapabilities(it) } - caps?.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) == true + // activeNetwork may not be cleared yet when this fires, so we + // can't rely on re-querying capabilities. Instead, treat the + // loss as offline unless a *different* network is still active + // (handoff case, e.g. WiFi → cellular). + val activeNet = try { + cm.activeNetwork } catch (e: SecurityException) { - // Permission revoked at runtime — assume offline - false + null } - if (!hasInternet && isConnected) { + val stillOnline = activeNet != null && activeNet != network + if (!stillOnline && isConnected) { isConnected = false listener?.invoke(false) } diff --git a/metarouter-sdk/src/main/java/com/metarouter/analytics/queue/PersistableEventQueue.kt b/metarouter-sdk/src/main/java/com/metarouter/analytics/queue/PersistableEventQueue.kt index a322add..9648971 100644 --- a/metarouter-sdk/src/main/java/com/metarouter/analytics/queue/PersistableEventQueue.kt +++ b/metarouter-sdk/src/main/java/com/metarouter/analytics/queue/PersistableEventQueue.kt @@ -314,7 +314,7 @@ class PersistableEventQueue( if (response == null) { writeCheckpoint(remaining()) - Logger.warn("Offline overflow drain paused — network error ($totalDrained drained so far)") + Logger.warn("Disk drain paused — network error ($totalDrained drained so far)") return totalDrained } @@ -335,26 +335,26 @@ class PersistableEventQueue( ResponseCategory.PAYLOAD_TOO_LARGE -> { if (currentBatchSize > 1) { currentBatchSize = maxOf(1, currentBatchSize / 2) - Logger.warn("Overflow drain: payload too large (413) — reduced batch to $currentBatchSize") + Logger.warn("Disk drain: payload too large (413) — reduced batch to $currentBatchSize") } else { - Logger.warn("Overflow drain: dropping oversized event at batchSize=1 (messageId: ${batch.first().messageId})") + Logger.warn("Disk drain: dropping oversized event at batchSize=1 (messageId: ${batch.first().messageId})") offset++ } } ResponseCategory.FATAL_CONFIG -> { - Logger.error("Overflow drain: fatal config error ${response.statusCode} — discarding ${events.size - offset} overflow events") + Logger.error("Disk drain: fatal config error ${response.statusCode} — discarding ${events.size - offset} events") // Notify the dispatcher so the main pipeline also shuts down. Matches // the main flush path's handling of auth/config failures. dispatcher.onFatalConfigError?.invoke(response.statusCode) return totalDrained } ResponseCategory.CLIENT_ERROR -> { - Logger.warn("Overflow drain: client error ${response.statusCode} — dropping batch of ${batch.size}") + Logger.warn("Disk drain: client error ${response.statusCode} — dropping batch of ${batch.size}") offset += batch.size } ResponseCategory.SERVER_ERROR, ResponseCategory.RATE_LIMITED -> { writeCheckpoint(remaining()) - Logger.warn("Offline overflow drain paused — ${response.statusCode} ($totalDrained drained so far)") + Logger.warn("Disk drain paused — ${response.statusCode} ($totalDrained drained so far)") return totalDrained } } @@ -362,7 +362,7 @@ class PersistableEventQueue( // All events drained successfully synchronized(diskLock) { deleteDiskStore() } - Logger.log("Offline overflow disk drain complete ($totalDrained events)") + Logger.log("Disk drain complete ($totalDrained events)") return totalDrained } finally { isDraining.set(false) diff --git a/metarouter-sdk/src/test/java/com/metarouter/analytics/dispatcher/DispatcherTest.kt b/metarouter-sdk/src/test/java/com/metarouter/analytics/dispatcher/DispatcherTest.kt index 644f142..37e4ddb 100644 --- a/metarouter-sdk/src/test/java/com/metarouter/analytics/dispatcher/DispatcherTest.kt +++ b/metarouter-sdk/src/test/java/com/metarouter/analytics/dispatcher/DispatcherTest.kt @@ -581,6 +581,63 @@ class DispatcherTest { assertEquals(3, networkClient.requests.size) // 3 batches of 10, 10, 5 } + @Test + fun `onFlushComplete does not fire when flush fails and schedules retry`() = runTest { + val dispatcher = createDispatcher() + networkClient.nextResponse = NetworkResponse(500, emptyMap(), null) + queue.enqueue(createEvent("msg-1")) + + var completionCount = 0 + dispatcher.onFlushComplete = { completionCount++ } + + dispatcher.flush() + + // Flush hit a 5xx and scheduled a retry — the drain (triggered via + // onFlushComplete) would just fail on the same failing endpoint, so + // the callback must not fire until a real success happens. + assertEquals(0, completionCount) + assertTrue(dispatcher.getDebugInfo().pendingRetry) + dispatcher.stop() + } + + @Test + fun `onFlushComplete fires after successful flush`() = runTest { + val dispatcher = createDispatcher() + networkClient.nextResponse = NetworkResponse(200, emptyMap(), null) + queue.enqueue(createEvent("msg-1")) + + var completionCount = 0 + dispatcher.onFlushComplete = { completionCount++ } + + dispatcher.flush() + + assertEquals(1, completionCount) + } + + @Test + fun `flush is a no-op while retry is pending`() = runTest { + val dispatcher = createDispatcher() + networkClient.nextResponse = NetworkResponse(500, emptyMap(), null) + queue.enqueue(createEvent("msg-1")) + + // First flush: 500 → retry scheduled. 1 HTTP request so far. + dispatcher.flush() + assertEquals(1, networkClient.requests.size) + assertTrue(dispatcher.getDebugInfo().pendingRetry) + + // Additional flush() calls while the retry is armed must not issue new + // HTTP requests — the pending retry is the next attempt. This mirrors + // the periodic flushJob tick or offer() auto-flush calling flush() during + // the backoff window. + dispatcher.flush() + dispatcher.flush() + + assertEquals("No additional requests while retry pending", 1, networkClient.requests.size) + assertTrue("Retry still pending", dispatcher.getDebugInfo().pendingRetry) + + dispatcher.stop() + } + // ===== Retry Backoff Floor Tests ===== @Test @@ -1029,8 +1086,10 @@ class DispatcherTest { queue.enqueue(createEvent("msg-1")) dispatcher.flush() - queue.enqueue(createEvent("msg-2")) - dispatcher.flush() + // Let the scheduled retry fire so the second failure hits the CB + // threshold naturally. flush() is a no-op while the retry is armed. + testScheduler.advanceTimeBy(1100) + testScheduler.runCurrent() // CB should be open after hitting failure threshold assertEquals(CircuitState.Open, breaker.getState()) @@ -1142,7 +1201,7 @@ class DispatcherTest { context = EventContext( app = null, device = null, - library = LibraryContext(name = "test-sdk", version = "1.1.0"), + library = LibraryContext(name = "test-sdk", version = "1.2.0"), locale = null, network = null, os = null, diff --git a/metarouter-sdk/src/test/java/com/metarouter/analytics/enrichment/EventEnrichmentServiceTest.kt b/metarouter-sdk/src/test/java/com/metarouter/analytics/enrichment/EventEnrichmentServiceTest.kt index 9b7b2cb..9423427 100644 --- a/metarouter-sdk/src/test/java/com/metarouter/analytics/enrichment/EventEnrichmentServiceTest.kt +++ b/metarouter-sdk/src/test/java/com/metarouter/analytics/enrichment/EventEnrichmentServiceTest.kt @@ -336,7 +336,7 @@ class EventEnrichmentServiceTest { ) val eventContext = EventContext( device = deviceContext, - library = LibraryContext(name = "metarouter-android", version = "1.1.0") + library = LibraryContext(name = "metarouter-android", version = "1.2.0") ) every { contextProvider.getContext() } returns eventContext @@ -360,7 +360,7 @@ class EventEnrichmentServiceTest { ) val eventContext = EventContext( device = deviceContext, - library = LibraryContext(name = "metarouter-android", version = "1.1.0") + library = LibraryContext(name = "metarouter-android", version = "1.2.0") ) every { contextProvider.getContext() } returns eventContext @@ -378,7 +378,7 @@ class EventEnrichmentServiceTest { val eventContext = EventContext( device = null, - library = LibraryContext(name = "metarouter-android", version = "1.1.0") + library = LibraryContext(name = "metarouter-android", version = "1.2.0") ) every { contextProvider.getContext() } returns eventContext diff --git a/metarouter-sdk/src/test/java/com/metarouter/analytics/network/AndroidNetworkMonitorTest.kt b/metarouter-sdk/src/test/java/com/metarouter/analytics/network/AndroidNetworkMonitorTest.kt new file mode 100644 index 0000000..9ade2dc --- /dev/null +++ b/metarouter-sdk/src/test/java/com/metarouter/analytics/network/AndroidNetworkMonitorTest.kt @@ -0,0 +1,75 @@ +package com.metarouter.analytics.network + +import android.content.Context +import android.net.ConnectivityManager +import android.net.Network +import android.net.NetworkCapabilities +import android.net.NetworkRequest +import android.os.Build +import android.util.Log +import io.mockk.Runs +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.slot +import io.mockk.unmockkStatic +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [Build.VERSION_CODES.TIRAMISU]) +class AndroidNetworkMonitorTest { + + @Before + fun setup() { + mockkStatic(Log::class) + every { Log.d(any(), any()) } returns 0 + every { Log.w(any(), any()) } returns 0 + every { Log.e(any(), any()) } returns 0 + } + + @After + fun teardown() { + unmockkStatic(Log::class) + } + + @Test + fun `onLost fires offline when lost network matches stale activeNetwork on WiFi-only device`() { + val context = mockk() + val cm = mockk(relaxed = true) + val wifiNetwork = mockk() + val caps = mockk() + + every { context.getSystemService(Context.CONNECTIVITY_SERVICE) } returns cm + every { cm.activeNetwork } returns wifiNetwork + every { cm.getNetworkCapabilities(wifiNetwork) } returns caps + every { caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) } returns true + + val cbSlot = slot() + every { + cm.registerNetworkCallback(any(), capture(cbSlot)) + } just Runs + + val monitor = AndroidNetworkMonitor(context) + var offlineCalls = 0 + var onlineCalls = 0 + monitor.start { connected -> + if (connected) onlineCalls++ else offlineCalls++ + } + + // Simulate the race: WiFi is torn down, but cm.activeNetwork hasn't + // been cleared yet — it still returns the dying network with INTERNET cap. + cbSlot.captured.onLost(wifiNetwork) + + assertEquals("offline listener should fire exactly once", 1, offlineCalls) + assertEquals("online listener should not fire", 0, onlineCalls) + assertFalse("monitor should report disconnected", monitor.isConnected) + } +} diff --git a/metarouter-sdk/src/test/java/com/metarouter/analytics/types/EventTest.kt b/metarouter-sdk/src/test/java/com/metarouter/analytics/types/EventTest.kt index a89dba5..5c647d0 100644 --- a/metarouter-sdk/src/test/java/com/metarouter/analytics/types/EventTest.kt +++ b/metarouter-sdk/src/test/java/com/metarouter/analytics/types/EventTest.kt @@ -67,7 +67,7 @@ class EventTest { context = EventContext( app = AppContext( name = "Test App", - version = "1.1.0", + version = "1.2.0", build = "100", namespace = "com.test.app" ), @@ -79,7 +79,7 @@ class EventTest { ), library = LibraryContext( name = "MetaRouter Android SDK", - version = "1.1.0" + version = "1.2.0" ), os = OSContext( name = "Android", @@ -123,7 +123,7 @@ class EventTest { context = EventContext( app = AppContext( name = "Test App", - version = "1.1.0", + version = "1.2.0", build = "100", namespace = "com.test.app" ), @@ -135,7 +135,7 @@ class EventTest { ), library = LibraryContext( name = "MetaRouter Android SDK", - version = "1.1.0" + version = "1.2.0" ), os = OSContext( name = "Android",